huggingface / huggingface/diffusers

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

Open
#14,388 6 comments 0 reactions 1 assignee Claimed by @yiyixuxu View on GitHub
bug modular-pipelines needs-env-info
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

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.