huggingface / huggingface/diffusers

stable_diffusion_xl model/pipeline review

Aperta
#13,610 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

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

Guida per i contributori

Apri la guida per i contributori

Direzione di ricerca

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.

Scritto dal modello di indicizzazione a partire dal testo della issue.

Valutazione

Stack tecnologico
python, pytorch
Ambito
documentation, machine-learning, testing-qa
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.