huggingface / huggingface/diffusers
stable_diffusion_xl model/pipeline review
- 主要言語
- 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