huggingface / huggingface/diffusers
deepfloyd_if model/pipeline review
- 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
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