Skip to content
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

[Feature]: Custom KDiffusion Scheduler #1242

Closed
KohakuBlueleaf opened this issue Jun 1, 2023 · 8 comments
Closed

[Feature]: Custom KDiffusion Scheduler #1242

KohakuBlueleaf opened this issue Jun 1, 2023 · 8 comments
Labels
enhancement New feature or request

Comments

@KohakuBlueleaf
Copy link

Feature description

Based on AUTOMATIC1111/stable-diffusion-webui#10621
and pr at AUTOMATIC1111/stable-diffusion-webui#10649

I think it is worth to add this feature into this repo too.
But the problem is, I don't know if @vladmandic you want the settings one or something more like my original pr AUTOMATIC1111/stable-diffusion-webui#10634

And if you want, I can add something like export the plot of the noise schedule. (I think this is doable?)

Version Platform Description

No response

@KohakuBlueleaf KohakuBlueleaf added the enhancement New feature or request label Jun 1, 2023
@vladmandic
Copy link
Owner

Thanks @KohakuBlueleaf - I'd love to have this!

I see that A1111 opted for global settings - I actually have different opinion, much closer to your original proposal.

The reason is that I believe that any setting that user may choose to modify frequently should be local and passed as parameter to actual generate function instead of generate function reading it from options.

Why?

  1. using global options causes significant churn as each update also triggers re-parsing of all options and saves updated config.json. So if you use control like input range and then update values to something like 1->2->3->4->3->2->1, you actually have no idea if final value is actually 1 or are some updates skipped because of lag. UI may read different than actual option. As such options should not be used for values that can frequently change.
  2. For any global option, value is read once generate actually starts, not at the time its submitted. Which means that working with queues can result in wrong values used. It doesn't matter if job queues don't exist, but I want them to make a reality! And then you just cant submit a job, change value and submit another since value would be read at the job start even if value was different when job was submitted.

Btw, for that exact reason I've reimplemented clip_skip to be a local value instead of global (it is no longer in opts). I need to do same with some others as well.

@KohakuBlueleaf
Copy link
Author

@vladmandic ok
I will try to add custom k_diffusion scheduler to your repo and then make a PR for it
and let it become something like a dropdown with additional setting section
or something like hires-fix

What I'm thinking is something like remove all the "karras" sampler, and then use the dropdown for user to choose the sceduler.

So k_diffusion sampler with scheduler karras (default sigma) will behave like old xxx karras sampler
What do you think about this?

@vladmandic
Copy link
Owner

yes, i think that behavior like hires fix would be perfect - show options when checkbox is selected.
and i don't mind messing with scheduler list
i'll try to do the same for unipc as its settings are right now global.

@vladmandic
Copy link
Owner

@KohakuBlueleaf btw, since you're interested in schedulers, take a look at #1268. i think notes raised in paper are very valid and this is pretty high-value item (also enabled via checkbox to have backwards compatibility) - what do you think?

@KohakuBlueleaf
Copy link
Author

@vladmandic first, there is 3 problem
1, snr=0 means sigma = inf, which is impossible in k_diffusion, you can only get some super high sigma (which already be possible with custom scheduler)

2, you need to change the alpha schedule in models to make sure k_diffusion has correct conversion between sigma and timesteps

3, I already try 1,2 on normal sd model and it just broken all the things, basically max_sigma higher than 1000 will give you super bad result. Normally max_sigma = 100 is the upper bound.
I guess this may caused by the scaling method and numerical error in k_diffusion

Conclusions:
If you have fixes for alpha schedule (which need to hijack sd-model), custom scheduler can let you use snr≈0.
Since I don't think use it for non-finetuned model is a good idea, I will not want to make a method to let user manualy change the alpha schedule in the model ckpt.

And if the model already contain fixed/different schedule, just use 0.0 for sigma min/max and it will read the value from models' params

@vladmandic
Copy link
Owner

@KohakuBlueleaf just looping back on this one - i see that your pr is merged in a1111 dev branch.
would you like to create pr here as well or should i look at porting at some time in the future.

@KohakuBlueleaf
Copy link
Author

@vladmandic I'm quite busy these days (until 7/5 I guess)
I think it is actually not hard to port?
you can consider just port by yourself

@vladmandic
Copy link
Owner

finally got around to this :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants