search

comfyanonymous / ComfyUI

The most powerful and modular diffusion model GUI, api and backend with a graph/nodes interface.
https://www.comfy.org/
GNU General Public License v3.0
51.34k stars 5.4k forks source link

Please combine the `BasicScheduler` and `AlignYourStepsScheduler` nodes #3590

Open Bocian-1 opened 3 months ago

Bocian-1 commented 3 months ago

I really see no point in keeping them separate other than AYS not having a model input, but can't that just be ignored, when one of AYS schedulers is selected? I would like to make a group node for sampling with some additional functionality and the SamplerCustom family of nodes has been very useful to that end, however with the BasicScheduler not having access to AYS I have to keep two versions of the group node and keep reconnecting everything every time I want to switch to/away from AYS.

JorgeR81 commented 3 months ago

I vote for this !

The KSamplers from Impact Pack already integrate the AYS scheduler. But we can't use certain nodes like the KSampler Config (rgthree) because the COMBO list it's different ( they won't connect ).

pipe

ltdrdata commented 3 months ago

I vote for this !

The KSamplers from Impact Pack already integrate the AYS scheduler. But we can't use certain nodes like the KSampler Config (rgthree) because the COMBO list it's different ( they won't connect ).

image Impact Pack provides adapter node.

mcmonkey4eva commented 3 months ago

SwarmKSampler integrates it directly in the sampler list as well

Bocian-1 commented 3 months ago

I see this is now implemented, thank you

JorgeR81 commented 3 months ago

I see this is now implemented, thank you

Was this also implemented on Comfy UI ?

I don't see it ...

ks

Bocian-1 commented 3 months ago

I see this is now implemented, thank you

Was this also implemented on Comfy UI ?

I don't see it ...

Have you updated ComfyUI?

I can see them: image Is it possible that an extension I have added them there even though it's a base ComfyUI node? When I tried to run it now, it crashed too: cannot access local variable 'sigmas' where it is not associated with a value

JorgeR81 commented 3 months ago

No, I don't have it. I updated today, before posting.

I even have the new ipndm samplers. But the scheduler list is the same as before. 

ays

Bocian-1 commented 3 months ago

Hmm... I closed this thinking it was added to the base ComfyUI as I can see it even on base Comfy nodes now, but I guess it must've been added by some extension then. It does solve my use case, or will anyway when it stops crashing, but I'll reopen this, since I believe getting this in base Comfy is worthwhile

ltdrdata commented 3 months ago

Hmm... I closed this thinking it was added to the base ComfyUI as I can see it even on base Comfy nodes now, but I guess it must've been added by some extension then. It does solve my use case, or will anyway when it stops crashing, but I'll reopen this, since I believe getting this in base Comfy is worthwhile

That was bug of Efficiency Nodes. They are gone after patched.

JorgeR81 commented 2 months ago

This custom node suite now adds the AYS schedulers directly to the KSampler node. https://github.com/pamparamm/ComfyUI-ppm?tab=readme-ov-file#schedulers

I didn't try it, but it seems to be working. https://www.youtube.com/watch?v=JgYzCEzHDrc ( Nerdy Rodent initially thought it was a Comfy UI default update, but then corrected in the comments ).