Skip to content

Modular: required=True is silently ignored when an InputParam carries a default #14388

Description

@yiyixuxu

Opened by Claude (Opus 5) on behalf of @yiyixuxu.

Describe the bug

In modular pipelines, an InputParam that is both required=True and carries a non-None default is silently not required. _check_inputs substitutes the default before it tests required, so the flag can never fire:

https://github.com/huggingface/diffusers/blob/main/src/diffusers/modular_pipelines/modular_pipeline.py#L512-L524

value = state.get(input_param.name)
if value is None:
    value = input_param.default          # a non-None default lands here
if input_param.required and value is None:
    raise ValueError(f"Required input '{input_param.name}' is missing")

This is easy to hit through InputParam.template, because the template supplies the default and an override only replaces the keys it names:

template_kwargs = INPUT_PARAM_TEMPLATES[template_name].copy()   # {"type_hint": int, "default": 50, ...}
template_kwargs.update(overrides)                               # {"required": True}
return cls(name=name, **template_kwargs)

So InputParam.template("num_inference_steps", required=True) reads like a decision to require the input, and produces an input that defaults to 50 instead. Nobody wrote the 50 — it arrived as a side effect of reaching for the template's type hint and description.

Reproduction

from diffusers.modular_pipelines.modular_pipeline_utils import InputParam

param = InputParam.template("num_inference_steps", required=True)
print(param.required, param.default)   # True 50

End to end, on a block that declares it that way — omitting the input runs at 50 rather than raising:

passed num_inference_steps=2 -> 1 timesteps
omitted                      -> 49 timesteps

Where it occurs

14 call sites on main, across four pipelines, where required=True cannot bite (found by matching InputParam.template(..., required=True) against the templates that carry a default, skipping the sites that pass default=None explicitly):

file line param
modular_pipelines/cosmos/before_denoise.py 953, 1224 num_inference_steps
modular_pipelines/cosmos/denoise.py 462 num_inference_steps
modular_pipelines/krea2/denoise.py 92, 259 num_inference_steps
modular_pipelines/ltx/before_denoise.py 281, 345 batch_size
modular_pipelines/ltx/decoders.py 79 dtype
modular_pipelines/ltx/denoise.py 48, 274 dtype
modular_pipelines/ltx/denoise.py 98, 210, 326 num_inference_steps
modular_pipelines/qwenimage/denoise.py 465 num_inference_steps

hunyuan_video1_5/denoise.py is the only place that noticed, and works around it by writing InputParam.template("num_inference_steps", required=True, default=None).

The templates carrying a default are batch_size, control_guidance_end, control_guidance_start, controlnet_conditioning_scale, dtype, layers, max_sequence_length, num_images_per_prompt, num_inference_steps, output_type and strength.

Why it matters beyond the flag

The contradiction also leaks into what a pipeline advertises. ModularPipeline.default_call_parameters maps every declared input to its default without filtering on required, so these inputs report a default they will not actually honour if the flag is ever made to work — and the same property is what ModularPipelineTesterMixin.optional_params is checked against, which means that assertion currently cannot distinguish an optional input from a required one.

Suggested fix

required=True and a default are contradictory: if an input is required, the default value is irrelevant and should not be there. Two ways to make that hold, both of which need a call:

  1. Reject the combination. Raise in InputParam.__post_init__ when required and default is not None. Every affected site then has to state what it means, and the mistake becomes unmakeable rather than silent.
  2. Make the flag bite. Test required before substituting the default in _check_inputs. Less invasive to write, but it leaves the contradictory declaration in place and merely picks a winner.

Either way the 14 sites above change behaviour — a request that omits one of those inputs starts raising where it used to fall back to the template's value — so each one needs its pipeline's author to say whether the input was meant to be required, or whether the default was meant to be real and required=True should simply go.

Happy to open a PR for whichever direction you prefer.

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions