-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Allow k_dpmpp_2s_ancestral and k_dpmpp_2m #1767
Conversation
Tested and works so far for 'k_dpmpp_2s_ancestral' and 'k_dpmpp_2m'. NOT working for 'k_dpm_fast', 'k_dpm_adaptive' out of the box
@spezialspezial - Any chance you can walk through the purpose of these changes? It doesn't seem to add any samplers to the constants the app uses, so it doesn't seem to be a full addition of samplers? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not approved. this kind of code is a security risk. plus there's no frontend support. compare the amount of code in #1389 - this is what's needed to add a new sampler.
The purpose is beeing able to use k_dpmpp_2s_ancestral and k_dpmpp_2m as an api user until you catch up with the webgui.
I'm using prompt2image, the argument is untyped but likely considered a string or None. Not sure about constants. You mean there is an enumeration to use? I agree, it's a probably not suitable from your point of view. Well, maybe it's useful to know I'm using it for a while now and there seems to be no problem at runtime. |
Very good samplers by the way, you're missing out |
Would love to have them added, but like @damian0815 said, we'd need you to make the full updates required to support this - Reference #1389 for guidance. Thanks! |
Makes sense. I've read about the Mac issues and I understand Apple user support is a concern. I'm not using a Mac nor the Birch-san fork which makes it hard to evaluate. I'm sure some solution will pop up from the Mac base. It's easy to patch for people inclined so no harm. |
if you scroll up a couple of lines from your changes you'll find they're already implemented
|
As far as i'm aware we already support these samplers on all platforms. |
Tested and works so far for 'k_dpmpp_2s_ancestral' and 'k_dpmpp_2m'. NOT working for 'k_dpm_fast', 'k_dpm_adaptive' out of the box