huggingface / huggingface/diffusers
stable_diffusion_xl model/pipeline review
- Lenguaje dominante
- Python
- Estrellas
- 34.5k
- Forks
- 7.3k
- Merge medio
- 3 d 3 h
- PR fusionados (30 d)
- 91
Descripción
# `stable_diffusion_xl` model/pipeline review
Commit tested: `0f1abc4ae8b0eb2a3b40e82a310507281144c423`
Review performed against the repository review rules.
Duplicate search: checked GitHub Issues and PRs for `stable_diffusion_xl`, affected class/function/file names, and each failure mode below. No likely duplicates found.
## Issue 1: Flax SDXL subpackage import lacks a dependency dummy
Affected code:
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/stable_diffusion_xl/__init__.py#L33-L37
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/stable_diffusion_xl/__init__.py#L53-L57
Problem:
When `transformers` is installed but `flax` is not, top-level `from diffusers import FlaxStableDiffusionXLPipeline` returns the expected dummy object, but `from diffusers.pipelines.stable_diffusion_xl import FlaxStableDiffusionXLPipeline` raises `ImportError`. The SDXL subpackage never adds `dummy_flax_and_transformers_objects` to `_dummy_objects`.
Impact:
Public lazy-loading behavior is inconsistent and users importing from the pipeline subpackage get an import failure instead of the standard dependency error dummy.
Reproduction:
```python
from diffusers.utils import is_flax_available, is_transformers_available
print(is_flax_available(), is_transformers_available())
from diffusers import FlaxStableDiffusionXLPipeline
print("top-level:", FlaxStableDiffusionXLPipeline)
from diffusers.pipelines.stable_diffusion_xl import FlaxStableDiffusionXLPipeline
print("subpackage:", FlaxStableDiffusionXLPipeline)
```
Relevant precedent:
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/__init__.py#L508-L528
Suggested fix:
```python
try:
if not (is_transformers_available() and is_flax_available()):
raise OptionalDependencyNotAvailable()
except OptionalDependencyNotAvailable:
from ...utils import dummy_flax_and_transformers_objects
_dummy_objects.update(get_objects_from_module(dummy_flax_and_transformers_objects))
else:
from ...schedulers.scheduling_pndm_flax import PNDMSchedulerState
_additional_imports.update({"PNDMSchedulerState": PNDMSchedulerState})
_import_structure["pipeline_flax_stable_diffusion_xl"] = ["FlaxStableDiffusionXLPipeline"]
```
## Issue 2: Negative crop coordinates are ignored in SDXL img2img/inpaint conditioning
Affected code:
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/stable_diffusion_xl/pipeline_stable_diffusion_xl_img2img.py#L858-L864
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/stable_diffusion_xl/pipeline_stable_diffusion_xl_inpaint.py#L963-L969
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/modular_pipelines/stable_diffusion_xl/before_denoise.py#L1152-L1158
Problem:
`negative_crops_coords_top_left` is accepted and passed into `_get_add_time_ids`, but the non-aesthetic branch uses `crops_coords_top_left` when building negative time ids.
Impact:
Users requesting different positive and negative crop conditioning silently get the positive crop coordinates for both branches, so negative micro-conditioning is wrong.
Reproduction:
```python
from types import SimpleNamespace
import torch
from diffusers import StableDiffusionXLImg2ImgPipeline
class FakePipe:
pass
pipe = FakePipe()
pipe.config = SimpleNamespace(requires_aesthetics_score=False)
pipe.unet = SimpleNamespace(
config=SimpleNamespace(addition_time_embed_dim=1),
add_embedding=SimpleNamespace(linear_1=SimpleNamespace(in_features=7)),
)
_, negative = StableDiffusionXLImg2ImgPipeline._get_add_time_ids(
pipe,
original_size=(64, 64),
crops_coords_top_left=(1, 2),
target_size=(64, 64),
aesthetic_score=6.0,
negative_aesthetic_score=2.0,
negative_original_size=(32, 32),
negative_crops_coords_top_left=(9, 10),
negative_target_size=(32, 32),
dtype=torch.float32,
text_encoder_projection_dim=1,
)
print(negative.tolist()) # contains 1, 2; expected 9, 10
```
Relevant precedent:
The text2img path passes negative crop coordinates through a separate `_get_add_time_ids` call:
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/stable_diffusion_xl/pipeline_stable_diffusion_xl.py#L1133-L1140
Suggested fix:
```python
add_neg_time_ids = list(negative_original_size + negative_crops_coords_top_left + negative_target_size)
```
## Issue 3: SDXL inpaint and instruct-pix2pix latent output bypasses cleanup and ignores `return_dict=False`
Affected code:
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/stable_diffusion_xl/pipeline_stable_diffusion_xl_inpaint.py#L1711-L1724
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/stable_diffusion_xl/pipeline_stable_diffusion_xl_instruct_pix2pix.py#L970-L980
Problem:
For `output_type="latent"`, these pipelines return immediately with `StableDiffusionXLPipelineOutput(images=latents)`. That skips `maybe_free_model_hooks()` and bypasses the later `return_dict` handling.
Impact:
`return_dict=False` returns the wrong type, and model offload cleanup is skipped for latent output.
Reproduction:
```python
import torch
from PIL import Image
from diffusers import StableDiffusionXLInpaintPipeline
pipe = StableDiffusionXLInpaintPipeline.from_pretrained(
"hf-internal-testing/tiny-stable-diffusion-xl-inpaint-pipe",
add_watermarker=False,
)
pipe.set_progress_bar_config(disable=True)
called = {"cleanup": False}
pipe.maybe_free_model_hooks = lambda: called.__setitem__("cleanup", True)
out = pipe(
"a cat",
image=Image.new("RGB", (64, 64), "white"),
mask_image=Image.new("L", (64, 64), 0),
num_inference_steps=1,
strength=1.0,
output_type="latent",
return_dict=False,
generator=torch.Generator("cpu").manual_seed(0),
)
print(type(out).__name__, called) # StableDiffusionXLPipelineOutput {'cleanup': False}
```
Relevant precedent:
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/stable_diffusion_xl/pipeline_stable_diffusion_xl.py#L1284-L1300
Suggested fix:
```python
else:
image = latents
self.maybe_free_model_hooks()
if not return_dict:
return (image,)
return StableDiffusionXLPipelineOutput(images=image)
```
## Issue 4: Latent output is passed through watermarking in SDXL img2img and modular decode
Affected code:
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/stable_diffusion_xl/pipeline_stable_diffusion_xl_img2img.py#L1477-L1484
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/modular_pipelines/stable_diffusion_xl/decoders.py#L129-L138
Problem:
`output_type="latent"` sets `image = latents`, but the img2img pipeline and modular decoder still call `watermark.apply_watermark(...)`. Text2img guards watermarking/postprocessing behind `output_type != "latent"`.
Impact:
Latent tensors are treated as decoded RGB images. With a real watermarker this can corrupt or fail for larger latent tensors; with any custom watermarker it is called for the wrong data type.
Reproduction:
```python
import torch
from PIL import Image
from diffusers import StableDiffusionXLImg2ImgPipeline
class SentinelWatermark:
def apply_watermark(self, images):
raise RuntimeError(f"watermark called for {tuple(images.shape)}")
pipe = StableDiffusionXLImg2ImgPipeline.from_pretrained(
"hf-internal-testing/tiny-stable-diffusion-xl-pipe",
add_watermarker=False,
)
pipe.set_progress_bar_config(disable=True)
pipe.watermark = SentinelWatermark()
pipe(
"a cat",
image=Image.new("RGB", (64, 64), "white"),
strength=1.0,
num_inference_steps=1,
output_type="latent",
generator=torch.Generator("cpu").manual_seed(0),
)
```
Relevant precedent:
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/stable_diffusion_xl/pipeline_stable_diffusion_xl.py#L1287-L1292
Suggested fix:
```python
if not output_type == "latent":
if self.watermark is not None:
image = self.watermark.apply_watermark(image)
image = self.image_processor.postprocess(image, output_type=output_type)
```
## Issue 5: Modular inpaint VAE encoder references `self.vae`
Affected code:
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/modular_pipelines/stable_diffusion_xl/encoders.py#L768-L774
Problem:
`StableDiffusionXLInpaintVaeEncoderStep._encode_vae_image()` uses `self.vae.config.scaling_factor`, but `self` is the block, not the pipeline/components object.
Impact:
Any inpaint modular pipeline using a VAE config with `latents_mean` and `latents_std` fails with `AttributeError`.
Reproduction:
```python
from types import SimpleNamespace
import torch
from diffusers.modular_pipelines.stable_diffusion_xl.encoders import StableDiffusionXLInpaintVaeEncoderStep
class FakeVAE:
config = SimpleNamespace(
force_upcast=False,
latents_mean=[0.0, 0.0, 0.0, 0.0],
latents_std=[1.0, 1.0, 1.0, 1.0],
scaling_factor=0.18215,
)
def encode(self, image):
return SimpleNamespace(latents=torch.ones(image.shape[0], 4, 2, 2, dtype=image.dtype))
components = SimpleNamespace(vae=FakeVAE())
StableDiffusionXLInpaintVaeEncoderStep()._encode_vae_image(
components, torch.zeros(1, 3, 16, 16), generator=None
)
```
Relevant precedent:
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/modular_pipelines/stable_diffusion_xl/encoders.py#L648-L653
Suggested fix:
```python
image_latents = (image_latents - latents_mean) * components.vae.config.scaling_factor / latents_std
```
## Issue 6: Modular SDXL generated docstring still contains TODO placeholders
Affected code:
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/modular_pipelines/stable_diffusion_xl/modular_blocks_stable_diffusion_xl.py#L320-L428
Problem:
`StableDiffusionXLAutoBlocks.__doc__` contains 36 `TODO: Add description.` placeholders. The modular review rules require generated auto-docstrings to be regenerated and verified with no TODO placeholders.
Impact:
The public docs/signature help for the main SDXL modular block are incomplete, especially for core inputs like `prompt`, `height`, `width`, `num_inference_steps`, ControlNet inputs, and denoising controls.
Reproduction:
```python
from diffusers import StableDiffusionXLAutoBlocks
doc = StableDiffusionXLAutoBlocks.__doc__ or ""
print(doc.count("TODO: Add description."))
assert "TODO: Add description." not in doc
```
Relevant precedent:
Other modular families should have generated docs with completed parameter descriptions after running `utils/modular_auto_docstring.py`.
Suggested fix:
Populate the missing `InputParam`/`OutputParam` descriptions or use matching templates, then run:
```bash
python utils/modular_auto_docstring.py --fix_and_overwrite
```
## Issue 7: Slow coverage is missing for Flax SDXL, SDXL instruct-pix2pix, and modular SDXL
Affected code:
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/stable_diffusion_xl/pipeline_flax_stable_diffusion_xl.py#L43
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/stable_diffusion_xl/pipeline_stable_diffusion_xl_instruct_pix2pix.py#L113
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/modular_pipelines/stable_diffusion_xl/modular_pipeline.py#L38
Problem:
Fast tests exist for SDXL text2img, img2img, inpaint, instruct-pix2pix, and modular SDXL, and slow tests exist for standard text2img/img2img/inpaint. Slow tests are missing for Flax SDXL, SDXL instruct-pix2pix, and modular SDXL.
Impact:
Real-checkpoint behavior, loading/offload behavior, and parity regressions for these variants can ship without integration coverage.
Reproduction:
```python
from pathlib import Path
paths = [
Path("tests/pipelines/stable_diffusion_xl/test_stable_diffusion_xl.py"),
Path("tests/pipelines/stable_diffusion_xl/test_stable_diffusion_xl_img2img.py"),
Path("tests/pipelines/stable_diffusion_xl/test_stable_diffusion_xl_inpaint.py"),
Path("tests/pipelines/stable_diffusion_xl/test_stable_diffusion_xl_instruction_pix2pix.py"),
Path("tests/modular_pipelines/stable_diffusion_xl/test_modular_pipeline_stable_diffusion_xl.py"),
]
for path in paths:
print(path, path.read_text(encoding="utf-8").count("@slow"))
print("Flax SDXL tests:", list(Path("tests").rglob("*flax*sdxl*")) + list(Path("tests").rglob("*sdxl*flax*")))
```
Relevant precedent:
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/tests/pipelines/stable_diffusion_xl/test_stable_diffusion_xl.py#L939-L940
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/tests/pipelines/stable_diffusion_xl/test_stable_diffusion_xl_img2img.py#L669-L670
Suggested fix:
Add at least one `@slow` integration test each for Flax SDXL, SDXL instruct-pix2pix, and modular SDXL using small deterministic prompts/seeds and existing tiny fixtures where possible.
Guía de contribución
Línea de trabajo
Start by separating the seven findings across the affected SDXL pipeline files, including the lazy-loading __init__.py files, img2img/inpaint pipelines, modular encoders and decoders, and modular_blocks_stable_diffusion_xl.py. Run the supplied Python reproductions and utils/modular_auto_docstring.py --fix_and_overwrite where relevant. Done means the reproductions pass, latent and dependency behaviors match the stated precedents, generated docs contain no TODO placeholders, and the missing slow coverage is added.
Escrito por el modelo de indexación a partir del texto del issue.
Evaluación
- Stack tecnológico
- python, pytorch
- Área
- documentation, machine-learning, testing-qa
- Tipo de issue
- Error
- Dificultad
- 5/5
- Tiempo estimado
- Más de una semana
- Estado de actividad
- Tranquilo
- Claridad
- Bastante claro
- Aptitud para principiantes
- 45/100