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

bug fix IWAE: use SGVB1 instead of SGVB2, detach sample weights gradi… #53

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

tongdaxu
Copy link

@tongdaxu tongdaxu commented Mar 15, 2022

Hi thanks for this amazing project, I like it.

I find an implementation detail of Importance Weighted Autoencoder concerning:

  • First, it is SGVB 1 (Auto-encoding Variational Bayes Eq 6) should be used instead of SGVB 2 (Auto-encoding Variational Bayes Eq 7, 10) for estimating the evidence lower bound
  • Second, the importance weights should be detached, or the gradient is different from (Importance Weighted Autoencoder Eq 14). To be specific:
    • in EQ 14: grad = \Sum w * \nabla ELBO
    • in current impl, grad = \Sum \nabla (w * ELBO) = \Sum w * \nabla ELBO + \nabla w * EBLO, an extra term is introduced

I made a loosy PR as I do not know how to run this repo properly. I would appreciate any effort to review and correct it.

Thanks

@tongdaxu
Copy link
Author

This problem is elaborated in issue: #34.

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

Successfully merging this pull request may close these issues.

1 participant