huggingface / huggingface/diffusers
ledits_pp model/pipeline review
- Lingua principale
- Python
- Stelle
- 34.5k
- Fork
- 7.3k
- Merge medio
- 3g 3h
- PR unite (30g)
- 91
Descrizione
# `ledits_pp` model/pipeline review
Commit tested: `0f1abc4ae8b0eb2a3b40e82a310507281144c423`
Review performed against the repository review rules.
Coverage checked: fast and slow test classes exist for both SD and SDXL LEdits++; fast tests passed locally with `.venv` (`3 passed, 1 skipped` for each file). Slow tests exist but were not run. Duplicate search was run with `gh search issues/prs`; no duplicates found for the issues below. Existing open issue https://github.com/huggingface/diffusers/issues/8826 covers a separate known LEdits++ empty attention-store crash, so I am not presenting that one as new.
## Issue 1: `LEditsPPInversionPipelineOutput` is missing from lazy exports
Affected code:
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/ledits_pp/__init__.py#L24-L27
Problem:
The lazy import structure exports `LEditsPPDiffusionPipelineOutput` twice and omits `LEditsPPInversionPipelineOutput`. The eager `TYPE_CHECKING` path imports both outputs, so behavior differs between slow/eager imports and normal lazy imports. `diffusers.pipelines.__init__` also imports these outputs in the eager path but does not expose them in the lazy import structure.
Impact:
Public imports documented under `pipelines.ledits_pp` fail at runtime for the inversion output class.
Reproduction:
```python
from diffusers.pipelines.ledits_pp import LEditsPPInversionPipelineOutput
# AttributeError: module diffusers.pipelines.ledits_pp has no attribute LEditsPPInversionPipelineOutput
```
Relevant precedent:
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/ledits_pp/pipeline_output.py#L27-L42
Suggested fix:
```python
_import_structure["pipeline_output"] = [
"LEditsPPDiffusionPipelineOutput",
"LEditsPPInversionPipelineOutput",
]
```
Also either add both output classes to `src/diffusers/pipelines/__init__.py` lazy exports or remove the eager-only imports there.
## Issue 2: SDXL editing after batched inversion has inconsistent embedding batch sizes
Affected code:
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/ledits_pp/pipeline_leditspp_stable_diffusion_xl.py#L538-L558
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/ledits_pp/pipeline_leditspp_stable_diffusion_xl.py#L1112-L1121
Problem:
`LEditsPPPipelineStableDiffusionXL.encode_prompt()` encodes edit concepts once per concept, not once per `(concept, image)` pair. After batched inversion, latents are repeated by `1 + enabled_editing_prompts`, but edit prompt embeddings and pooled embeddings are not expanded the same way.
Impact:
SDXL LEdits++ cannot edit a batch of inverted images with multiple edit prompts. The failure is currently untested because fast tests cover batched inversion only, not batched editing.
Reproduction:
```python
from diffusers import LEditsPPPipelineStableDiffusionXL
from tests.pipelines.ledits_pp.test_ledits_pp_stable_diffusion_xl import LEditsPPPipelineStableDiffusionXLFastTests
case = LEditsPPPipelineStableDiffusionXLFastTests()
pipe = LEditsPPPipelineStableDiffusionXL(**case.get_dummy_components())
pipe.set_progress_bar_config(disable=True)
inputs = case.get_dummy_inversion_inputs("cpu")
inputs["num_inversion_steps"] = 2
inputs["skip"] = 0.0
pipe.invert(**inputs)
pipe(editing_prompt=["wearing glasses", "sunshine"], output_type="latent")
# RuntimeError: mat1 and mat2 shapes cannot be multiplied ...
```
Relevant precedent:
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/ledits_pp/pipeline_leditspp_stable_diffusion.py#L632-L645
Suggested fix:
Expand edit concept embeddings per image in `__call__` or `encode_prompt`, while keeping `num_edit_tokens` indexed per concept. The implementation should preserve the inversion path, where `editing_prompt` is reused as the source prompt and already represents one prompt per image.
## Issue 3: SDXL non-square image sizes are swapped for micro-conditioning
Affected code:
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/ledits_pp/pipeline_leditspp_stable_diffusion_xl.py#L1574-L1576
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/ledits_pp/pipeline_leditspp_stable_diffusion_xl.py#L1103-L1110
Problem:
`invert()` computes `width` from latent height and `height` from latent width, then stores `self.size = (height, width)`. For non-square images, SDXL added time IDs receive swapped original/target sizes. The `target_size` argument accepted by `__call__` is also ignored; the call always uses `self.size`.
Impact:
Non-square SDXL edits are conditioned on the wrong dimensions, which can degrade output and makes the documented `target_size` parameter ineffective.
Reproduction:
```python
from diffusers import LEditsPPPipelineStableDiffusionXL
from tests.pipelines.ledits_pp.test_ledits_pp_stable_diffusion_xl import LEditsPPPipelineStableDiffusionXLFastTests
case = LEditsPPPipelineStableDiffusionXLFastTests()
pipe = LEditsPPPipelineStableDiffusionXL(**case.get_dummy_components())
pipe.set_progress_bar_config(disable=True)
inputs = case.get_dummy_inversion_inputs("cpu")
inputs["image"] = inputs["image"][0].resize((64, 32))
inputs.update({"height": 32, "width": 64, "num_inversion_steps": 2, "skip": 0.0})
pipe.invert(**inputs)
print(pipe.size) # (64, 32), expected (32, 64)
```
Relevant precedent:
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/stable_diffusion_xl/pipeline_stable_diffusion_xl.py#L1030-L1031
Suggested fix:
```python
height = x0.shape[-2] * self.vae_scale_factor
width = x0.shape[-1] * self.vae_scale_factor
self.size = (height, width)
```
Then use `target_size = target_size or self.size` in `__call__`.
## Issue 4: Callback tensor allowlists include names that are not in scope
Affected code:
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/ledits_pp/pipeline_leditspp_stable_diffusion.py#L301-L304
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/ledits_pp/pipeline_leditspp_stable_diffusion.py#L1235-L1243
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/ledits_pp/pipeline_leditspp_stable_diffusion_xl.py#L336-L344
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/ledits_pp/pipeline_leditspp_stable_diffusion_xl.py#L1016-L1026
Problem:
The SD pipeline allows `prompt_embeds`, but the denoising loop has no `prompt_embeds` local. The SDXL pipeline allows `negative_add_time_ids`, but that local is commented out and never defined; SDXL also comments out its `check_inputs()` call, so invalid callback tensor names are not rejected early.
Impact:
Documented callback customization crashes with `KeyError` instead of either passing the requested tensor or raising the normal validation error.
Reproduction:
```python
from diffusers import LEditsPPPipelineStableDiffusion
from tests.pipelines.ledits_pp.test_ledits_pp_stable_diffusion import LEditsPPPipelineStableDiffusionFastTests
case = LEditsPPPipelineStableDiffusionFastTests()
pipe = LEditsPPPipelineStableDiffusion(**case.get_dummy_components())
pipe.set_progress_bar_config(disable=True)
inputs = case.get_dummy_inversion_inputs("cpu")
inputs.update({"image": inputs["image"][0], "num_inversion_steps": 1, "skip": 0.0})
pipe.invert(**inputs)
def cb(pipe, step, timestep, kwargs):
return kwargs
pipe(
editing_prompt="cat",
output_type="latent",
use_intersect_mask=False,
callback_on_step_end=cb,
callback_on_step_end_tensor_inputs=["prompt_embeds"],
)
# KeyError: 'prompt_embeds'
```
Relevant precedent:
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/stable_diffusion_xl/pipeline_stable_diffusion_xl.py#L232-L237
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/stable_diffusion_xl/pipeline_stable_diffusion_xl.py#L636-L640
Suggested fix:
Make `_callback_tensor_inputs` match actual denoising-loop locals, call `check_inputs()` in SDXL, and add focused fast tests for every allowed callback tensor.
## Issue 5: SDXL IP-Adapter path calls the VAE image encoder instead of IP-Adapter encoding
Affected code:
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/ledits_pp/pipeline_leditspp_stable_diffusion_xl.py#L1123-L1128
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/ledits_pp/pipeline_leditspp_stable_diffusion_xl.py#L1440-L1473
Problem:
`__call__()` accepts `ip_adapter_image`, but calls `self.encode_image(ip_adapter_image, device, num_images_per_prompt)`. In this pipeline, `encode_image()` is the VAE inversion helper, not the CLIP/IP-Adapter image encoder from SDXL. The positional arguments are interpreted as `dtype` and `height`.
Impact:
The advertised IP-Adapter path is unusable and fails before preparing image embeddings.
Reproduction:
```python
from diffusers import LEditsPPPipelineStableDiffusionXL
from tests.pipelines.ledits_pp.test_ledits_pp_stable_diffusion_xl import LEditsPPPipelineStableDiffusionXLFastTests
case = LEditsPPPipelineStableDiffusionXLFastTests()
pipe = LEditsPPPipelineStableDiffusionXL(**case.get_dummy_components())
pipe.set_progress_bar_config(disable=True)
inputs = case.get_dummy_inversion_inputs("cpu")
inputs.update({"image": inputs["image"][0], "num_inversion_steps": 1, "skip": 0.0})
pipe.invert(**inputs)
pipe(editing_prompt="cat", ip_adapter_image=inputs["image"], output_type="latent")
# ValueError: height and width must be > 0
```
Relevant precedent:
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/stable_diffusion_xl/pipeline_stable_diffusion_xl.py#L522-L590
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/stable_diffusion_xl/pipeline_stable_diffusion_xl.py#L1153-L1160
Suggested fix:
Rename the VAE helper to something like `encode_vae_image()`, restore/copy SDXL’s `encode_image()` and `prepare_ip_adapter_image_embeds()`, add `ip_adapter_image_embeds`, and include `image_encoder` in the offload sequence.
## Issue 6: `invert()` and cross-attention masking permanently replace user attention processors
Affected code:
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/ledits_pp/pipeline_leditspp_stable_diffusion.py#L497-L519
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/ledits_pp/pipeline_leditspp_stable_diffusion.py#L1343-L1344
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/ledits_pp/pipeline_leditspp_stable_diffusion_xl.py#L811-L834
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/ledits_pp/pipeline_leditspp_stable_diffusion_xl.py#L1543-L1544
Problem:
`invert()` unconditionally calls `self.unet.set_attn_processor(AttnProcessor())`. `prepare_unet()` also replaces all processors with either `LEDITSCrossAttnProcessor` or plain `AttnProcessor()`. Neither path restores the original processors.
Impact:
Any configured attention backend or custom processor, including `AttnProcessor2_0`, xFormers-style processors, LoRA/IP-Adapter processors, or user-supplied processors, is silently discarded after inversion or masked editing.
Reproduction:
```python
from diffusers import LEditsPPPipelineStableDiffusion
from diffusers.models.attention_processor import AttnProcessor2_0
from tests.pipelines.ledits_pp.test_ledits_pp_stable_diffusion import LEditsPPPipelineStableDiffusionFastTests
case = LEditsPPPipelineStableDiffusionFastTests()
pipe = LEditsPPPipelineStableDiffusion(**case.get_dummy_components())
pipe.unet.set_attn_processor(AttnProcessor2_0())
before = {type(p).__name__ for p in pipe.unet.attn_processors.values()}
inputs = case.get_dummy_inversion_inputs("cpu")
inputs.update({"image": inputs["image"][0], "num_inversion_steps": 1, "skip": 0.0})
pipe.set_progress_bar_config(disable=True)
pipe.invert(**inputs)
after = {type(p).__name__ for p in pipe.unet.attn_processors.values()}
print(before, after) # {'AttnProcessor2_0'} {'AttnProcessor'}
```
Relevant precedent:
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/models/attention_processor.py#L2696-L2786
Suggested fix:
Store the original `unet.attn_processors` before installing LEdits processors and restore them in a `finally` block after the temporary operation. For non-intercepted attention layers, preserve the existing processor instead of replacing it with bare `AttnProcessor()`.
## Issue 7: CPU offload paths use `self.device` instead of `_execution_device`
Affected code:
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/ledits_pp/pipeline_leditspp_stable_diffusion.py#L948-L995
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/ledits_pp/pipeline_leditspp_stable_diffusion.py#L1388-L1398
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/ledits_pp/pipeline_leditspp_stable_diffusion_xl.py#L1010-L1014
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/ledits_pp/pipeline_leditspp_stable_diffusion_xl.py#L1459-L1467
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/ledits_pp/pipeline_leditspp_stable_diffusion_xl.py#L1650-L1655
Problem:
Both pipelines declare `model_cpu_offload_seq`, but many runtime tensors are moved to `self.device`. Under `enable_model_cpu_offload()`, `self.device` can remain CPU while `_execution_device` is the actual accelerator. Standard pipelines use `_execution_device` at call time for this reason.
Impact:
CPU offload can put prompts, latents, masks, smoothing kernels, and inversion noise on the wrong device, causing device mismatches or unexpectedly running heavy work on CPU.
Reproduction:
```python
import torch
from diffusers import LEditsPPPipelineStableDiffusionXL
from tests.pipelines.ledits_pp.test_ledits_pp_stable_diffusion_xl import LEditsPPPipelineStableDiffusionXLFastTests
if not torch.cuda.is_available():
raise SystemExit("requires CUDA to exercise model CPU offload")
case = LEditsPPPipelineStableDiffusionXLFastTests()
pipe = LEditsPPPipelineStableDiffusionXL(**case.get_dummy_components())
pipe.enable_model_cpu_offload()
print(pipe.device, pipe._execution_device) # CPU vs CUDA under offload
inputs = case.get_dummy_inversion_inputs("cpu")
inputs.update({"image": inputs["image"][0], "num_inversion_steps": 2, "skip": 0.0})
pipe.invert(**inputs)
pipe(editing_prompt="cat", output_type="latent", use_cross_attn_mask=True)
```
Relevant precedent:
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/stable_diffusion/pipeline_stable_diffusion.py#L955-L955
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/latent_consistency_models/pipeline_latent_consistency_text2img.py#L786-L786
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/latent_consistency_models/pipeline_latent_consistency_img2img.py#L849-L849
Suggested fix:
At the start of `__call__()` and `invert()`, set `device = self._execution_device` and use that local for tensor movement/allocation. Pass `device` into VAE image encoding helpers instead of reading `self.device` inside them.
Guida per i contributori
Apri la guida per i contributori
Direzione di ricerca
Inizia dai file della pipeline LEdits++ interessati e dai test veloci in tests/pipelines/ledits_pp/test_ledits_pp_stable_diffusion.py e test_ledits_pp_stable_diffusion_xl.py. Riproduci separatamente ogni caso segnalato relativo a importazione, batching, dimensionamento, callback, IP-Adapter e processore dell’attenzione, quindi aggiungi una copertura di regressione mirata ed esegui le classi di test veloci pertinenti. Il lavoro è completato quando i fallimenti segnalati sono coperti e i test esistenti di inversione e modifica continuano a passare.
Scritto dal modello di indicizzazione a partire dal testo della issue.
Valutazione
- Stack tecnologico
- python
- Ambito
- machine-learning
- 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