huggingface / huggingface/diffusers
Modular: `required=True` is silently ignored when an `InputParam` carries a default
- Dominant language
- Python
- Stars
- 34.5k
- Forks
- 7.3k
- Avg merge
- 3d 3h
- Merged PRs (30d)
- 91
Description
> 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
```python
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:
```python
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
```python
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.
Contributor guide
Assessment
This issue has not been assessed yet.