Skip to content

Conversation

CiaraStrawberry
Copy link

As it is, you have to call set_timesteps to initialize the correct karras sigmas values with the EulerDiscreteScheduler, which isn't done during training and so would cause incorrect noise for anyone attempting to implement a training script with SVD.

@patil-suraj

As it is, you have to call set_timesteps to initialize the correct karras sigmas noise, which isn't done during training and so can cause incorrectly set noise.
@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.

@patrickvonplaten
Copy link
Contributor

I don't fully understand this fix, the EulerDiscreteScheduler already has some karras sigmas

@CiaraStrawberry
Copy link
Author

The problem is this -> sigmas = self._convert_to_karras(in_sigmas=sigmas, num_inference_steps=num_train_timesteps)

This function is only called in "set_timesteps" right now, while the add_noise and step functions expect it to have been called, which means if you're writing a training script, you need to call set_timesteps or calculate the sigmas yourself for the timesteps with 0.25 * sigma.log(), which is how svd expects the timestep input to be done.

https://github.com/pixeli99/SVD_Xtend/blob/main/train_svd.py

You can see it with pixeli99s training script, he had to externalize a bunch of the scheduler functionality into the training loop, with my change, much of that would not have been needed, so the 0.25 * sigma.log() part would already be done, which would mean the noisy_latents code could be done with set_noise as usual.

@patrickvonplaten
Copy link
Contributor

cc @yiyixuxu

@patil-suraj
Copy link
Contributor

@patrickvonplaten this is the issue we had discussed when making changes to Euler for SVD.

SVD follows full EDM style training, so it changes the sigmas and the pre-conditioning and if we want to do training like that we need to adapt the scheduler init to use correct sigmas, and pre-conditioning. In general this change should apply to all karras-style schedulers (euler, euler-a, heun etc) And it's not enough to just correct sigmas, we also need to adapt the scaling params (c_in, c_skip, c_noise etc) for all prediction types.

I'm not fully sure if we should put this EulerDiscreteScheduler or create a new scheduler class for this which strictly follows EDM, the name DiscreteScheduler is not consistent with this since EMD is continuous formulation.

@patrickvonplaten
Copy link
Contributor

@patrickvonplaten this is the issue we had discussed when making changes to Euler for SVD.

SVD follows full EDM style training, so it changes the sigmas and the pre-conditioning and if we want to do training like that we need to adapt the scheduler init to use correct sigmas, and pre-conditioning. In general this change should apply to all karras-style schedulers (euler, euler-a, heun etc) And it's not enough to just correct sigmas, we also need to adapt the scaling params (c_in, c_skip, c_noise etc) for all prediction types.

I'm not fully sure if we should put this EulerDiscreteScheduler or create a new scheduler class for this which strictly follows EDM, the name DiscreteScheduler is not consistent with this since EMD is continuous formulation.

If it applies to all schedulers, it'd be better to apply it directly to the schedulers in my opinion

Copy link
Contributor

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.

@github-actions github-actions bot added the stale Issues that haven't received updates label Jan 26, 2024
@github-actions github-actions bot closed this Feb 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale Issues that haven't received updates
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants