Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

is KLD calculation correct? #3

Open
victor-shepardson opened this issue Sep 30, 2017 · 5 comments
Open

is KLD calculation correct? #3

victor-shepardson opened this issue Sep 30, 2017 · 5 comments

Comments

@victor-shepardson
Copy link

AGE/src/losses.py

Lines 38 to 41 in 0915760

t1 = (samples_var.pow(2) + samples_mean.pow(2)) / 2
t2 = -samples_var.log()
KL = (t1 + t2 - 0.5).mean()

KLD appears to use variance in place of standard deviation. utils.var() computes variance as squared distance from mean. Then it's squared again in the KLN01Loss module. Should it be (in the default 'qp' direction):

t1 = samples_var + samples_mean.pow(2)
t2 = -samples_var.log()

KL = (t1 + t2 - 1).mean()/2

?

(Additionally, the paper gives the KLD as a sum but here it's a mean, changing the meaning of the hyperparameters weighting the reconstruction losses)

@DmitryUlyanov
Copy link
Owner

Hi, yes, it looks like a mistake. Thanks for spotting it.

I will also change sum to mean in the paper, thank again!

Best,
Dmitry

@DmitryUlyanov
Copy link
Owner

I will fix it in in several days, when I will have time to make sure everything still works.

@victor-shepardson
Copy link
Author

No problem! I think the reconstruction losses in the paper are similarly given as norms, where they are also means in the code. And the latent space loss is said to be L2 but appears to really be cosine.

@jshanna100
Copy link

The encoder transforms all output vectors to have a norm of 1 (i.e. mapping to a unit sphere). But if this is the case, a batch of such vectors cannot reach the unit Gaussian as demanded by the KL loss function, even when perfectly distributed around the sphere.

Have I missed something, or shouldn't the loss function be calculated with the standard deviation of the transformed vectors, rather than 1?

@DmitryUlyanov
Copy link
Owner

KL divergence in fact will not be zero in the perfect case, but when KL is minimal Q ~ uniform on sphere, that is what we want.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants