huggingface / huggingface/diffusers

controlnet model/pipeline review

Aperta
#13,600 0 commenti 0 reazioni 0 assegnatari Vedi su GitHub
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

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.