huggingface / huggingface/diffusers

stable_diffusion_3 model/pipeline review

Offen
#13,611 0 Kommentare 0 Reaktionen 0 zugewiesene Personen Auf GitHub ansehen
Vorherrschende Sprache
Python
Sterne
34.5k
Forks
7.3k
Ø Merge
3 T. 3 Std.
Gemergte PRs (30 T.)
91

Beschreibung

# `stable_diffusion_3` model/pipeline review

Commit tested: `0f1abc4ae8b0eb2a3b40e82a310507281144c423`

Review performed against the repository review rules.

## Issue 1: SD3 inpaint decode drops the VAE shift

Affected code:
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/stable_diffusion_3/pipeline_stable_diffusion_3_inpaint.py#L1364-L1367

Problem:
`StableDiffusion3InpaintPipeline` encodes VAE latents with `(latents - shift_factor) * scaling_factor`, but decodes with only `latents / scaling_factor`. The missing `+ self.vae.config.shift_factor` makes inpaint decoding inconsistent with the other SD3 pipelines and with its own encode path. Duplicate search found no matching issue/PR.

Impact:
User-visible inpaint outputs are decoded from the wrong latent distribution whenever the VAE has a nonzero `shift_factor`, which SD3 VAEs do.

Reproduction:
```python
import torch
from diffusers import AutoencoderKL

vae = AutoencoderKL(
sample_size=8, in_channels=3, out_channels=3, block_out_channels=(4,),
layers_per_block=1, latent_channels=16, norm_num_groups=1,
use_quant_conv=False, use_post_quant_conv=False,
shift_factor=0.25, scaling_factor=2.0,
)
latents = torch.randn(1, 16, 8, 8)
current = vae.decode(latents / vae.config.scaling_factor, return_dict=False)[0]
expected = vae.decode((latents / vae.config.scaling_factor) + vae.config.shift_factor, return_dict=False)[0]
print((current - expected).abs().max())
```

Relevant precedent:
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/stable_diffusion_3/pipeline_stable_diffusion_3.py#L1132-L1136
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/stable_diffusion_3/pipeline_stable_diffusion_3_img2img.py#L1146-L1150

Suggested fix:
```python
latents = (latents / self.vae.config.scaling_factor) + self.vae.config.shift_factor
image = self.vae.decode(latents, return_dict=False, generator=generator)[0]
```

## Issue 2: SD3 ControlNet pipelines cannot use dynamic-shifting schedulers

Affected code:
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/controlnet_sd3/pipeline_stable_diffusion_3_controlnet.py#L1101-L1108
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/controlnet_sd3/pipeline_stable_diffusion_3_controlnet_inpainting.py#L1272-L1277

Problem:
The base SD3 pipelines compute and pass `mu` when `FlowMatchEulerDiscreteScheduler.config.use_dynamic_shifting=True`. The ControlNet SD3 pipelines call `retrieve_timesteps()` without `mu` handling and expose no `mu` argument. Duplicate search found no matching issue/PR.

Impact:
SD3.5-style scheduler configs with dynamic shifting fail before inference, so ControlNet is inconsistent with the rest of the SD3 family.

Reproduction:
```python
from diffusers import FlowMatchEulerDiscreteScheduler
from diffusers.pipelines.controlnet_sd3.pipeline_stable_diffusion_3_controlnet import retrieve_timesteps

scheduler = FlowMatchEulerDiscreteScheduler(use_dynamic_shifting=True)
retrieve_timesteps(scheduler, num_inference_steps=2, device="cpu")
```

Relevant precedent:
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/stable_diffusion_3/pipeline_stable_diffusion_3.py#L1013-L1038
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/stable_diffusion_3/pipeline_stable_diffusion_3_inpaint.py#L1156-L1177

Suggested fix:
Add the same `mu` argument, `calculate_shift()` logic, and `scheduler_kwargs["mu"]` handling used by the base SD3 pipelines before calling `retrieve_timesteps()`.

## Issue 3: Duplicate: `controlnet_pooled_projections` tensor path is broken

Affected code:
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/controlnet_sd3/pipeline_stable_diffusion_3_controlnet.py#L1134-L1138
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/controlnet_sd3/pipeline_stable_diffusion_3_controlnet_inpainting.py#L1267-L1270

Problem:
This is already reported in open issue https://github.com/huggingface/diffusers/issues/9686. When a user passes a tensor for `controlnet_pooled_projections`, the code evaluates it with Python `or`, which raises `RuntimeError: Boolean value of Tensor with more than one value is ambiguous`.

Impact:
The public `controlnet_pooled_projections` argument cannot be used reliably, and SD3 ControlNet inference/training validation can diverge from the intended pooled-projection conditioning path.

Reproduction:
```python
import torch

controlnet_pooled_projections = torch.ones(1, 8)
pooled_prompt_embeds = torch.zeros(1, 8)
controlnet_pooled_projections = controlnet_pooled_projections or pooled_prompt_embeds
```

Relevant precedent:
Existing duplicate: https://github.com/huggingface/diffusers/issues/9686

Suggested fix:
```python
if controlnet_config.force_zeros_for_pooled_projection:
controlnet_pooled_projections = torch.zeros_like(pooled_prompt_embeds)
elif controlnet_pooled_projections is None:
controlnet_pooled_projections = pooled_prompt_embeds
```

## Issue 4: `SD3ControlNetModel.from_transformer()` mutates the source transformer config

Affected code:
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/models/controlnets/controlnet_sd3.py#L253-L260

Problem:
`config = transformer.config` aliases the transformer's live config, then writes ControlNet-specific values into it. Duplicate search found no matching issue/PR.

Impact:
Calling `from_transformer()` silently changes `transformer.config.num_layers` and adds `extra_conditioning_channels`, which can corrupt later serialization, logging, or pipeline construction using the original transformer.

Reproduction:
```python
from diffusers import SD3ControlNetModel, SD3Transformer2DModel

transformer = SD3Transformer2DModel(
sample_size=4, patch_size=1, in_channels=4, out_channels=4, num_layers=3,
attention_head_dim=4, num_attention_heads=2, caption_projection_dim=8,
joint_attention_dim=8, pooled_projection_dim=8,
)
print(dict(transformer.config).get("num_layers"))
SD3ControlNetModel.from_transformer(transformer, num_layers=1, num_extra_conditioning_channels=2, load_weights_from_transformer=False)
print(dict(transformer.config).get("num_layers"), dict(transformer.config).get("extra_conditioning_channels"))
```

Relevant precedent:
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/models/controlnets/controlnet_flux.py#L135-L141
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/models/controlnets/controlnet_qwenimage.py#L113-L119

Suggested fix:
```python
config = dict(transformer.config)
config["num_layers"] = num_layers or transformer.config.num_layers
config["extra_conditioning_channels"] = num_extra_conditioning_channels
controlnet = cls.from_config(config)
```

## Issue 5: ControlNet inpaint rejects documented IP-Adapter image embeds

Affected code:
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/controlnet_sd3/pipeline_stable_diffusion_3_controlnet_inpainting.py#L780-L787
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/controlnet_sd3/pipeline_stable_diffusion_3_controlnet_inpainting.py#L928-L969
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/controlnet_sd3/pipeline_stable_diffusion_3_controlnet_inpainting.py#L1111-L1114

Problem:
The docstring and `prepare_ip_adapter_image_embeds()` path expect `ip_adapter_image_embeds` to be a tensor, but `check_inputs()` rejects tensors and requires a list. Duplicate search found no matching issue/PR.

Impact:
Users cannot pass precomputed IP-Adapter image embeddings to `StableDiffusion3ControlNetInpaintingPipeline` even though the public signature documents that path.

Reproduction:
```python
import torch
from diffusers import StableDiffusion3ControlNetInpaintingPipeline

pipe = object.__new__(StableDiffusion3ControlNetInpaintingPipeline)
pipe.vae_scale_factor = 1
pipe.patch_size = 1
pipe._callback_tensor_inputs = ["latents"]
pipe.controlnet = object()
pipe.check_inputs(
height=8, width=8, image=torch.zeros(1, 3, 8, 8),
prompt=None, prompt_2=None, prompt_3=None,
prompt_embeds=torch.zeros(1, 2, 8),
pooled_prompt_embeds=torch.zeros(1, 8),
ip_adapter_image_embeds=torch.zeros(1, 2, 8),
control_guidance_start=[0.0], control_guidance_end=[1.0],
)
```

Relevant precedent:
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/stable_diffusion_3/pipeline_stable_diffusion_3.py#L742-L760

Suggested fix:
```python
if ip_adapter_image_embeds is not None and ip_adapter_image_embeds.ndim not in [3, 4]:
raise ValueError(
f"`ip_adapter_image_embeds` has to be a 3D or 4D tensor but is {ip_adapter_image_embeds.ndim}D"
)
```

## Issue 6: Slow tests are missing for SD3 inpaint and SD3 ControlNet inpaint

Affected code:
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/tests/pipelines/stable_diffusion_3/test_pipeline_stable_diffusion_3_inpaint.py#L38-L163
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/tests/pipelines/controlnet_sd3/test_controlnet_inpaint_sd3.py#L45-L208

Problem:
Both files only define fast tests. The target family has slow coverage for SD3 text-to-image, SD3 img2img, and SD3 ControlNet, but not for the two inpaint variants. Duplicate search found no matching issue/PR.

Impact:
Real checkpoint behavior, offload behavior, VAE shift handling, and image/mask preprocessing are not covered for the inpaint variants. This gap would have allowed Issue 1 to remain invisible in CI.

Reproduction:
```python
from pathlib import Path

for path in [
Path("tests/pipelines/stable_diffusion_3/test_pipeline_stable_diffusion_3_inpaint.py"),
Path("tests/pipelines/controlnet_sd3/test_controlnet_inpaint_sd3.py"),
]:
text = path.read_text()
print(path, "@slow" in text, "SlowTests" in text)
```

Relevant precedent:
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/tests/pipelines/stable_diffusion_3/test_pipeline_stable_diffusion_3.py#L227-L235
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/tests/pipelines/stable_diffusion_3/test_pipeline_stable_diffusion_3_img2img.py#L162-L166
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/tests/pipelines/controlnet_sd3/test_controlnet_sd3.py#L228-L245

Suggested fix:
Add `@slow` / `@require_big_accelerator` classes for `StableDiffusion3InpaintPipeline` and `StableDiffusion3ControlNetInpaintingPipeline` using real SD3-family checkpoints, CPU/GPU offload, and deterministic output-slice assertions.

Beitragsleitfaden

Beitragsleitfaden öffnen

Rechercherichtung

Start with the affected SD3 pipeline files under src/diffusers/pipelines/stable_diffusion_3 and src/diffusers/pipelines/controlnet_sd3, plus src/diffusers/models/controlnets/controlnet_sd3.py, and run the supplied reproductions. Review the existing fast tests in tests/pipelines/stable_diffusion_3/test_pipeline_stable_diffusion_3_inpaint.py and tests/pipelines/controlnet_sd3/test_controlnet_inpaint_sd3.py alongside the cited base-pipeline implementations. Done means the reported behaviors are corrected or covered without regressions, including slow inpaint coverage.

Vom Indexierungsmodell aus dem Issue-Text verfasst.

Bewertung

Tech-Stack
python, pytorch
Bereich
machine-learning, testing-qa
Issue-Typ
Bug
Schwierigkeit
4/5
Geschätzter Aufwand
3-5 Tage
Aktivitätsstatus
Ruhig
Klarheit
Größtenteils klar
Anfängerfreundlichkeit
45/100

Neue Issues direkt in Ihr Postfach

Eine kurze Übersicht über anfängerfreundliche GitHub-Issues.