Skip to content

Conversation

@yiyixuxu
Copy link
Collaborator

@yiyixuxu yiyixuxu commented Jan 27, 2024

This refactor affects 16 schedulers that implement step_index counter, except the dpm multistep inverse scheduler (I did not update it; I removed the #copied from statement)

  1. added begin_index property and set_begin_index() method to all schedulers that implement the step_index counter. This will allow pipelines to set the first index for the scheduler's step_index counter 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)
  2. added an index_for_timestep method, which contains the logic to convert timestep into sigma index. This is now used by both _init_step_index() and add_noise() methods to decide the very first step_index when begin_index has not been set from the pipeline. The DPM schedulers (dpm-multistep, dpm-singlastep, deis, unipc, sasolver) has slightly different index_for_timestep method from the rest of the schedulers.
  3. refactored _init_step_index() and add_noise() methods across the schedulers so they have consistent definitions and added the #Copied from statement whenever possible
  4. remove the legacy _index_counter in heun and a couple other schedulers - it was introduced before we had the step_index counter and no longer needed

@HuggingFaceDocBuilderDev

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.

@yiyixuxu yiyixuxu changed the title [wip]Scheduler.set_begin_index [refactor]Scheduler.set_begin_index Jan 29, 2024
Comment on lines 118 to 120
@begin_index.setter
def begin_index(self, index):
self._begin_index = index
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok will remove :)

@patrickvonplaten
Copy link
Contributor

Generally this PR looks good to me - just wondering about the self.begin_index setter method, it doesn't seem like we need it.

@yiyixuxu yiyixuxu merged commit adcbe67 into main Feb 1, 2024
@yiyixuxu yiyixuxu deleted the scheduler-set-begin-index branch February 1, 2024 19:51
dg845 pushed a commit to dg845/diffusers that referenced this pull request Feb 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants