Skip to content

Conversation

Balladie
Copy link
Contributor

Current implementation of res_multistep_ancestral sampler seems not to work appropriately with RF-based models.

The original RES paper formulates its theory in VE format, and this restores VP/RF-style formulation of RES in which alpha appears, with eta accordingly to match the definition of the one already in dpmpp_2m_sde. Another change is the integrating variable from negative log-sigma to actual negative log-SNR.

This results in small difference in ODE sampling (no changes in VE) like in the other samplers, but makes notable difference in stochastic samplers on both RF and VE models. I've checked some results on FLUX Krea and SDXL and it seems to produce images well on ancestral sampling while keeping similar aesthetics in both models.

Example results in FLUX Krea:

before, res_multistep_ancestral (step=20, eta=1.0) after, res_multistep_ancestral (step=20, eta=1.0)
flux_krea_res_multistep_ancestral_s20_before flux_krea_res_multistep_ancestral_s20_after
before, res_multistep (step=20) after, res_multistep (step=20)
flux_krea_res_multistep_s20_before flux_krea_res_multistep_s20_after

Example results in SDXL:

before, res_multistep_ancestral (step=25, eta=1.0) after, res_multistep_ancestral (step=25, eta=1.0)
sdxl_res_multistep_ancestral_s25_before sdxl_res_multistep_ancestral_s25_after
before, res_multistep (step=25) after, res_multistep (step=25)
sdxl_res_multistep_s25_before sdxl_res_multistep_s25_after

@chaObserv
Copy link
Contributor

The main reason that changes the result of SDXL is the first step, which currently performs an ODE update all the time.

@Balladie
Copy link
Contributor Author

The main reason that changes the result of SDXL is the first step, which currently performs an ODE update all the time.

Thanks for the catch. Restored the first stochastic step to its previous one. Does not feel clean but checked it produces the same on SDXL.

@comfyanonymous
Copy link
Owner

This is good but I think people might complain it breaks their workflows since res_multistep is a popular sampler to use with rf models and this changes the output a bit.

@Balladie
Copy link
Contributor Author

This is good but I think people might complain it breaks their workflows since res_multistep is a popular sampler to use with rf models and this changes the output a bit.

ye some also might prefer the old integral, in that case would it be acceptable to make a separate function for ancestral like the current implementations of other samplers

@chaObserv
Copy link
Contributor

Alternatively, dpmpp_2m_sde with heun solver type can be viewed as the logSNR parameterization of RES 2m.
For example, in A1111, RES is implemented via the 2m SDE and named "DPM++ 2m SDE Heun".

@Balladie
Copy link
Contributor Author

Yeah there would be multiple options if we want to keep the old res_multistep as it is, for ex:

  • Make a separate res_multistep_ancestral function
  • Forward it to dpmpp_2m_sde with Heun, and name it like dpmpp_2m_sde_heun

Choice would depend on the preference of naming and confusion between samplers. I think both is fine, not sure which would be preferred though

@comfyanonymous
Copy link
Owner

dpmpp_2m_sde_heun is good if it really is the same thing.

@Balladie
Copy link
Contributor Author

Balladie commented Aug 25, 2025

Thanks for the opinion. I opened a new pull request in #9542 adding dpmpp_2m_sde_heun and checked them produce equal results.

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

Successfully merging this pull request may close these issues.

3 participants