huggingface / huggingface/diffusers

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

Offen
#14,353 5 Kommentare 0 Reaktionen 0 zugewiesene Personen Auf GitHub ansehen
Vorherrschende Sprache
Python
Sterne
34.5k
Forks
7.3k
Ø Merge
3 T. 3 Std.
Gemergte PRs (30 T.)
91

Beschreibung

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

Beitragsleitfaden

Beitragsleitfaden öffnen

Rechercherichtung

Beginnen Sie mit den vier im Issue aufgeführten Scheduler-Dateien und -Methoden und lesen Sie anschließend die relevanten Hinweise in AGENTS.md. Vergleichen Sie jeden dokumentierten Parameter mit seiner tatsächlichen Verwendung und überprüfen Sie die gemeldeten AST-Befunde. Erledigt ist die Aufgabe, wenn die vom Maintainer genehmigte Behandlung – Entfernung, Implementierung oder ausdrückliche Ablehnung – konsistent auf alle betroffenen Methoden angewendet wurde.

Vom Indexierungsmodell aus dem Issue-Text verfasst.

Bewertung

Tech-Stack
python, pytorch
Bereich
machine-learning
Issue-Typ
Bug
Schwierigkeit
4/5
Geschätzter Aufwand
3-5 Tage
Aktivitätsstatus
Aktiv
Klarheit
Muss geklärt werden
Anfängerfreundlichkeit
38/100

Neue Issues direkt in Ihr Postfach

Eine kurze Übersicht über anfängerfreundliche GitHub-Issues.