-
Notifications
You must be signed in to change notification settings - Fork 5
Add ancestral sampling and advanced node #7
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
Conversation
|
Absolutey amazing!!!! Thank you!!!! I had given up after messing up the scaling but I feel that it could help with selecting better values. |
Not a problem! You're one of the few people doing fun experimental stuff and I've had fun playing with your toys. Glad to give something back.
Do you mean getting ancestral stuff to work at all or something else? |
Oh :D
I don't remember much really. It's was about the scaling and how it worked. I'll admit I barely gave it a try, saw I was getting noise as an output and went back into the sampler's logic itself. |
|
If I wanted to add it as ancestral in the preset list which setting would you recommand? |
Ah, I see. It can be sort of complicated for flow models, so maybe that was the issue. In any case, it works now (I hope!) so not really something you need to worry about anymore.
Most ancestral samplers just use eta=1.0, so you could do that. Or eta=0.5 for a milder ancestral effect. Ancestralness on the distance steps ( |
|
(forgot to say thanks ^^) |
|
I was trying around and I wonder about something. Since you mentionned not having good results with non-flow models for the internal steps I got to wonder if maybe this line (at line 163): Did not take into account that the first internal step is euler and so should be readjusted regarding this? I could widely be wrong since I'm now getting only the half of it but I figured I would ask rather than trying without really understanding. Because if I change it to: I can set all ancestral-related settings to 1 and get an image while without the modification I get a confetti cannon. What is your opinion? |
|
Sorry about the slow reply!
Ahh, I see I phrased that part poorly and even left out a word. What I meant was that it can have weird results and requires some tweaking. I don't think it's necessarily bad, though 1.0 ETA on those steps may generally be too much.
To be honest, I don't really understand it either. :) You almost certainly understand the math behind sampling/noise addition better than I do, I'm just decent at implementing this sort of stuff from an existing reference. It makes sense that noise addition on internal distance steps would have a strong effect since intuitively it seems like this would be compounding the effect of ETA. One way to deal with that would be just to set a low ETA manually (the approach I used). Scaling it based on the internal steps seems like something that should work also.
Using that approach, do you still see a noticeable difference between using ETA for the internal steps vs just leaving it at 0? If so, then sounds like a good idea! Like I said though, your judgement for this stuff is probably better than mine so I'd be inclined to just defer to whatever approach you think works best. |
But actually no 😅🤣 The idea which made me tempted to try an ancestral version initially was to change seed in the internal steps to get some wiggling in the prediction and make more use of the distance weights. Currently I'm trying this but I'm never sure about the dosage of noise. So rather than shortening it (ETA at 1 currently gives blurry results with my modification) wrongly I figured it would be better to ask you :) |
This pull adds ancestral sampling support as well as an advanced node that exposes the various settings (and allows setting ETA to enable ancestral sampling). The ancestral sampling side works for both flow and non-flow models and was tested on SD 1.5, Wan, Flux, and Cosmos Predict2. These changes should not affect results when ancestral sampling isn't enabled.
I tried to keep these changes minimal and use your code style when possible but a little refactoring was necessary. I also expanded your
first_onlyoption to allow setting the start/end step ranges for distance sampling.There's an experimental option to use ancestralness in the internal distance steps as well. Interesting, it works pretty well with flow models (doesn't seem as good with non-flow). Example with Flux, no ancestral sampling:
vs with these parameters (previous generation was the same, just with both ETA parameters set to 0):
Hopefully this pull is along the lines of something you'd be interested in merging. If you need anything changed, please let me know!