huggingface / huggingface/diffusers

Scheduler `step()` methods silently ignore documented sampling parameters (`s_churn`, `s_noise`, `eta`, `generator`, `variance_noise`)

Open
#14,353 5 comments 0 reactions 0 assignees View on GitHub
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.