add index_counter to DPMSolverMultistepScheduler

[yiyixuxu]

fix:#4122

[HuggingFaceDocBuilderDev]

The documentation is not available anymore as the PR was closed or merged.

[patrickvonplaten]

Looks nice!

1.) Could you maybe double-check that this is the correct implementation by running StableDiffusionImg2ImgPipeline with different strength values for SDv1-5? E.g. can you make sure that this code: #1866 (comment) works with your fix.
2.) Could you then propagate this fix also to all other schedulers that require this change (probs all k-diffusion ones expect DDIM, PNDM, DDPM)

[yiyixuxu]

actually, this didn't work - going to understand the algorithm a little bit more, but currently, having duplicated timesteps, i.e. when we have this timestep is equal to the next (s0=s1 in below code), r0 will be 0, cause the inf values in the calculation

diffusers/src/diffusers/schedulers/scheduling_dpmsolver_multistep.py

Line 514 in 8e5921c

r0 = h_0 / h

[yiyixuxu]

@patrickvonplaten

I'm not too sure what to do here -

For this scheduler, the "step" calculation is still based on timesteps; so it won't work when there are duplicated timesteps( when more than one sigma points to the same timestep)
https://github.com/huggingface/diffusers/blob/main/src/diffusers/schedulers/scheduling_dpmsolver_multistep.py#L486

Should we update the calculation to be based on sigmas?

[patrickvonplaten]

Yes I think updating the computation to be based on sigmas is the move here. Let's just make sure our step(...) API doesn't change :-)

Also gently pinging @LuChengTHU here - do you have any thoughts here maybe?

[LuChengTHU]

Hi all, it seems that this PR wants to fix the repeated time steps? But I'm not sure why the repeated time steps occurs, could you please explain the reason to me so that I could know the bugs in my code :) Thank you so much!

[yiyixuxu]

@LuChengTHU

The repeated time steps only occurs when we use the sigmas for step size based on the k-diffusion paper. When we convert the sigma to timesteps, sometimes same timestep is mapped to more than one sigmas, hence the repeated timesteps

Basically, this is the part of the code responsible https://github.com/huggingface/diffusers/blob/main/src/diffusers/schedulers/scheduling_dpmsolver_multistep.py#L269

We currently use a de-duplication logic to address the repeated timesteps, so if you run below code, you will get 95 steps, even though we asked for 100

from diffusers import DPMSolverMultistepScheduler

scheduler = DPMSolverMultistepScheduler(
    beta_start=0.00085,
    beta_end=0.012,
    beta_schedule="scaled_linear",
    prediction_type="epsilon",
    num_train_timesteps=1000,
    trained_betas=None,
    thresholding=False,
    algorithm_type="dpmsolver++",
    solver_type="midpoint",
    lower_order_final=True,
    use_karras_sigmas=True,
)

scheduler.set_timesteps(100, device="cuda")
print(len(scheduler.timesteps))

and this is the timesteps before the de-duplicate

 [999 992 985 978 971 963 956 948 940 933 924 916 908 899 891 882 873 864
 854 845 835 825 815 805 794 783 772 761 749 737 725 713 700 687 674 660
 646 632 618 603 587 572 556 540 523 506 489 472 454 436 418 400 382 364
 345 327 309 291 273 256 239 222 205 190 174 160 146 133 120 108  97  87
  78  69  61  53  47  41  35  31  26  23  19  16  14  12  10   8   7   6
   4   4   3   2   2   1   1   1   0   0]

[patrickvonplaten]

@yiyixuxu let's maybe just give this a try to fix it and see later with design

[github-actions]

This issue has been automatically marked as stale because it has not had recent activity. If you think this still needs to be addressed please comment on this thread.

Please note that issues that do not follow the contributing guidelines are likely to be ignored.

[patrickvonplaten]

@yiyixuxu let me know if you need help to get this one finished

[yiyixuxu]

it's been worked on this PR #4986

[patrickvonplaten]

Super-seeded by #4986