fix: raise a clear error in UnCLIPScheduler.set_timesteps for fewer than 2 steps - #14574
Open
kyo-zzz wants to merge 1 commit into
Open
fix: raise a clear error in UnCLIPScheduler.set_timesteps for fewer than 2 steps#14574kyo-zzz wants to merge 1 commit into
kyo-zzz wants to merge 1 commit into
Conversation
…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].
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. 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 |
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.
Fixes #14586
Summary
UnCLIPScheduler.set_timestepscrashes with a bareZeroDivisionErrorwhennum_inference_steps == 1, and silently returns an empty schedule whennum_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. ## Reproductionpython 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 causesrc/diffusers/schedulers/scheduling_unclip.py, inset_timesteps:python step_ratio = (self.config.num_train_timesteps - 1) / (self.num_inference_steps - 1)Thenum_inference_steps - 1denominator is reached whenever fewer than 2 steps are requested. The docstring doesn't state a minimum, andUnCLIPPipelineforwards 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 approachMiniMaxH3Scheduleralready 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, sonum_inference_steps == 1has no well-defined schedule; defining one would change numerics without a reference, so validation is preferred over a special-case path.num_inference_steps == 2still spans the full schedule ([999, 0]for the default 1000-step config). ## Testtest_set_timesteps_at_least_two_stepsintests/schedulers/test_scheduler_unclip.py:num_inference_steps0 and 1 raiseValueErrormentioning the parameter; 2 produces[999, 0]. The new case fails onmain(noValueErrorraised) and passes with this change. Fulltest_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 existingUnCLIPSchedulerTest/SchedulerCommonTestpattern; tiny and fast. - pitfalls: no timestep-dtype / numerical change (timesteps remainint64). - No# Copied fromneeded (no copied method). - Note (not from this PR):python utils/check_dummies.pyreports a pre-existing bitsandbytes dummy-shim drift onmainunrelated 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 flaggedPNDMSchedulerand the newHeliosSchedulerfor 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.