huggingface / huggingface/diffusers

controlnet model/pipeline review

Open
#13,600 0 comments 0 reactions 0 assignees View on GitHub
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.