Feature/ali #597
Feature/ali #597
Conversation
|
Nice work! Quick comment from a glimpse (will add more details later): since you're only calling parent methods for |
edward/inferences/ali.py
Outdated
| ``ALI`` matches a mapping from data to latent variables and a | ||
| mapping from latent variables to data through a joint | ||
| discriminator. The encoder approximates the posterior p(z|x) | ||
| when the network is stochastic. |
There was a problem hiding this comment.
Actually this is untrue. The terminology "Adversarially learned inference" is a misnomer because the distribution p( z | x) in GANs is degenerate. More accurately would be to say
Using a joint discriminator, ``ALI`` learns a mapping from noise to data
as well as an inverse mapping from data to noise. The inference network
is the inverse mapping.
We discuss this misconception in my paper on deep implicit models (see the noise vs latent variables part of section 2.1).
I personally prefer the other paper which calls it "adversarial feature learning"/BiGANs; to me this description is more accurate. So I'd be for BiGANInference, but if you feel strongly about ALI, that's okay too.
edward/inferences/ali.py
Outdated
|
|
||
|
|
||
| class ALI(GANInference): | ||
| """Adversarially Learned Inference (Dumoulin et al., 2016) or |
There was a problem hiding this comment.
Both were published at ICLR 2017, so you can update the citation with ... et al., 2017.
edward/inferences/ali.py
Outdated
| >>> with tf.variable_scope("Gen"): | ||
| >>> xf = gen_data(z_ph) | ||
| >>> zf = gen_latent(x_ph) | ||
| >>> inference = ed.ALI({xf: x_data, zf: z_samples}, discriminator) |
There was a problem hiding this comment.
To match the interface of other inference methods, what about
ed.ALI(latent_vars, data, discriminative_network) # signature
ed.ALI({zf: qz}, {xf: x_data}, discriminator) # exampleHere the model's noise zf is binded to the inference network output qz, and the model's generated output xf is binded to data x_data.
|
@AlexLewandowski Great work on the updates. Is this waiting on anything? Happy to merge. |
|
Yeah, you can merge. I'm working on a project so if I have any other ideas, I'll push them this way. |
edward/inferences/bigan_inference.py
Outdated
| >>> with tf.variable_scope("Gen"): | ||
| >>> xf = gen_data(z_ph) | ||
| >>> zf = gen_latent(x_ph) | ||
| >>> inference = ed.BiGANInference({latent_vars: qz}, {xf: x_data}, discriminator) |
There was a problem hiding this comment.
Should this line be {z_ph: zf} and {xf: x_ph}?
edward/inferences/bigan_inference.py
Outdated
|
|
||
| def build_loss_and_gradients(self, var_list): | ||
|
|
||
| x_true = list(six.itervalues(self.data))[0] |
There was a problem hiding this comment.
With a dictionary, self.data doesn't guarantee the first key-value pair is x and the second is z. I think you can keep the self.latent_vars / self.data attributes stored separately, doing something like this for __init__:
if not callable(discriminator):
raise TypeError("discriminator must be a callable function.")
self.discriminator = discriminator
# call grandparent's method; avoid parent (GANInference)
super(GANInference, self).__init__(latent_vars, data)In addition, for consistency with other inferences, the zs should be ordered as z_true: z_fake, i.e., the prior latent variable is binded to the output of the inference network. With the above you can do
z_true = list(six.iterkeys(self.latent_vars))[0]
z_fake = list(six.itervalues(self.latent_vars))[0]There was a problem hiding this comment.
Yeah that's my mistake, I didn't realize that the order was different. I made the changes, hopefully this is more consistent.
* Create ali.py in proper branch * Added example for ali.py * Made example/ali.py more inline with example/gan.py * Mistake in hyperparameter placement following last commit * Update and rename ali.py to bigan_inference.py * Update __init__.py * Update __init__.py * updated and renamed example/ali.py to example/bigan_example.py * updated and renamed example/ali.py to example/bigan_example.py * change bigan_example.py and bigan_inference.py to adhere to PEP8
Following the pull request from last week, I added an example for ali.py. The MNIST reconstructions are not perfect, but I haven't fiddled with the hyperparameters very much.