huggingface / huggingface/diffusers
controlnet model/pipeline review
- Lingua principale
- Python
- Stelle
- 34.5k
- Fork
- 7.3k
- Merge medio
- 3g 3h
- PR unite (30g)
- 91
Descrizione
# `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.
Guida per i contributori
Apri la guida per i contributori
Direzione di ricerca
Inizia leggendo le esportazioni collegate in src/diffusers/__init__.py e src/diffusers/models/__init__.py, quindi esamina i punti di ingresso del modello e della pipeline ControlNet nelle sezioni collegate di controlnet_union.py, multicontrolnet.py, multicontrolnet_union.py, pipeline_controlnet.py e pipeline_controlnet_union_sd_xl.py. Esegui le riproduzioni per ogni caso segnalato e aggiungi una copertura di regressione affinché esportazioni, costruttori, validazione, scale nulle, ordine dei canali, percorsi di salvataggio e valori restituiti corrispondano ai contratti pubblici indicati.
Scritto dal modello di indicizzazione a partire dal testo della issue.
Valutazione
- Stack tecnologico
- python, pytorch
- Ambito
- backend-api-design, machine-learning
- Tipo di issue
- Bug
- Difficoltà
- 5/5
- Tempo stimato
- Più di una settimana
- Stato di attività
- Tranquilla
- Chiarezza
- Abbastanza chiara
- Idoneità per principianti
- 45/100