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