Skip to content

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

Description

@4ktLuffy

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:
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 thems_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.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions