This repository was archived by the owner on Feb 7, 2025. It is now read-only.
446 Fix the way reversed_step in DDIM treats the first alpha in alphas_cumprod #452
+13
−8
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
…mprod. This value in now set to zero or taken from the last element in the arrary depending on a value of an init argument. Also, names of local variables in reversed_step are fixed to reflect the actual logic.
Contributor
Author
|
Not sure about using the same |
marksgraham
approved these changes
Dec 21, 2023
Collaborator
marksgraham
left a comment
marksgraham
left a comment
There was a problem hiding this comment.
Hi @matanat
This looks good to me, thanks!
I think it is best to define a new variable first_alpha_cumprod, as you have done - it is clearer this way and will not break any compatibility with already implemented code.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
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.
In
reversed_step,when traversing the timesteps all the way to the last, an exception is raised by exceeding the boundaries of thealphas_cumprod. This is now fixed by setting the first value of thealphas_cumprodto either zero or to the second-to-last element. The behavior is set by the init argumentset_alpha_to_onewhich control the behavior regarding the first element in the array.Also, names of local variables in the function are fixed.
Linked to: issue 446