Skip to content

Reject an empty custom timestep list with a clear ValueError - #25

Open
alex-16moro wants to merge 2 commits into
ramp-demofrom
ramp/empty-timesteps-demo
Open

alex-16moro wants to merge 2 commits into
ramp-demofrom
ramp/empty-timesteps-demo

Conversation

@alex-16moro

Copy link
Copy Markdown
Owner

Why

DDPMScheduler.set_timesteps(timesteps=[]) currently fails with IndexError: list index out of range when it reads timesteps[0], which gives the caller no hint about what went wrong.

What changed

  • DDPMScheduler.set_timesteps raises ValueError ("timesteps cannot be empty.") for an empty custom list. The check runs after the existing mutual-exclusion check and before timesteps[0] is read.
  • DDPMParallelScheduler.set_timesteps is a # Copied from copy and was updated with utils/check_copies.py --fix_and_overwrite.
  • Both docstrings now state that the list must be non-empty.
  • Valid lists, single-element lists and num_inference_steps schedules behave as before.

Considered and rejected: substituting a default such as [999]. The library's guidance prefers a concise error to silently correcting input, and 999 is only meaningful when num_train_timesteps is 1000.

Validation

Ramp Kit full verification: 12/12 checks PASS (acceptance, scheduler tests, lint, format, upstream quality, copies, dummies, support list, forward docstrings, dependency table, dependencies, test strength). The test-strength replay removed the fix and the new test failed as intended. Patch digest 02c34f9999a89c1131050dd001c3e7a0bf1dbb7b4f18db93b0b4bf62108ad3da.

Reviewer attention

  • Error wording, and precedence: when both arguments are passed, the existing mutual-exclusion error still wins.
  • The parallel-scheduler test is in tests/schedulers/test_scheduler_ddpm.py because the kit's editable scope excludes the parallel test file; it can move if preferred.
Open in Web聽Open in Cursor聽

Co-authored-by: alex-16moro <alex-16moro@users.noreply.github.com>
Co-authored-by: alex-16moro <alex-16moro@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants