TinyTerra / ComfyUI_tinyterraNodes

A selection of nodes for Stable Diffusion ComfyUI
GNU General Public License v3.0
332 stars 39 forks source link

[Feature] Align Your Steps scheduler #116

Closed sh1ny closed 1 month ago

sh1ny commented 1 month ago

Hi, can AYS be implemented in the tinyKSampler node ? It makes quite the difference in generation:

Screenshot 2024-05-23 132828

sh1ny commented 1 month ago

Here's a fair-er example, where i am comparing ttn (karras ) with inspire + the default custom sampler node , all with AYS:

Screenshot 2024-05-23 135829

TinyTerra commented 1 month ago

I'll take a closer look in the morning.. Not sure at this point if adding it to the current tinyksampler makes sense vs a separate ttN custom ksamp..

Any thoughts there?

Also unsure whether it makes more sense to embed it into the Ksamp vs having a sampler/sigma's input, for easier future proofing for different custom schedulers..

Embedding is perhaps more in line with the rest of the pack though.

sh1ny commented 1 month ago

The way i view it, we should be able to reproduce same images ( given samplers etc. allow it ) with any of the approaches the pack (will) offer. So if you decide to go custom sampler node, it should be able to achieve the exact same result as the current tinyksampler with loader. In other words, you'd have to separate and integrate all the other features.

Not sure how big of a task this is and not sure how worth it is. I would use an advanced sampler, just because i like to have more control and monolithic nodes often hide what's going on underneath. On the other hand i do like to use the monolithic nodes when i want to throw something together real fast, they're a life saver.

In the end, i agree either embedding or optional sampler/sigma input would be more in line with what the pack offers so far.

TinyTerra commented 1 month ago

Majority of the core code is already seperated, and is reused across the pipeksamplers and the tinyksamp.. I think it makes the most sense to have an advanced version of the (base) tinyksamp since that's currently lacking from the non-pipe nodes in the pack.

I'll see what I end up doing re widget vs input.. input would definitely be easier, but widget with dynamic widgets would be nicer maybe..

I'll try to work on it tomorrow if I have time.

TinyTerra commented 1 month ago

Added AYS Scheduler to KSamps Seems like i was overcomplicating it in my mind. After looking at it a bit closer there was a straightforward way to add it to the existing Scheduler widget list.

Let me know if you find any discrepancies, or if you'd still be interested in an 'advanced' version of the tinyKSamp

sh1ny commented 1 month ago

Hey,

That's amazing and it works perfect, i played with it for the past hour or so:

Screenshot 2024-05-24 145102

About the advanced node, while i think personally i will have interest in it, i am unsure how popular that would be vs the effort required.

I have 2 other things in mind that can be added, that i think would be easier and more popular due to their usability, but i will open separate requests for them.

I think this one is nicely done, thank you!