huggingface / huggingface/diffusers

stable_diffusion_3 model/pipeline review

Open
#13,611 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

# `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.

Contributor guide

Open the contributing guide

Research direction

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.

Written by the indexing model from the issue text.

Assessment

Tech stack
python, pytorch
Domain
machine-learning, testing-qa
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.