huggingface / huggingface/diffusers
controlnet model/pipeline review
- Dominant language
- Python
- Stars
- 34.5k
- Forks
- 7.3k
- Avg merge
- 3d 3h
- Merged PRs (30d)
- 91
Description
# `controlnet` model/pipeline review
Commit tested: `0f1abc4ae8b0eb2a3b40e82a310507281144c423`
Review performed against the repository review rules.
## Issue 1: `MultiControlNetUnionModel` is missing from top-level exports
Affected code:
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/__init__.py#L229-L269
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/models/__init__.py#L75-L79
Problem:
`MultiControlNetUnionModel` is exported from `diffusers.models`, but not from `diffusers`, while adjacent public ControlNet classes are top-level exports.
Impact:
Users cannot follow the standard `from diffusers import ...` pattern for this public wrapper.
Reproduction:
```python
from diffusers import ControlNetUnionModel, MultiControlNetModel
print(ControlNetUnionModel, MultiControlNetModel)
from diffusers import MultiControlNetUnionModel
```
Relevant precedent:
`MultiControlNetModel` is top-level exported.
Suggested fix:
```python
# src/diffusers/__init__.py
# Add "MultiControlNetUnionModel" next to "MultiControlNetModel"
# in both the lazy _import_structure["models"] list and TYPE_CHECKING imports.
```
## Issue 2: `ControlNetUnionModel()` default constructor crashes
Affected code:
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/models/controlnets/controlnet_union.py#L182
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/models/controlnets/controlnet_union.py#L307-L308
Problem:
`addition_time_embed_dim` defaults to `None`, but the constructor always uses it to create `Timesteps` and `TimestepEmbedding`.
Impact:
A documented public model constructor fails before forward or serialization can be tested.
Reproduction:
```python
from diffusers import ControlNetUnionModel
ControlNetUnionModel(
in_channels=4,
conditioning_channels=3,
down_block_types=("DownBlock2D",),
block_out_channels=(8,),
layers_per_block=1,
norm_num_groups=4,
cross_attention_dim=16,
attention_head_dim=1,
num_trans_channel=8,
num_trans_head=1,
num_proj_channel=8,
conditioning_embedding_out_channels=(4, 8),
)
```
Relevant precedent:
`ControlNetModel` only constructs `add_time_proj` when the matching addition embedding mode requires it.
Suggested fix:
```python
if addition_time_embed_dim is None:
raise ValueError("`addition_time_embed_dim` must be set for `ControlNetUnionModel`.")
```
## Issue 3: Union pipelines advertise `control_mode=None` but crash
Affected code:
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/controlnet/pipeline_controlnet_union_sd_xl.py#L1007
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/controlnet/pipeline_controlnet_union_sd_xl.py#L1184-L1189
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/controlnet/pipeline_controlnet_union_sd_xl.py#L793-L805
Problem:
The Union pipelines default `control_mode` to `None`, wrap it into `[None]`, then compare `None >= num_control_type`.
Impact:
Calling a Union pipeline without explicitly passing `control_mode` fails with a `TypeError`.
Reproduction:
```python
from types import MethodType
from diffusers import ControlNetUnionModel, StableDiffusionXLControlNetUnionPipeline
pipe = object.__new__(StableDiffusionXLControlNetUnionPipeline)
pipe._callback_tensor_inputs = []
pipe.check_image = MethodType(lambda self, image, prompt, prompt_embeds: None, pipe)
pipe.controlnet = ControlNetUnionModel(
in_channels=4,
conditioning_channels=3,
down_block_types=("DownBlock2D",),
block_out_channels=(8,),
layers_per_block=1,
norm_num_groups=4,
cross_attention_dim=16,
attention_head_dim=1,
addition_time_embed_dim=8,
num_trans_channel=8,
num_trans_head=1,
num_proj_channel=8,
conditioning_embedding_out_channels=(4, 8),
)
control_mode = None
if not isinstance(control_mode, list):
control_mode = [control_mode]
pipe.check_inputs(
prompt="a prompt",
prompt_2=None,
image=[object()],
control_guidance_start=[0.0],
control_guidance_end=[1.0],
control_mode=control_mode,
callback_on_step_end_tensor_inputs=[],
)
```
Relevant precedent:
Merged PR https://github.com/huggingface/diffusers/pull/10747 added multi-union handling but did not make `None` a valid default.
Suggested fix:
```python
if control_mode is None:
control_mode = 0
if not isinstance(control_mode, list):
control_mode = [control_mode]
```
## Issue 4: Multi-ControlNet scale length validation is unreachable or missing
Affected code:
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/controlnet/pipeline_controlnet.py#L701-L712
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/models/controlnets/multicontrolnet.py#L47-L63
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/models/controlnets/controlnet_union.py#L691
Problem:
The `elif isinstance(controlnet_conditioning_scale, list)` length check is unreachable after the preceding `if isinstance(..., list)`. In Union variants, some multi-condition paths have no equivalent scale length check.
Impact:
A too-short scale list passes validation, and later `zip(...)` silently drops later ControlNets or later Union conditions.
Reproduction:
```python
from types import MethodType
import torch
from diffusers import MultiControlNetModel, StableDiffusionControlNetPipeline
class DummyControlNet(torch.nn.Module):
pass
pipe = object.__new__(StableDiffusionControlNetPipeline)
pipe._callback_tensor_inputs = []
pipe.controlnet = MultiControlNetModel([DummyControlNet(), DummyControlNet()])
pipe.check_image = MethodType(lambda self, image, prompt, prompt_embeds: None, pipe)
pipe.check_inputs(
prompt="a prompt",
image=[object(), object()],
callback_steps=None,
callback_on_step_end_tensor_inputs=[],
controlnet_conditioning_scale=[0.5],
control_guidance_start=[0.0, 0.0],
control_guidance_end=[1.0, 1.0],
)
print("No error, but one scale for two ControlNets should be rejected.")
```
Relevant precedent:
Issue https://github.com/huggingface/diffusers/issues/11828 is related to Union scale/list acceptance, but not this silent truncation.
Suggested fix:
```python
if isinstance(controlnet_conditioning_scale, list):
if any(isinstance(i, list) for i in controlnet_conditioning_scale):
raise ValueError("Batched varying conditioning scales are not supported.")
if len(controlnet_conditioning_scale) != len(self.controlnet.nets):
raise ValueError("Scale list length must match the number of ControlNets.")
```
## Issue 5: All-zero `MultiControlNetUnionModel` scales return `None`
Affected code:
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/models/controlnets/multicontrolnet_union.py#L52-L83
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/controlnet/pipeline_controlnet_union_sd_xl.py#L1518-L1522
Problem:
`MultiControlNetUnionModel.forward()` skips every ControlNet whose scale is `0.0`. If all scales are zero, it returns `(None, None)`. Union pipelines then iterate over `down_block_res_samples` in guess-mode CFG.
Impact:
Valid schedules such as `control_guidance_start/end` outside a step, or explicit `controlnet_conditioning_scale=[0.0]`, can crash in guess mode.
Reproduction:
```python
import torch
from diffusers.models import MultiControlNetUnionModel
class DummyUnion(torch.nn.Module):
config = type("Config", (), {"num_control_type": 6})()
def forward(self, *args, **kwargs):
return [torch.ones(1, 1, 1, 1)], torch.ones(1, 1, 1, 1)
multi = MultiControlNetUnionModel([DummyUnion()])
down, mid = multi(
sample=torch.zeros(1, 4, 8, 8),
timestep=0,
encoder_hidden_states=torch.zeros(1, 1, 4),
controlnet_cond=[torch.zeros(1, 3, 8, 8)],
control_type=[torch.zeros(1, 6)],
control_type_idx=[[0]],
conditioning_scale=[0.0],
return_dict=False,
)
[torch.cat([torch.zeros_like(d), d]) for d in down]
```
Relevant precedent:
`MultiControlNetModel` does not skip zero scales; it lets the child model return zeroed residual tensors.
Suggested fix:
```python
# Do not skip zero scales. Let the child ControlNet return correctly shaped zero residuals.
# Remove:
if scale == 0.0:
continue
```
## Issue 6: `ControlNetUnionModel` rejects documented `bgr` channel order
Affected code:
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/models/controlnets/controlnet_union.py#L143-L145
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/models/controlnets/controlnet_union.py#L608-L611
Problem:
The config/docstring exposes `controlnet_conditioning_channel_order`, but Union forward only accepts `"rgb"` and raises for `"bgr"`.
Impact:
Union behaves inconsistently with `ControlNetModel` and rejects a documented compatibility mode.
Reproduction:
```python
import torch
from diffusers import ControlNetUnionModel
model = ControlNetUnionModel(
in_channels=4,
conditioning_channels=3,
down_block_types=("DownBlock2D",),
block_out_channels=(8,),
layers_per_block=1,
norm_num_groups=4,
cross_attention_dim=16,
attention_head_dim=1,
addition_time_embed_dim=8,
num_trans_channel=8,
num_trans_head=1,
num_proj_channel=8,
conditioning_embedding_out_channels=(4, 8),
controlnet_conditioning_channel_order="bgr",
)
model(
sample=torch.randn(1, 4, 8, 8),
timestep=0,
encoder_hidden_states=torch.randn(1, 2, 16),
controlnet_cond=[torch.randn(1, 3, 8, 8)],
control_type=torch.zeros(1, 6),
control_type_idx=[0],
return_dict=False,
)
```
Relevant precedent:
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/models/controlnets/controlnet.py#L654-L664
Suggested fix:
```python
if channel_order == "rgb":
pass
elif channel_order == "bgr":
controlnet_cond = [torch.flip(cond, dims=[1]) for cond in controlnet_cond]
else:
raise ValueError(f"unknown `controlnet_conditioning_channel_order`: {channel_order}")
```
## Issue 7: Multi-ControlNet wrappers diverge from public API contracts
Affected code:
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/models/controlnets/multicontrolnet.py#L37-L73
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/models/controlnets/multicontrolnet.py#L75-L176
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/models/controlnets/multicontrolnet_union.py#L47-L83
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/models/controlnets/multicontrolnet_union.py#L86-L192
Problem:
The wrappers accept `os.PathLike` but concatenate paths with strings. They also accept `return_dict=True` but always return tuples.
Impact:
`Path` users get `TypeError`, and direct model callers do not get the advertised `ControlNetOutput`.
Reproduction:
```python
from pathlib import Path
from tempfile import TemporaryDirectory
import torch
from diffusers import MultiControlNetModel
from diffusers.models import MultiControlNetUnionModel
class DummyControlNet(torch.nn.Module):
def save_pretrained(self, save_directory, **kwargs):
print(save_directory)
for cls in (MultiControlNetModel, MultiControlNetUnionModel):
with TemporaryDirectory() as tmp:
try:
cls([DummyControlNet()]).save_pretrained(Path(tmp) / "controlnet")
except Exception as e:
print(cls.__name__, type(e).__name__, e)
```
Relevant precedent:
Related closed issue for multi-control save layout: https://github.com/huggingface/diffusers/issues/7814
Suggested fix:
```python
save_directory = os.fspath(save_directory)
...
model_path_to_load = os.fspath(pretrained_model_path)
...
if not return_dict:
return down_block_res_samples, mid_block_res_sample
return ControlNetOutput(
down_block_res_samples=down_block_res_samples,
mid_block_res_sample=mid_block_res_sample,
)
```
## Issue 8: Test coverage is missing for several target files
Affected code:
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/controlnet/pipeline_controlnet_union_sd_xl.py#L175
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/controlnet/pipeline_controlnet_blip_diffusion.py#L85
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/controlnet/pipeline_flax_controlnet.py#L113
Problem:
No tests under `tests/pipelines/controlnet`, `tests/models/controlnets`, or `tests/single_file` reference `ControlNetUnionModel`, `MultiControlNetUnionModel`, the Union pipelines, `BlipDiffusionControlNetPipeline`, `FlaxControlNetModel`, or `FlaxStableDiffusionControlNetPipeline`. Slow tests are also missing for SDXL img2img and SDXL inpaint ControlNet files.
Impact:
The confirmed regressions above are not covered by fast tests, and several public or deprecated target pipelines have no slow coverage.
Reproduction:
```python
from pathlib import Path
roots = [Path("tests/pipelines/controlnet"), Path("tests/models/controlnets"), Path("tests/single_file")]
files = [p for root in roots for p in root.rglob("*.py")]
for label, terms in {
"Union": ["ControlNetUnionModel", "MultiControlNetUnionModel", "StableDiffusionXLControlNetUnion"],
"BLIP": ["BlipDiffusionControlNetPipeline"],
"Flax": ["FlaxControlNetModel", "FlaxStableDiffusionControlNetPipeline"],
}.items():
hits = [str(p) for p in files if any(term in p.read_text(encoding="utf-8") for term in terms)]
print(label, hits or "NO TEST REFERENCES")
for p in sorted(Path("tests/pipelines/controlnet").glob("test_controlnet*.py")):
print(p, "@slow" in p.read_text(encoding="utf-8"))
```
Relevant precedent:
PR https://github.com/huggingface/diffusers/pull/10747 introduced `MultiControlNetUnionModel`; the discussion explicitly called out adding tests, but this checkout has no Union test references.
Suggested fix:
Add fast tests for Union constructor, `control_mode=None`, multi-union zero scales, scale length validation, top-level import, and save/load PathLike. Add slow Union pipeline tests and slow SDXL img2img/inpaint ControlNet tests. For BLIP-Diffusion ControlNet, either add deprecated-pipeline smoke coverage or document why it is intentionally untested.
Duplicate search status: searched GitHub Issues and PRs for `controlnet`, `ControlNetUnionModel`, `MultiControlNetUnionModel`, `control_mode None`, `addition_time_embed_dim`, `controlnet_conditioning_scale`, `bgr`, `PathLike save_pretrained`, BLIP, and Flax. Related items found were #10747, #11828, and #7814, but I did not find exact open duplicates for the issues above.
Contributor guide
Research direction
Start by reading the linked exports in src/diffusers/__init__.py and src/diffusers/models/__init__.py, then inspect the ControlNet model and pipeline entry points in the linked controlnet_union.py, multicontrolnet.py, multicontrolnet_union.py, pipeline_controlnet.py, and pipeline_controlnet_union_sd_xl.py sections. Run the reproductions for each reported case and add regression coverage so exports, constructors, validation, zero scales, channel order, save paths, and return values match the stated public contracts.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python, pytorch
- Domain
- backend-api-design, machine-learning
- Issue type
- Bug
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 45/100