Merged
Conversation
|
The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update. |
added 13 commits
January 27, 2024 08:33
…step_index copy from dpm
yiyixuxu
changed the title
Comment on lines
118
to
120
| @begin_index.setter | ||
| def begin_index(self, index): | ||
| self._begin_index = index |
Contributor
There was a problem hiding this comment.
Suggested change
| @begin_index.setter | |
| def begin_index(self, index): | |
| self._begin_index = index | |
| @begin_index.setter | |
| def begin_index(self, index): | |
| self._begin_index = index |
Are we using this function?
I don't really see a self.begin_index = ... anywhere
Collaborator
Author
There was a problem hiding this comment.
ok will remove :)
Contributor
|
Generally this PR looks good to me - just wondering about the |
AmericanPresidentJimmyCarter
pushed a commit
to AmericanPresidentJimmyCarter/diffusers
that referenced
this pull request
Apr 26, 2024
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 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.
This refactor affects 16 schedulers that implement
step_indexcounter, except the dpm multistep inverse scheduler (I did not update it; I removed the#copied fromstatement)begin_indexproperty andset_begin_index()method to all schedulers that implement thestep_indexcounter. This will allow pipelines to set the first index for the scheduler'sstep_indexcounter before the denoising loop. This is particularly useful for img2img pipelines because it may have duplicated first timesteps (see more on https://github.com/huggingface/diffusers/pull/5746/files)index_for_timestepmethod, which contains the logic to convert timestep into sigma index. This is now used by both_init_step_index()andadd_noise()methods to decide the very firststep_indexwhenbegin_indexhas not been set from the pipeline. The DPM schedulers (dpm-multistep, dpm-singlastep, deis, unipc, sasolver) has slightly differentindex_for_timestepmethod from the rest of the schedulers._init_step_index()andadd_noise()methods across the schedulers so they have consistent definitions and added the#Copied fromstatement whenever possible_index_counterin heun and a couple other schedulers - it was introduced before we had the step_index counter and no longer needed