Skip to content

fix: correct double increment on flow denoisers sigma calculations#1372

Open
wbruna wants to merge 1 commit intoleejet:masterfrom
wbruna:sd_fix_flow_sigma
Open

fix: correct double increment on flow denoisers sigma calculations#1372
wbruna wants to merge 1 commit intoleejet:masterfrom
wbruna:sd_fix_flow_sigma

Conversation

@wbruna
Copy link
Copy Markdown
Contributor

@wbruna wbruna commented Mar 28, 2026

t_to_sigma already increments t, so it ends up generating wrong values for sigma_max (1.00033) and sigma_min. This affects schedulers that rely on those values: exponential, karras, bong_tangent and kl_optimal.

bong_tangent 8 steps, before:
1.00033, 0.971839, 0.930154, 0.863995, 0.746076, 0.503155, 0.217895, 0.0752291, 0

after:
1, 0.97143, 0.929634, 0.863298, 0.745065, 0.501497, 0.215478, 0.0724315, 0

t_to_sigma already increments t, so it ends up generating wrong values
for sigma_max (1.00033) and sigma_min. This affects schedulers that
rely on those values: exponential, karras, bong_tangent and kl_optimal.

bong_tangent 8 steps, before:
1.00033, 0.971839, 0.930154, 0.863995, 0.746076, 0.503155, 0.217895, 0.0752291, 0

after:
1, 0.97143, 0.929634, 0.863298, 0.745065, 0.501497, 0.215478, 0.0724315, 0
@leejet
Copy link
Copy Markdown
Owner

leejet commented Mar 31, 2026

I think a better change would be to remove t = t + 1 from t_to_sigma.

@wbruna
Copy link
Copy Markdown
Contributor Author

wbruna commented Mar 31, 2026

I think a better change would be to remove t = t + 1 from t_to_sigma.

That would look more correct, but... schedulers that rely on t_to_sigma seem to be working fine at the moment, even with CompVisDenoiser. And the FluxFlowDenoiser do that increment, too... So I believe the schedulers should be doing that increment, instead? Then CompVisDenoiser will need to decrement it, to be consistent with the current log_sigmas table?

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.

2 participants