Skip to content

fix: raise a clear error in UnCLIPScheduler.set_timesteps for fewer than 2 steps - #14574

Open
kyo-zzz wants to merge 1 commit into
huggingface:mainfrom
kyo-zzz:fix/unclip-set-timesteps-min-steps
Open

fix: raise a clear error in UnCLIPScheduler.set_timesteps for fewer than 2 steps#14574
kyo-zzz wants to merge 1 commit into
huggingface:mainfrom
kyo-zzz:fix/unclip-set-timesteps-min-steps

Conversation

@kyo-zzz

@kyo-zzz kyo-zzz commented Aug 23, 2026

Copy link
Copy Markdown

Fixes #14586

Summary UnCLIPScheduler.set_timesteps crashes with a bare ZeroDivisionError when num_inference_steps == 1, and silently returns an empty schedule when num_inference_steps == 0. The step ratio is (num_train_timesteps - 1) / (num_inference_steps - 1) (the karlo-style inclusive-endpoint schedule), so a single step has no interval to interpolate and the formula divides by zero. ## Reproduction python from diffusers import UnCLIPScheduler scheduler = UnCLIPScheduler() scheduler.set_timesteps(1) # ZeroDivisionError: division by zero scheduler.set_timesteps(0) # silently: scheduler.timesteps == tensor([], dtype=torch.int64) ## Root cause src/diffusers/schedulers/scheduling_unclip.py, in set_timesteps: python step_ratio = (self.config.num_train_timesteps - 1) / (self.num_inference_steps - 1) The num_inference_steps - 1 denominator is reached whenever fewer than 2 steps are requested. The docstring doesn't state a minimum, and UnCLIPPipeline forwards the user's value straight through, so a low value surfaces as an opaque crash. ## Fix Validate up front and raise a concise error for the unsupported case (matching the approach MiniMaxH3Scheduler already takes for the same situation, and the repo's code-style rule to "raise a concise error for unsupported cases rather than adding complex fallback logic"). The karlo schedule interpolates the two endpoints of the training range, so num_inference_steps == 1 has no well-defined schedule; defining one would change numerics without a reference, so validation is preferred over a special-case path. num_inference_steps == 2 still spans the full schedule ([999, 0] for the default 1000-step config). ## Test test_set_timesteps_at_least_two_steps in tests/schedulers/test_scheduler_unclip.py: num_inference_steps 0 and 1 raise ValueError mentioning the parameter; 2 produces [999, 0]. The new case fails on main (no ValueError raised) and passes with this change. Full test_scheduler_unclip.py: 29 passed, 2 skipped. ## Self-review Manual pass against .ai/references/review-rules.md + the linked guides: - code_style: the change adds an error for an unsupported input, not a "just-in-case" safety check or a fallback path — consistent with code_style.md. - testing: scheduler-level regression test follows the existing UnCLIPSchedulerTest/SchedulerCommonTest pattern; tiny and fast. - pitfalls: no timestep-dtype / numerical change (timesteps remain int64). - No # Copied from needed (no copied method). - Note (not from this PR): python utils/check_dummies.py reports a pre-existing bitsandbytes dummy-shim drift on main unrelated to this change; not touched here. ## Discovery Found by running a small edge-case sweep (set_timesteps(1) / set_timesteps(2) across the scheduler suite checking finiteness, monotonicity, and length). The same sweep flagged PNDMScheduler and the new HeliosScheduler for separate low-step crashes — happy to file those as follow-ups if useful. ## AI assistance Root-cause analysis, the fix, and the tests were developed with AI assistance (agentic coding session). I reviewed the change against the repo's review rules and ran the tests locally.

…n 2 steps

The karlo-style step ratio divides by num_inference_steps - 1, so
num_inference_steps=1 crashed with a bare ZeroDivisionError and 0 silently
produced an empty schedule. Validate the input up front (same approach as
MiniMaxH3Scheduler) and document the constraint. n=2 still spans the full
training schedule: [999, 0].
@github-actions github-actions Bot added tests schedulers size/S PR with diff < 50 LOC labels Aug 23, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Hi @kyo-zzz, thanks for the PR! It does not appear to link an issue it fixes. If this PR addresses an existing issue, please add a closing keyword (e.g. Fixes #1234) to the PR description so the issue is linked. See the contribution guide for more details. If this PR intentionally does not fix a tracked issue, a maintainer can add the no-issue-needed label to silence this reminder.

Please note that PRs without a linked issue are likely to be automatically closed 10 days after this notice.

Once the PR links an issue (or gets the no-issue-needed label), you can ignore this message — it stays here as a comment, but it no longer applies.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

schedulers size/S PR with diff < 50 LOC tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

UnCLIPScheduler.set_timesteps crashes (ZeroDivisionError) for 1 step and silently returns an empty schedule for 0 steps

1 participant