Fix denoising sampling #16
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The q_posterior function is not correct and not needed for sampling.
|
Disclaimer: I'm still figuring out the sampling code :D not sure what's correct. I'm just wondering to which equations in the DDPM paper the As of now, it seems that |
|
closing the PR as hojonathanho/diffusion#5 seems to answer it... |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The
q_posteriorfunction as implemented is not correct and is not needed for sampling. Recall in Ho et al. equation (7) is forx_0which is not known during sampling.And in Alg. 2 line 4 we just need the slightly denoised sample.
Issue flagged to me by @NielsRogge so all credit to him if I am not mistaken... and if I am mistaken then it's my fault!