fix(scheduler): reject step ranges that produce empty timestep slices#2012
Open
laurigates wants to merge 1 commit into
Open
fix(scheduler): reject step ranges that produce empty timestep slices#2012laurigates wants to merge 1 commit into
laurigates wants to merge 1 commit into
Conversation
Previously a WanVideoSchedulerv2 / WanVideoScheduler configured with a
step range that resolves to an empty timestep slice (e.g. steps=4,
start_step=8, end_step=-1) would silently build a scheduler with zero
timesteps. The downstream sampler's main loop then never executes, and
the post-loop return statement references variables (callback_latent,
noise_pred, ...) that are only assigned inside the loop:
UnboundLocalError: cannot access local variable 'callback_latent'
where it is not associated with a value
This is especially confusing in HIGH/LOW two-sampler chains because the
HIGH sampler runs to completion (model load, JIT compile, full step
count) before the LOW sampler's broken scheduler crashes — wasting
significant GPU time on work that was always going to be discarded.
Two layers of validation now catch this:
1. wanvideo/schedulers/__init__.py: get_scheduler() now also rejects
start_step >= steps (the existing check only fired when end_step
was not -1) and end_step > steps. These run for any caller, not
just the public scheduler nodes.
2. nodes_sampler.py: WanVideoScheduler now defines VALIDATE_INPUTS,
which ComfyUI invokes at prompt-queue time before any node in the
graph executes. This rejects the prompt with a precise error before
any model loads or any sampling begins, eliminating the wasted GPU
work entirely. WanVideoSchedulerv2 inherits the check.
Each error message names the offending values (start_step, end_step,
steps) and explains why the range is invalid, so the user can fix the
widget directly without having to reconstruct what the symptom maps to.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
A
WanVideoSchedulerv2(or v1) configured with a step range that resolves to an emptytimestepsslice — e.g.steps=4, start_step=8, end_step=-1, orsteps=8, end_step=12— currently builds a scheduler with zero timesteps. The downstream sampler's main loop (for idx, t in enumerate(timesteps[ttm_start_step:])) never executes, and the post-loopreturnreferences variables that are only assigned inside the loop:(
callback_latentis the most common offender on the v2 sampler return path; v1 hits the same class of error onnoise_pred.)This is especially painful for HIGH/LOW two-sampler chains: the HIGH sampler runs to completion — full model load, JIT compile, full step count — before the LOW sampler's broken scheduler builds and crashes. On a 14B Wan 2.2 model that's a couple of minutes of GPU time burned on work that was always going to be discarded.
A real-world misconfiguration that triggered this:
LOW's
timesteps[8:]on a length-4 sequence is[]. The HIGH branch ran 4 steps, fp8 compiled, then died on the LOW return.Why the existing check misses it
get_scheduler()already validatesstart_step >= end_step, but only whenend_step != -1. Theend_step=-1("run to the end") path has no upper bound onstart_step, so anything>= len(timesteps)slips through.Fix
Two layers, so the validation fires regardless of how the scheduler was constructed:
1.
get_scheduler()— defensive check for any callerAdds:
start_step >= steps→ rejectend_step > steps(whenend_step != -1) → rejectstart_step >= end_stepcheck kept and given a more informative message.This protects callers that build a scheduler outside the public
WanVideoScheduler[v2]node, and it covers the float-sigma-threshold path that the original check already handled.2.
VALIDATE_INPUTSonWanVideoScheduler— rejects at prompt-queue timeVALIDATE_INPUTSis ComfyUI's pre-execution hook: it runs before any node in the graph is invoked. So a misconfigured scheduler stops the whole prompt before HIGH loads the model, with a red error in the UI pointing directly at the offending node. No GPU time is consumed at all.WanVideoSchedulerv2inherits fromWanVideoSchedulerand picks up the check automatically.Error-message philosophy
Each failure message names all three values (
start_step,end_step,steps) and explains the consequence. The original"start_step must be less than end_step"wasn't enough to map back to "I typo'd 8 for 4 in the LOW branch's start_step." The new messages let the user fix the widget without first having to reverse-engineer the symptom.What about auto-fixing?
Considered and rejected.
start=8, end=-1onsteps=4is ambiguous: the user might have meantsteps=8total orstart=2for a 4-step split — no way to tell. Refusing with a precise message is the right move; silent clamping would hide misconfigurations behind plausibly-shaped but unintended output.Test plan
python3 -c "import ast; ast.parse(...)"clean on both edited files.callback_latentUnboundLocalError onmainwithWanVideoSchedulerv2(steps=4, start_step=8, end_step=-1)."start_step (8) must be < steps (4); the requested range would produce an empty timestep slice and the sampler would crash mid-graph."steps=8, start_step=4, end_step=-1;steps=12, start_step=7, end_step=10; etc.) still validate and run.float) is unchanged — VALIDATE_INPUTS only sees the int widget values;get_scheduler()keeps the existing float branch.Related
This addresses the same class of "fail loudly, fail fast" gap that #2011 (
use_zero_initearly-exit returning a 2-tuple where the caller unpacks 3) targets — both bugs would have produced clearer symptoms with a guard one layer up. Happy to fold both into a single PR if that's preferred.