-
Notifications
You must be signed in to change notification settings - Fork 6.6k
Description
What API design would you like to have changed or added to the library? Why?
@jonatanklosko brought up an issue with some confusing code (why are alphas indexed differently across PNDMScheduler and DDIMScheduler), and some inconsistency in the offset usage.
Two questions:
- Why does only one shift by offset?
- Can
offsetbe changed from an arbitraryintto aboolsetting that shifts by 1 (the integer shift is more confusing if the only use case is stable diffusion inference withoffset = 1)
Discussion began in #444, see here
diffusers/src/diffusers/schedulers/scheduling_pndm.py
Lines 338 to 339 in 8eaaa54
| alpha_prod_t = self.alphas_cumprod[timestep + 1 - self._offset] | |
| alpha_prod_t_prev = self.alphas_cumprod[timestep_prev + 1 - self._offset] |
diffusers/src/diffusers/schedulers/scheduling_ddim.py
Lines 204 to 205 in 8eaaa54
| alpha_prod_t = self.alphas_cumprod[timestep] | |
| alpha_prod_t_prev = self.alphas_cumprod[prev_timestep] if prev_timestep >= 0 else self.final_alpha_cumprod |
I can tell you that the PNDMScheduler and DDIMScheduler have indexed the alpha's differently for a while. Before introducing offset, PNDMScheduler had
alpha_prod_t = self.alphas_cumprod[timestep + 1]
alpha_prod_t_prev = self.alphas_cumprod[timestep_prev + 1]
And DDIMScheduler had
alpha_prod_t = self.alphas_cumprod[timestep]
alpha_prod_t_prev = self.alphas_cumprod[timestep_prev]
At minimum, we can figure this out and document better!