huggingface / huggingface/diffusers

deepfloyd_if model/pipeline review

Open
#13,646 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Python
Stars
34.5k
Forks
7.3k
Avg merge
3d 3h
Merged PRs (30d)
91

Description

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

Contributor guide

Open the contributing guide

Research direction

Start by separating the six findings, then inspect the linked deepfloyd_if pipeline files, __init__.py, and t5_film_transformer.py alongside the existing tests. Run the supplied NumPy, strength, import, and gradient reproductions, and search tests/ for T5FilmDecoder. Done means each confirmed behavior has targeted coverage and the documented expected behavior is restored without regressions.

Written by the indexing model from the issue text.

Assessment

Tech stack
python, pytorch
Domain
machine-learning, testing-qa
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.