huggingface / huggingface/diffusers
stable_diffusion_3 model/pipeline review
- 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
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