huggingface / huggingface/diffusers

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

Aperta
#14,353 5 commenti 0 reazioni 0 assegnatari Vedi su GitHub
Lingua principale
Python
Stelle
34.5k
Fork
7.3k
Merge medio
3g 3h
PR unite (30g)
91

Descrizione

### 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.

Guida per i contributori

Apri la guida per i contributori

Direzione di ricerca

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.

Scritto dal modello di indicizzazione a partire dal testo della issue.

Valutazione

Stack tecnologico
python, pytorch
Ambito
machine-learning
Tipo di issue
Bug
Difficoltà
4/5
Tempo stimato
3-5 giorni
Stato di attività
Attiva
Chiarezza
Da chiarire
Idoneità per principianti
38/100

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.