huggingface / diffusers

🤗 Diffusers: State-of-the-art diffusion models for image and audio generation in PyTorch and FLAX.
https://huggingface.co/docs/diffusers
Apache License 2.0
26.11k stars 5.38k forks source link

`TCDScheduler` : `eta` out of range? #8305

Closed Clement-Lelievre closed 4 months ago

Clement-Lelievre commented 5 months ago

Describe the bug

Hi,

I am starting to play with the relatively recent scheduler TCDScheduler and encountered a bug yesterday.

The thing is, I'm embarrassed to say I am unable to provide the exact details of the bug. Right now I am unable to reproduce it again but I'd like to make sure it gets eradicated.

Here is however information from my memory that may help reproduce it:

_schedulers = [
    DDIMScheduler,
    DDPMScheduler,
    DEISMultistepScheduler,
    DPMSolverMultistepScheduler,
    DPMSolverSinglestepScheduler,
    EulerAncestralDiscreteScheduler,
    EulerDiscreteScheduler,
    HeunDiscreteScheduler,
    KDPM2AncestralDiscreteScheduler,
    KDPM2DiscreteScheduler,
    LCMScheduler,
    LMSDiscreteScheduler,
    PNDMScheduler,
    TCDScheduler,
    UniPCMultistepScheduler,
]

I had a cursory glance at diffusers code and couldn't see how I managed to somehow get an invalid eta value. I saw it defaults to 0.3 in TCDScheduler.step Do you have a hint? A way to produce the bug when switching schedulers, maybe by being clumsy about the specific config params to use?

This is perhaps best dealt with by either @mhh0318 or @vanakema as they're the main contributors to this scheduler's code. (I am aware that the code originates from an external repo (jabir-zheng)). cc @yi

Reproduction

as stated above, no MRE unfortunately but some level of detail to help narrow down the problem

Logs

again unfortunately all I know is I ran into the `AssertionError "gamma must be less than or equal to 1.0"`

System Info

Who can help?

@yiyixuxu @vanakema @mhh

vanakema commented 4 months ago

Oh man haha, I only appear like one of the main contributors because I did a huge refactoring of the Tensor type used across the repo. Someone else would definitely be better equip to answer this

Clement-Lelievre commented 4 months ago

Update: the error came from our implem, not diffusers'. (FWIW, I had generated a random eta value for DDIMScheduler that was > 1 and then the scheduler was changed to TCDScheduler with the same eta value, triggering the assertionerror I linked above)

I'll close this issue