-
Notifications
You must be signed in to change notification settings - Fork 27k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Protect alphas_cumprod during refiner switchover #14979
Conversation
|
I'm not sure this should really be considered an issue with I could look into moving it to within the reload function, but I think that this would have more potential to cause issues in other parts of the code and I would need to test it further to verify that it doesn't. In the mean time, I've removed protection of |
I just don't get this or why it helps. How is it happening that Edit: I guess it's because of this code in processing.py:
But then there should be the same problem when switching models during hires fix. |
It is related to the implementation of zero terminal SNR and the compatibility fix that came with it. Doing either of those things requires changing the model alphas_cumprod to reflect those changes, and how it is implemented currently is that this is done at runtime, before each sampling step, as an override. When the model weights are reloaded, this override is undone, since the newly-loaded model weights come with their own alphas_cumprod value (which in almost every case will be the same as it is by default on the other model, but which will be different if we overrode the values to something other than what is in the model). And then this will result in the wrong timestep being called for the value of sigma used on that sampling step. As in my example, the sigma value of 0.5725 would correspond to timestep 190 under a zero terminal SNR schedule, but would correspond to timestep 200 on the default schedule -- which is a problem, because it means that not only is the schedule now wrong, it means the refiner is now sampling from a step it was never meant to sample from. The alphas_cumprod override will be reapplied next step, but at this point there's already going to be artifacts in the final image from the bad refiner step. I do see the logical solution to this problem to be maintaining the override through the model switch. I'll look into whether hiresfix has the same issue, though I would expect it to be far more benign there in any case. |
If I'm understanding this right, the proper fix, I think, would be to take this code from processing:
And put it in a separate function, and call this function both in processing where it was originally and in |
I'm not entirely sure where the best place to put the alpha override function is, but I had to relocate it to sd_models.py to avoid circular imports, however it can be easily moved to any other location that won't cause circular import errors if needed. The fix works as it stands. |
Protect alphas_cumprod during refiner switchover
Protect alphas_cumprod during refiner switchover
Description
Screenshots/videos:
Before fix, image generated on DPM++ 2M, overridden with Karras schedule and sigma_max of 1500, 50 steps:
This specific schedule causes one of the sampling steps to be changed from timestep 190 to timestep 200. The highest timestep a typical refiner is trained for is 199 (last 200, zero indexed), so this is out of the range for the model and causes extra noise in the output.
After the fix is applied:
The resulting image looks much cleaner (particularly the background at the top left).
Checklist: