huggingface / huggingface/diffusers
Scheduler `step()` methods silently ignore documented sampling parameters (`s_churn`, `s_noise`, `eta`, `generator`, `variance_noise`)
- Dominant language
- Python
- Stars
- 34.5k
- Forks
- 7.3k
- Avg merge
- 3d 3h
- Merged PRs (30d)
- 91
Description
### Describe the bug
Four scheduler `step()` methods accept parameters that are documented in their `Args:` block but never referenced in the function body. Passing them has no effect and no warning is raised. Filing as one systematic issue per the *fix patterns, not one-offs* guideline.
| File | Method | Accepted but never used |
|---|---|---|
| `scheduling_flow_match_euler_discrete.py`:423 | `step()` | `s_churn`, `s_tmin`, `s_tmax`, `s_noise` |
| `scheduling_ddim_cogvideox.py`:326 | `step()` | `eta`, `use_clipped_model_output`, `generator`, `variance_noise` |
| `scheduling_dpm_cogvideox.py`:401 | `step()` | `eta`, `use_clipped_model_output`, `variance_noise` |
| `scheduling_helios.py`:311 | `step_euler()` | `generator` |
### Why this is user-visible
- **`generator` is discarded** in `DDIMSchedulerCogVideoX.step()` and `HeliosScheduler.step_euler()`. A caller passing a seeded `torch.Generator` for reproducibility gets no error and no reproducibility.
- **`eta` is discarded** in both CogVideoX schedulers. `eta` is the DDIM stochasticity control (`0` = deterministic, `1` = DDPM); setting it does nothing.
- **`s_noise` is discarded** in `FlowMatchEulerDiscreteScheduler.step()`, where it is documented as *"Scaling factor for noise added to the sample"*. The `stochastic_sampling` branch calls `randn_tensor(...)` and never applies it:
```python
if self.config.stochastic_sampling:
x0 = sample - current_sigma * model_output
noise = randn_tensor(sample.shape, generator=generator, device=sample.device, dtype=sample.dtype)
prev_sample = (1.0 - next_sigma) * x0 + next_sigma * noise # s_noise never applied
```
`s_churn`, `s_tmin` and `s_tmax` in that same signature also have empty docstring descriptions, which suggests they were copied from `EulerDiscreteScheduler` without being wired up.
### How this was found
An AST scan comparing each function's parameter list against every `ast.Name` referenced in its body, with the signature and docstring excluded so documentation mentions don't count as usage. Verified per-function rather than by grep. Happy to share the script.
### Which fix do you want?
Two defensible directions, and I'd rather not guess:
1. **Remove the dead parameters** — matches the `AGENTS.md` guidance (*"do not carry unused method parameters 'for API consistency'"*), but changes a public signature.
2. **Implement them** — `s_noise` in particular reads as a missing implementation rather than dead weight, since the stochastic branch it belongs to does exist.
A third option would be raising on non-default values instead of ignoring them, per *"raise a concise error for unsupported cases"*.
Happy to open a PR once a maintainer confirms both the scope and which direction you'd prefer.
Contributor guide
Research direction
Start with the four scheduler files and methods listed in the issue, then read the relevant guidance in AGENTS.md. Compare each documented parameter with its actual use and review the reported AST findings. Done means the maintainer-approved treatment—removal, implementation, or explicit rejection—is applied consistently across the affected methods.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python, pytorch
- Domain
- machine-learning
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Needs clarification
- Newbie friendliness
- 38/100