Repository navigation
stable_diffusion_xl model/pipeline review #13610
Description
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:
diffusers/src/diffusers/pipelines/stable_diffusion_xl/__init__.py
Lines 33 to 37 in 0f1abc4
| if is_transformers_available() and is_flax_available(): | |
| from ...schedulers.scheduling_pndm_flax import PNDMSchedulerState | |
| _additional_imports.update({"PNDMSchedulerState": PNDMSchedulerState}) | |
| _import_structure["pipeline_flax_stable_diffusion_xl"] = ["FlaxStableDiffusionXLPipeline"] |
diffusers/src/diffusers/pipelines/stable_diffusion_xl/__init__.py
Lines 53 to 57 in 0f1abc4
| if not (is_transformers_available() and is_flax_available()): | |
| raise OptionalDependencyNotAvailable() | |
| except OptionalDependencyNotAvailable: | |
| from ...utils.dummy_flax_objects import * | |
| else: |
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:
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:
diffusers/src/diffusers/pipelines/__init__.py
Lines 508 to 528 in 0f1abc4
| try: | |
| if not (is_flax_available() and is_transformers_available()): | |
| raise OptionalDependencyNotAvailable() | |
| except OptionalDependencyNotAvailable: | |
| from ..utils import dummy_flax_and_transformers_objects # noqa F403 | |
| _dummy_objects.update(get_objects_from_module(dummy_flax_and_transformers_objects)) | |
| else: | |
| _import_structure["controlnet"].extend(["FlaxStableDiffusionControlNetPipeline"]) | |
| _import_structure["stable_diffusion"].extend( | |
| [ | |
| "FlaxStableDiffusionImg2ImgPipeline", | |
| "FlaxStableDiffusionInpaintPipeline", | |
| "FlaxStableDiffusionPipeline", | |
| ] | |
| ) | |
| _import_structure["stable_diffusion_xl"].extend( | |
| [ | |
| "FlaxStableDiffusionXLPipeline", | |
| ] | |
| ) |
Suggested fix:
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:
diffusers/src/diffusers/pipelines/stable_diffusion_xl/pipeline_stable_diffusion_xl_img2img.py
Lines 858 to 864 in 0f1abc4
| add_neg_time_ids = list( | |
| negative_original_size + negative_crops_coords_top_left + (negative_aesthetic_score,) | |
| ) | |
| else: | |
| add_time_ids = list(original_size + crops_coords_top_left + target_size) | |
| add_neg_time_ids = list(negative_original_size + crops_coords_top_left + negative_target_size) | |
diffusers/src/diffusers/pipelines/stable_diffusion_xl/pipeline_stable_diffusion_xl_inpaint.py
Lines 963 to 969 in 0f1abc4
| add_neg_time_ids = list( | |
| negative_original_size + negative_crops_coords_top_left + (negative_aesthetic_score,) | |
| ) | |
| else: | |
| add_time_ids = list(original_size + crops_coords_top_left + target_size) | |
| add_neg_time_ids = list(negative_original_size + crops_coords_top_left + negative_target_size) | |
diffusers/src/diffusers/modular_pipelines/stable_diffusion_xl/before_denoise.py
Lines 1152 to 1158 in 0f1abc4
| add_neg_time_ids = list( | |
| negative_original_size + negative_crops_coords_top_left + (negative_aesthetic_score,) | |
| ) | |
| else: | |
| add_time_ids = list(original_size + crops_coords_top_left + target_size) | |
| add_neg_time_ids = list(negative_original_size + crops_coords_top_left + negative_target_size) | |
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:
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, 10Relevant precedent:
The text2img path passes negative crop coordinates through a separate _get_add_time_ids call:
diffusers/src/diffusers/pipelines/stable_diffusion_xl/pipeline_stable_diffusion_xl.py
Lines 1133 to 1140 in 0f1abc4
| if negative_original_size is not None and negative_target_size is not None: | |
| negative_add_time_ids = self._get_add_time_ids( | |
| negative_original_size, | |
| negative_crops_coords_top_left, | |
| negative_target_size, | |
| dtype=prompt_embeds.dtype, | |
| text_encoder_projection_dim=text_encoder_projection_dim, | |
| ) |
Suggested fix:
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:
diffusers/src/diffusers/pipelines/stable_diffusion_xl/pipeline_stable_diffusion_xl_inpaint.py
Lines 1711 to 1724 in 0f1abc4
| else: | |
| return StableDiffusionXLPipelineOutput(images=latents) | |
| # apply watermark if available | |
| if self.watermark is not None: | |
| image = self.watermark.apply_watermark(image) | |
| image = self.image_processor.postprocess(image, output_type=output_type) | |
| if padding_mask_crop is not None: | |
| image = [self.image_processor.apply_overlay(mask_image, original_image, i, crops_coords) for i in image] | |
| # Offload all models | |
| self.maybe_free_model_hooks() |
Lines 970 to 980 in 0f1abc4
| else: | |
| return StableDiffusionXLPipelineOutput(images=latents) | |
| # apply watermark if available | |
| if self.watermark is not None: | |
| image = self.watermark.apply_watermark(image) | |
| image = self.image_processor.postprocess(image, output_type=output_type) | |
| # Offload all models | |
| self.maybe_free_model_hooks() |
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:
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:
diffusers/src/diffusers/pipelines/stable_diffusion_xl/pipeline_stable_diffusion_xl.py
Lines 1284 to 1300 in 0f1abc4
| else: | |
| image = latents | |
| if not output_type == "latent": | |
| # apply watermark if available | |
| if self.watermark is not None: | |
| image = self.watermark.apply_watermark(image) | |
| image = self.image_processor.postprocess(image, output_type=output_type) | |
| # Offload all models | |
| self.maybe_free_model_hooks() | |
| if not return_dict: | |
| return (image,) | |
| return StableDiffusionXLPipelineOutput(images=image) |
Suggested fix:
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:
diffusers/src/diffusers/pipelines/stable_diffusion_xl/pipeline_stable_diffusion_xl_img2img.py
Lines 1477 to 1484 in 0f1abc4
| else: | |
| image = latents | |
| # apply watermark if available | |
| if self.watermark is not None: | |
| image = self.watermark.apply_watermark(image) | |
| image = self.image_processor.postprocess(image, output_type=output_type) |
diffusers/src/diffusers/modular_pipelines/stable_diffusion_xl/decoders.py
Lines 129 to 138 in 0f1abc4
| else: | |
| block_state.images = block_state.latents | |
| # apply watermark if available | |
| if hasattr(components, "watermark") and components.watermark is not None: | |
| block_state.images = components.watermark.apply_watermark(block_state.images) | |
| block_state.images = components.image_processor.postprocess( | |
| block_state.images, output_type=block_state.output_type | |
| ) |
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:
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:
diffusers/src/diffusers/pipelines/stable_diffusion_xl/pipeline_stable_diffusion_xl.py
Lines 1287 to 1292 in 0f1abc4
| if not output_type == "latent": | |
| # apply watermark if available | |
| if self.watermark is not None: | |
| image = self.watermark.apply_watermark(image) | |
| image = self.image_processor.postprocess(image, output_type=output_type) |
Suggested fix:
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:
diffusers/src/diffusers/modular_pipelines/stable_diffusion_xl/encoders.py
Lines 768 to 774 in 0f1abc4
| image_latents = image_latents.to(dtype) | |
| if latents_mean is not None and latents_std is not None: | |
| latents_mean = latents_mean.to(device=image_latents.device, dtype=dtype) | |
| latents_std = latents_std.to(device=image_latents.device, dtype=dtype) | |
| image_latents = (image_latents - latents_mean) * self.vae.config.scaling_factor / latents_std | |
| else: | |
| image_latents = components.vae.config.scaling_factor * image_latents |
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:
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:
diffusers/src/diffusers/modular_pipelines/stable_diffusion_xl/encoders.py
Lines 648 to 653 in 0f1abc4
| if latents_mean is not None and latents_std is not None: | |
| latents_mean = latents_mean.to(device=image_latents.device, dtype=dtype) | |
| latents_std = latents_std.to(device=image_latents.device, dtype=dtype) | |
| image_latents = (image_latents - latents_mean) * components.vae.config.scaling_factor / latents_std | |
| else: | |
| image_latents = components.vae.config.scaling_factor * image_latents |
Suggested fix:
image_latents = (image_latents - latents_mean) * components.vae.config.scaling_factor / latents_stdIssue 6: Modular SDXL generated docstring still contains TODO placeholders
Affected code:
diffusers/src/diffusers/modular_pipelines/stable_diffusion_xl/modular_blocks_stable_diffusion_xl.py
Lines 320 to 428 in 0f1abc4
| Inputs: | |
| prompt (`None`, *optional*): | |
| TODO: Add description. | |
| prompt_2 (`None`, *optional*): | |
| TODO: Add description. | |
| negative_prompt (`None`, *optional*): | |
| TODO: Add description. | |
| negative_prompt_2 (`None`, *optional*): | |
| TODO: Add description. | |
| cross_attention_kwargs (`None`, *optional*): | |
| TODO: Add description. | |
| clip_skip (`None`, *optional*): | |
| TODO: Add description. | |
| ip_adapter_image (`Image | ndarray | Tensor | list | list | list`, *optional*): | |
| The image(s) to be used as ip adapter | |
| height (`None`, *optional*): | |
| TODO: Add description. | |
| width (`None`, *optional*): | |
| TODO: Add description. | |
| image (`None`, *optional*): | |
| TODO: Add description. | |
| mask_image (`None`, *optional*): | |
| TODO: Add description. | |
| padding_mask_crop (`None`, *optional*): | |
| TODO: Add description. | |
| dtype (`dtype`, *optional*): | |
| The dtype of the model inputs | |
| generator (`None`, *optional*): | |
| TODO: Add description. | |
| preprocess_kwargs (`dict | NoneType`, *optional*): | |
| A kwargs dictionary that if specified is passed along to the `ImageProcessor` as defined under | |
| `self.image_processor` in [diffusers.image_processor.VaeImageProcessor] | |
| num_images_per_prompt (`None`, *optional*, defaults to 1): | |
| TODO: Add description. | |
| ip_adapter_embeds (`list`, *optional*): | |
| Pre-generated image embeddings for IP-Adapter. Can be generated from ip_adapter step. | |
| negative_ip_adapter_embeds (`list`, *optional*): | |
| Pre-generated negative image embeddings for IP-Adapter. Can be generated from ip_adapter step. | |
| num_inference_steps (`None`, *optional*, defaults to 50): | |
| TODO: Add description. | |
| timesteps (`None`, *optional*): | |
| TODO: Add description. | |
| sigmas (`None`, *optional*): | |
| TODO: Add description. | |
| denoising_end (`None`, *optional*): | |
| TODO: Add description. | |
| strength (`None`, *optional*, defaults to 0.3): | |
| TODO: Add description. | |
| denoising_start (`None`, *optional*): | |
| TODO: Add description. | |
| latents (`None`): | |
| TODO: Add description. | |
| image_latents (`Tensor`, *optional*): | |
| The latents representing the reference image for image-to-image/inpainting generation. Can be generated | |
| in vae_encode step. | |
| mask (`Tensor`, *optional*): | |
| The mask for the inpainting generation. Can be generated in vae_encode step. | |
| masked_image_latents (`Tensor`, *optional*): | |
| The masked image latents for the inpainting generation (only for inpainting-specific unet). Can be | |
| generated in vae_encode step. | |
| original_size (`None`, *optional*): | |
| TODO: Add description. | |
| target_size (`None`, *optional*): | |
| TODO: Add description. | |
| negative_original_size (`None`, *optional*): | |
| TODO: Add description. | |
| negative_target_size (`None`, *optional*): | |
| TODO: Add description. | |
| crops_coords_top_left (`None`, *optional*, defaults to (0, 0)): | |
| TODO: Add description. | |
| negative_crops_coords_top_left (`None`, *optional*, defaults to (0, 0)): | |
| TODO: Add description. | |
| aesthetic_score (`None`, *optional*, defaults to 6.0): | |
| TODO: Add description. | |
| negative_aesthetic_score (`None`, *optional*, defaults to 2.0): | |
| TODO: Add description. | |
| control_image (`None`, *optional*): | |
| TODO: Add description. | |
| control_mode (`None`, *optional*): | |
| TODO: Add description. | |
| control_guidance_start (`None`, *optional*, defaults to 0.0): | |
| TODO: Add description. | |
| control_guidance_end (`None`, *optional*, defaults to 1.0): | |
| TODO: Add description. | |
| controlnet_conditioning_scale (`None`, *optional*, defaults to 1.0): | |
| TODO: Add description. | |
| guess_mode (`None`, *optional*, defaults to False): | |
| TODO: Add description. | |
| crops_coords (`tuple | NoneType`, *optional*): | |
| The crop coordinates to use for preprocess/postprocess the image and mask, for inpainting task only. Can | |
| be generated in vae_encode step. | |
| controlnet_cond (`Tensor`, *optional*): | |
| The control image to use for the denoising process. Can be generated in prepare_controlnet_inputs step. | |
| conditioning_scale (`float`, *optional*): | |
| The controlnet conditioning scale value to use for the denoising process. Can be generated in | |
| prepare_controlnet_inputs step. | |
| controlnet_keep (`list`, *optional*): | |
| The controlnet keep values to use for the denoising process. Can be generated in | |
| prepare_controlnet_inputs step. | |
| **denoiser_input_fields (`None`, *optional*): | |
| All conditional model inputs that need to be prepared with guider. It should contain | |
| prompt_embeds/negative_prompt_embeds, add_time_ids/negative_add_time_ids, | |
| pooled_prompt_embeds/negative_pooled_prompt_embeds, and ip_adapter_embeds/negative_ip_adapter_embeds | |
| (optional).please add `kwargs_type=denoiser_input_fields` to their parameter spec (`OutputParam`) when | |
| they are created and added to the pipeline state | |
| eta (`None`, *optional*, defaults to 0.0): | |
| TODO: Add description. | |
| output_type (`None`, *optional*, defaults to pil): | |
| TODO: Add description. |
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:
from diffusers import StableDiffusionXLAutoBlocks
doc = StableDiffusionXLAutoBlocks.__doc__ or ""
print(doc.count("TODO: Add description."))
assert "TODO: Add description." not in docRelevant 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:
python utils/modular_auto_docstring.py --fix_and_overwriteIssue 7: Slow coverage is missing for Flax SDXL, SDXL instruct-pix2pix, and modular SDXL
Affected code:
diffusers/src/diffusers/pipelines/stable_diffusion_xl/pipeline_flax_stable_diffusion_xl.py
Line 43 in 0f1abc4
| class FlaxStableDiffusionXLPipeline(FlaxDiffusionPipeline): |
Line 113 in 0f1abc4
| class StableDiffusionXLInstructPix2PixPipeline( |
diffusers/src/diffusers/modular_pipelines/stable_diffusion_xl/modular_pipeline.py
Line 38 in 0f1abc4
| class StableDiffusionXLModularPipeline( |
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:
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:
diffusers/tests/pipelines/stable_diffusion_xl/test_stable_diffusion_xl.py
Lines 939 to 940 in 0f1abc4
| @slow | |
| class StableDiffusionXLPipelineIntegrationTests(unittest.TestCase): |
diffusers/tests/pipelines/stable_diffusion_xl/test_stable_diffusion_xl_img2img.py
Lines 669 to 670 in 0f1abc4
| @slow | |
| class StableDiffusionXLImg2ImgPipelineIntegrationTests(unittest.TestCase): |
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.
Activity