huggingface / huggingface/diffusers

ddpm model/pipeline review

Offen
#13,649 0 Kommentare 0 Reaktionen 0 zugewiesene Personen Auf GitHub ansehen
Vorherrschende Sprache
Python
Sterne
34.5k
Forks
7.3k
Ø Merge
3 T. 3 Std.
Gemergte PRs (30 T.)
91

Beschreibung

# `ddpm` model/pipeline review

Commit tested: `0f1abc4ae8b0eb2a3b40e82a310507281144c423`

Review performed against the repository review rules.

Coverage status: fast DDPM tests exist, and a slow accelerator integration test exists. Local run: `2 passed, 1 skipped` for `tests/pipelines/ddpm/test_ddpm.py`; the skipped test is marked `@slow`.

## Issue 1: `DDPMPipeline` does not run latents on the offload execution device

Affected code:
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/ddpm/pipeline_ddpm.py#L111-L139

Problem:
`DDPMPipeline` declares `model_cpu_offload_seq = "unet"` but initializes `image` on `self.device`, not `self._execution_device`, and never calls `self.maybe_free_model_hooks()` before returning. Under `enable_model_cpu_offload()`, `self.device` remains CPU while the UNet forward is executed on the accelerator by accelerate hooks. That leaves the scheduler step operating on mismatched CPU/GPU tensors.

Duplicate search:
No duplicate found for `DDPMPipeline enable_model_cpu_offload`, `DDPMPipeline maybe_free_model_hooks`, or `DDPMPipeline _execution_device`.

Impact:
Users enabling CPU offload can hit device mismatch failures or leave the UNet resident on the accelerator after the call, defeating the offload contract.

Reproduction:
```python
import torch
from diffusers import DDPMPipeline, DDPMScheduler, UNet2DModel

if not torch.cuda.is_available():
raise SystemExit("CUDA/accelerator required")

unet = UNet2DModel(
block_out_channels=(4, 8),
layers_per_block=1,
norm_num_groups=4,
sample_size=8,
in_channels=3,
out_channels=3,
down_block_types=("DownBlock2D", "AttnDownBlock2D"),
up_block_types=("AttnUpBlock2D", "UpBlock2D"),
)
pipe = DDPMPipeline(unet=unet, scheduler=DDPMScheduler(num_train_timesteps=2))
pipe.set_progress_bar_config(disable=True)
pipe.enable_model_cpu_offload(device="cuda")

pipe(num_inference_steps=1, output_type="np")
```

Relevant precedent:
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/ddim/pipeline_ddim.py#L150
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/pipeline_utils.py#L1284-L1294

Suggested fix:
```python
device = self._execution_device

if device.type == "mps":
image = randn_tensor(image_shape, generator=generator, dtype=self.unet.dtype)
image = image.to(device)
else:
image = randn_tensor(image_shape, generator=generator, device=device, dtype=self.unet.dtype)

# ...

if output_type == "pil":
image = self.numpy_to_pil(image)

self.maybe_free_model_hooks()
```

## Issue 2: Generator lists are not validated against `batch_size`

Affected code:
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/ddpm/pipeline_ddpm.py#L111-L116

Problem:
`DDPMPipeline.__call__` accepts `generator: torch.Generator | list[torch.Generator]`, but it does not validate list length. A one-element list for a larger batch is silently treated like a single generator, and other short lists can fail with a raw `IndexError`.

Duplicate search:
No duplicate found for `DDPMPipeline generator list` or `DDPMPipeline generator list length`.

Impact:
Per-sample seeding behaves inconsistently and invalid inputs do not produce the clear `ValueError` users get from related pipelines.

Reproduction:
```python
import torch
from diffusers import DDPMPipeline, DDPMScheduler, UNet2DModel

unet = UNet2DModel(
block_out_channels=(4, 8),
layers_per_block=1,
norm_num_groups=4,
sample_size=8,
in_channels=3,
out_channels=3,
down_block_types=("DownBlock2D", "AttnDownBlock2D"),
up_block_types=("AttnUpBlock2D", "UpBlock2D"),
)
pipe = DDPMPipeline(unet=unet, scheduler=DDPMScheduler(num_train_timesteps=2))
pipe.set_progress_bar_config(disable=True)

pipe(
batch_size=3,
generator=[torch.Generator(device="cpu").manual_seed(i) for i in range(2)],
num_inference_steps=1,
output_type="np",
)
```

Relevant precedent:
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/ddim/pipeline_ddim.py#L144-L150
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/consistency_models/pipeline_consistency_models.py#L96-L100

Suggested fix:
```python
if isinstance(generator, list) and len(generator) != batch_size:
raise ValueError(
f"You have passed a list of generators of length {len(generator)}, but requested an effective batch"
f" size of {batch_size}. Make sure the batch size matches the length of the generators."
)
```

## Issue 3: Invalid `output_type` values silently return NumPy

Affected code:
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/ddpm/pipeline_ddpm.py#L131-L134

Problem:
Any `output_type` other than `"pil"` falls through to NumPy output. This includes typos and common values like `"pt"`, with no warning or error.

Duplicate search:
No duplicate found for `DDPMPipeline output_type` or `DDPMPipeline invalid output_type`.

Impact:
A misspelled output type can silently change behavior, making downstream code receive `np.ndarray` when the caller expected a different format or a validation error.

Reproduction:
```python
from diffusers import DDPMPipeline, DDPMScheduler, UNet2DModel

unet = UNet2DModel(
block_out_channels=(4, 8),
layers_per_block=1,
norm_num_groups=4,
sample_size=8,
in_channels=3,
out_channels=3,
down_block_types=("DownBlock2D", "AttnDownBlock2D"),
up_block_types=("AttnUpBlock2D", "UpBlock2D"),
)
pipe = DDPMPipeline(unet=unet, scheduler=DDPMScheduler(num_train_timesteps=2))
pipe.set_progress_bar_config(disable=True)

images = pipe(num_inference_steps=1, output_type="definitely-not-valid").images
print(type(images).__name__) # ndarray
```

Relevant precedent:
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/consistency_models/pipeline_consistency_models.py#L111-L124
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/image_processor.py#L764-L779

Suggested fix:
```python
if output_type not in ["pt", "np", "pil"]:
raise ValueError("output_type must be one of ['pt', 'np', or 'pil'].")

image = (image / 2 + 0.5).clamp(0, 1)
if output_type == "pt":
pass
else:
image = image.cpu().permute(0, 2, 3, 1).numpy()
if output_type == "pil":
image = self.numpy_to_pil(image)
```

Beitragsleitfaden

Beitragsleitfaden öffnen

Rechercherichtung

Start with src/diffusers/pipelines/ddpm/pipeline_ddpm.py and compare the referenced DDIM and pipeline utility implementations. Run tests/pipelines/ddpm/test_ddpm.py, including the slow accelerator case, then add coverage for offload execution, generator-list length validation, and invalid output types. Done means the regressions are handled with clear validation and existing behavior remains covered.

Vom Indexierungsmodell aus dem Issue-Text verfasst.

Bewertung

Tech-Stack
python, pytorch
Bereich
machine-learning
Issue-Typ
Bug
Schwierigkeit
3/5
Geschätzter Aufwand
1-2 Tage
Aktivitätsstatus
Ruhig
Klarheit
Klar beschrieben
Anfängerfreundlichkeit
68/100

Neue Issues direkt in Ihr Postfach

Eine kurze Übersicht über anfängerfreundliche GitHub-Issues.