huggingface / huggingface/diffusers

deepfloyd_if model/pipeline review

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

# `deepfloyd_if` model/pipeline review

Commit tested: `0f1abc4ae8b0eb2a3b40e82a310507281144c423`

Review performed against the repository review rules. Note: `.ai/review-rules.md` references `AGENTS.md`, but that file is absent in this checkout; the remaining referenced rule files were applied.

Duplicate search status: searched GitHub Issues and PRs for `deepfloyd_if`, affected class/function names, NumPy image/mask failures, `IFPipelineOutput` import behavior, strength validation, and `T5FilmDecoder` coverage. I found no likely duplicates.

## Issue 1: Unbatched NumPy image inputs are rejected as batch-size mismatches

Affected code:
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/deepfloyd_if/pipeline_if_img2img.py#L422-L449
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/deepfloyd_if/pipeline_if_superresolution.py#L539-L566
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/deepfloyd_if/pipeline_if_inpainting.py#L427-L489
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/deepfloyd_if/pipeline_if_img2img_superresolution.py#L576-L638
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/deepfloyd_if/pipeline_if_inpainting_superresolution.py#L576-L671

Problem:
The validators treat `np.ndarray.shape[0]` as batch size for every NumPy image. A normal single image shaped `(H, W, C)` is interpreted as batch size `H`, even though the preprocessors can handle single HWC arrays.

Impact:
Users passing valid single NumPy images get a misleading batch-size error. Fast tests only cover tensor batches and slow tests use PIL images, so this path is untested.

Reproduction:
```python
import numpy as np
import torch
from diffusers import DDPMScheduler, IFImg2ImgPipeline, UNet2DConditionModel

unet = UNet2DConditionModel(
sample_size=8, in_channels=3, out_channels=6, layers_per_block=1,
block_out_channels=(8,), down_block_types=("CrossAttnDownBlock2D",),
up_block_types=("CrossAttnUpBlock2D",), cross_attention_dim=4,
attention_head_dim=4, norm_num_groups=1,
)
pipe = IFImg2ImgPipeline(None, None, unet, DDPMScheduler(num_train_timesteps=10, variance_type="learned_range"), None, None, None, False)

pipe.check_inputs(
prompt=None,
image=np.zeros((8, 8, 3), dtype=np.float32),
batch_size=1,
callback_steps=1,
prompt_embeds=torch.zeros(1, 77, 4),
negative_prompt_embeds=torch.zeros(1, 77, 4),
)
```

Relevant precedent:
The local preprocessors already wrap non-list NumPy images and would handle HWC as a single image if validation allowed it.

Suggested fix:
```python
def _image_batch_size(image):
if isinstance(image, list):
return len(image)
if isinstance(image, PIL.Image.Image):
return 1
if isinstance(image, np.ndarray):
return image.shape[0] if image.ndim == 4 else 1
if isinstance(image, torch.Tensor):
return image.shape[0] if image.ndim == 4 else 1
raise TypeError(type(image))
```

## Issue 2: Batched NumPy masks are converted to 5D tensors

Affected code:
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/deepfloyd_if/pipeline_if_inpainting.py#L668-L713
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/deepfloyd_if/pipeline_if_inpainting_superresolution.py#L745-L791

Problem:
`preprocess_mask_image()` wraps a non-list NumPy mask in a list, then blindly applies `m[None, None, :]`. For a batched mask shaped `(B, H, W)`, this returns `(1, 1, B, H, W)` instead of `(B, 1, H, W)`.

Impact:
Batched NumPy masks either fail later during broadcasting or apply the mask with the wrong shape. This is not covered by the current tests.

Reproduction:
```python
import numpy as np
from diffusers import DDPMScheduler, IFInpaintingPipeline, UNet2DConditionModel

unet = UNet2DConditionModel(
sample_size=8, in_channels=3, out_channels=6, layers_per_block=1,
block_out_channels=(8,), down_block_types=("CrossAttnDownBlock2D",),
up_block_types=("CrossAttnUpBlock2D",), cross_attention_dim=4,
attention_head_dim=4, norm_num_groups=1,
)
pipe = IFInpaintingPipeline(None, None, unet, DDPMScheduler(num_train_timesteps=10, variance_type="learned_range"), None, None, None, False)

mask = np.zeros((2, 8, 8), dtype=np.float32)
print(pipe.preprocess_mask_image(mask).shape) # torch.Size([1, 1, 2, 8, 8])
```

Relevant precedent:
Tensor masks in the same method distinguish 2D single masks from 3D batched masks before adding channel dimensions.

Suggested fix:
```python
elif isinstance(mask_image[0], np.ndarray):
mask_image = np.stack(mask_image, axis=0) if len(mask_image) > 1 else mask_image[0]

if mask_image.ndim == 2:
mask_image = mask_image[None, None, :, :]
elif mask_image.ndim == 3:
mask_image = mask_image[:, None, :, :]
elif mask_image.ndim == 4 and mask_image.shape[-1] == 1:
mask_image = mask_image.transpose(0, 3, 1, 2)
else:
raise ValueError(f"Unsupported mask shape: {mask_image.shape}")

mask_image = (mask_image >= 0.5).astype(np.float32)
mask_image = torch.from_numpy(mask_image)
```

## Issue 3: `strength` is documented as constrained but invalid values are accepted

Affected code:
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/deepfloyd_if/pipeline_if_img2img.py#L378-L449
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/deepfloyd_if/pipeline_if_img2img.py#L627-L637
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/deepfloyd_if/pipeline_if_inpainting.py#L382-L489
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/deepfloyd_if/pipeline_if_img2img_superresolution.py#L531-L638
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/deepfloyd_if/pipeline_if_inpainting_superresolution.py#L533-L671

Problem:
The docs say `strength` must be between 0 and 1, but the four img2img/inpainting variants never validate it. Negative values can silently produce empty outputs, and values above 1 are clamped by timestep math and behave like 1.

Impact:
Invalid user input produces surprising generation behavior instead of a clear `ValueError`.

Reproduction:
```python
import torch
from diffusers import DDPMScheduler, IFImg2ImgPipeline, UNet2DConditionModel

unet = UNet2DConditionModel(
sample_size=8, in_channels=3, out_channels=6, layers_per_block=1,
block_out_channels=(8,), down_block_types=("CrossAttnDownBlock2D",),
up_block_types=("CrossAttnUpBlock2D",), cross_attention_dim=4,
attention_head_dim=4, norm_num_groups=1,
)
pipe = IFImg2ImgPipeline(None, None, unet, DDPMScheduler(num_train_timesteps=10, variance_type="learned_range"), None, None, None, False)

embeds = torch.zeros(1, 77, 4)
image = torch.zeros(1, 3, 8, 8)
out = pipe(prompt_embeds=embeds, negative_prompt_embeds=embeds, image=image, strength=-0.1, num_inference_steps=2, output_type="pt")
print(out.images.shape) # torch.Size([0, 3, 8, 8])
```

Relevant precedent:
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/stable_diffusion/pipeline_stable_diffusion_img2img.py#L656-L670

Suggested fix:
```python
def check_inputs(..., strength, ...):
if strength < 0 or strength > 1:
raise ValueError(f"The value of `strength` should be in [0.0, 1.0], but is {strength}")
```

## Issue 4: `IFPipelineOutput` is hidden behind torch+transformers lazy-import guards

Affected code:
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/deepfloyd_if/__init__.py#L24-L43

Problem:
`IFPipelineOutput` is a lightweight dataclass that only needs NumPy/PIL/BaseOutput, but it is added to `_import_structure` only when both torch and transformers are available.

Impact:
In dependency-light environments, users cannot import an output type that does not require the missing dependency.

Reproduction:
```python
import importlib
import sys
import diffusers.utils.import_utils as iu

iu._transformers_available = False
for name in list(sys.modules):
if name.startswith("diffusers.pipelines.deepfloyd_if"):
del sys.modules[name]

module = importlib.import_module("diffusers.pipelines.deepfloyd_if")
print(hasattr(module, "IFPipelineOutput")) # False
from diffusers.pipelines.deepfloyd_if import IFPipelineOutput # ImportError
```

Relevant precedent:
The `timesteps` constants in the same `__init__.py` are already exported outside the torch+transformers guard.

Suggested fix:
```python
_import_structure = {
"timesteps": [...],
"pipeline_output": ["IFPipelineOutput"],
}
```

## Issue 5: `encode_prompt()` detaches gradients in all IF pipelines

Affected code:
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/deepfloyd_if/pipeline_if.py#L168-L320
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/deepfloyd_if/pipeline_if_img2img.py#L192-L333
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/deepfloyd_if/pipeline_if_superresolution.py#L302-L455

Problem:
`encode_prompt()` is decorated with `@torch.no_grad()` across the copied IF variants. `__call__()` is already no-grad, so the helper-level decorator prevents advanced callers from using `encode_prompt()` with gradients enabled.

Impact:
Prompt-embedding optimization and training-style workflows cannot reuse the public helper without silently detaching tensors.

Reproduction:
```python
import torch
from diffusers import DDPMScheduler, IFPipeline, UNet2DConditionModel

unet = UNet2DConditionModel(
sample_size=8, in_channels=3, out_channels=6, layers_per_block=1,
block_out_channels=(8,), down_block_types=("CrossAttnDownBlock2D",),
up_block_types=("CrossAttnUpBlock2D",), cross_attention_dim=4,
attention_head_dim=4, norm_num_groups=1,
)
pipe = IFPipeline(None, None, unet, DDPMScheduler(num_train_timesteps=10, variance_type="learned_range"), None, None, None, False)

x = torch.randn(1, 77, 4, requires_grad=True)
prompt_embeds, _ = pipe.encode_prompt(prompt=None, do_classifier_free_guidance=False, prompt_embeds=x, num_images_per_prompt=2)
print(x.requires_grad, prompt_embeds.requires_grad) # True False
```

Relevant precedent:
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/flux/pipeline_flux.py#L311
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/flux/pipeline_flux.py#L652

Suggested fix:
```python
# Remove @torch.no_grad() from encode_prompt() in all IF pipeline copies.
# Keep @torch.no_grad() on __call__().
```

## Issue 6: `T5FilmDecoder` has no direct fast or slow tests

Affected code:
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/models/transformers/t5_film_transformer.py#L25-L146

Problem:
No tests under `tests/` mention `T5FilmDecoder`. This leaves config serialization, save/load, forward shape, dtype behavior, and attention processor behavior unexercised for the model file in scope.

Impact:
Regressions in the model can land without fast model-test coverage or slow checkpoint smoke coverage.

Reproduction:
```python
from pathlib import Path

hits = [str(p) for p in Path("tests").rglob("*.py") if "T5FilmDecoder" in p.read_text(encoding="utf-8")]
print(hits) # []
assert hits, "No tests mention T5FilmDecoder"
```

Relevant precedent:
Other transformer model families have direct tests under `tests/models/transformers/`.

Suggested fix:
Add a small `tests/models/transformers/test_models_t5_film_transformer.py` covering tiny config construction, forward pass, save/load, and dtype/device movement. Add slow coverage only if there is a maintained pretrained `T5FilmDecoder` checkpoint to smoke-test.

Guida per i contributori

Apri la guida per i contributori

Direzione di ricerca

Inizia separando i sei risultati, quindi esamina i file della pipeline deepfloyd_if collegati, __init__.py e t5_film_transformer.py insieme ai test esistenti. Esegui le riproduzioni fornite per NumPy, strength, import e gradient, e cerca T5FilmDecoder in tests/. Il lavoro è completato quando ogni comportamento confermato dispone di una copertura mirata e il comportamento atteso documentato è stato ripristinato senza regressioni.

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

Valutazione

Stack tecnologico
python, pytorch
Ambito
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
35/100

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.