huggingface / huggingface/diffusers

stable_diffusion_xl model/pipeline review

オープン
#13,610 コメント 0 件 リアクション 0 件 担当者 0 名 GitHub で見る
主要言語
Python
スター
34.5k
フォーク
7.3k
平均マージ
3日 3時間
マージ済み PR(30日)
91

説明

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

コントリビューションガイド

コントリビューションガイドを開く

調査の方向性

まず、lazy loading 用の __init__.py ファイル、img2img/inpaint パイプライン、モジュール式のエンコーダーとデコーダー、modular_blocks_stable_diffusion_xl.py など、影響を受ける SDXL パイプラインファイルに7つの調査結果を振り分けます。提供されている Python の再現コードと、該当する場合は utils/modular_auto_docstring.py --fix_and_overwrite を実行します。再現コードがパスし、latent と依存関係の動作が記載された先例と一致し、生成されたドキュメントに TODO プレースホルダーがなく、不足している slow のカバレッジが追加されれば完了です。

索引モデルが issue の本文から書いたものです。

評価

技術スタック
python, pytorch
領域
documentation, machine-learning, testing-qa
issue の種類
バグ
難易度
5/5
見積もり時間
1週間以上
活発さ
静か
明瞭さ
おおむね明確
初心者へのやさしさ
45/100

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。