huggingface / huggingface/diffusers

marigold model/pipeline review

オープン
#13,628 コメント 0 件 リアクション 0 件 担当者 0 名 GitHub で見る
主要言語
Python
スター
34.5k
フォーク
7.3k
平均マージ
3日 3時間
マージ済み PR(30日)
91

説明

# `marigold` model/pipeline review

Commit tested: `0f1abc4ae8b0eb2a3b40e82a310507281144c423`

Review performed against the repository review rules.

## Issue 1: NumPy HWC images validate returned latents against the wrong shape

Affected code:
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/marigold/pipeline_marigold_depth.py#L273-L312
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/marigold/pipeline_marigold_intrinsics.py#L285-L324
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/marigold/pipeline_marigold_normals.py#L258-L297

Problem:
`check_inputs()` reads every tensor/array size with `img.shape[-2:]`. That is correct for torch CHW/NCHW, but wrong for NumPy HWC/NHWC. A valid latent for a `(32, 64, 3)` NumPy image should be `(1, 4, 4, 8)`, but validation expects `(1, 4, 8, 1)`.

Impact:
Users following the documented `output_latent=True` reuse path can round-trip PIL/torch inputs, but NumPy image inputs reject their own valid latents or allow invalid latents that fail later in denoising.

Reproduction:
```python
import numpy as np
import torch
from types import SimpleNamespace
from diffusers import MarigoldDepthPipeline

pipe = object.__new__(MarigoldDepthPipeline)
pipe.vae_scale_factor = 8
pipe.vae = SimpleNamespace(config=SimpleNamespace(block_out_channels=[1, 1, 1, 1], latent_channels=4))
pipe.scale_invariant = False
pipe.shift_invariant = False

pipe.check_inputs(
image=np.zeros((32, 64, 3), dtype=np.float32),
num_inference_steps=1,
ensemble_size=1,
processing_resolution=0,
resample_method_input="bilinear",
resample_method_output="bilinear",
batch_size=1,
ensembling_kwargs=None,
latents=torch.zeros(1, 4, 4, 8),
generator=None,
output_type="pt",
output_uncertainty=False,
)
```

Relevant precedent:
`MarigoldImageProcessor.load_image_canonical()` already treats NumPy as HWC/NHWC before converting to NCHW.
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/marigold/marigold_image_processing.py#L175-L185

Suggested fix:
```python
if isinstance(img, np.ndarray):
if img.ndim == 2:
H_i, W_i = img.shape
N_i = 1
elif img.ndim == 3:
H_i, W_i = img.shape[:2]
N_i = 1
else:
N_i, H_i, W_i = img.shape[:3]
else:
H_i, W_i = img.shape[-2:]
N_i = img.shape[0] if img.ndim == 4 else 1
```

## Issue 2: Generator lists are reused unsliced across Marigold manual batches

Affected code:
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/marigold/pipeline_marigold_depth.py#L514-L528
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/marigold/pipeline_marigold_intrinsics.py#L520-L534
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/marigold/pipeline_marigold_normals.py#L491-L505

Problem:
The pipelines validate a generator list of length `num_images * ensemble_size`, then process predictions in smaller manual batches. Each scheduler step receives the full list instead of the current slice. `LCMScheduler.step()` samples per-step noise, and `randn_tensor()` uses only the first `shape[0]` generators, so later batches reuse the wrong generators.

Impact:
Batched Marigold LCM inference is not equivalent to separate seeded calls when `batch_size < num_images * ensemble_size` and `num_inference_steps > 1`.

Reproduction:
```python
import torch
from transformers import CLIPTextConfig, CLIPTextModel, CLIPTokenizer
from diffusers import AutoencoderTiny, LCMScheduler, MarigoldDepthPipeline, UNet2DConditionModel

def make_pipe():
torch.manual_seed(0)
unet = UNet2DConditionModel(
block_out_channels=(32, 64), layers_per_block=1, sample_size=4,
in_channels=8, out_channels=4,
down_block_types=("DownBlock2D", "CrossAttnDownBlock2D"),
up_block_types=("CrossAttnUpBlock2D", "UpBlock2D"),
cross_attention_dim=32,
)
vae = AutoencoderTiny(in_channels=3, out_channels=3, latent_channels=4)
scheduler = LCMScheduler(prediction_type="v_prediction", beta_schedule="scaled_linear")
text_encoder = CLIPTextModel(CLIPTextConfig(
bos_token_id=0, eos_token_id=2, hidden_size=32, intermediate_size=37,
num_attention_heads=4, num_hidden_layers=1, pad_token_id=1, vocab_size=1000,
))
tokenizer = CLIPTokenizer.from_pretrained("hf-internal-testing/tiny-random-clip")
pipe = MarigoldDepthPipeline(unet, vae, scheduler, text_encoder, tokenizer, "depth", True, True).to("cpu")
pipe.set_progress_bar_config(disable=True)
return pipe

def gen(seed):
return torch.Generator(device="cpu").manual_seed(seed)

image = torch.full((1, 3, 32, 32), 0.5)
batched = make_pipe()(image=[image[0], image[0]], num_inference_steps=2, processing_resolution=0,
batch_size=1, generator=[gen(0), gen(1)], output_type="pt").prediction
single_seed_1 = make_pipe()(image=image, num_inference_steps=2, processing_resolution=0,
generator=gen(1), output_type="pt").prediction

print((batched[1] - single_seed_1[0]).abs().max().item()) # non-zero
```

Relevant precedent:
`randn_tensor()` consumes generator lists by batch position, so callers must pass a list matching the current batch.
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/utils/torch_utils.py#L167-L195

Suggested fix:
```python
batch_generator = generator
if isinstance(generator, list):
batch_generator = generator[i : i + effective_batch_size]

batch_pred_latent = self.scheduler.step(
noise, t, batch_pred_latent, generator=batch_generator
).prev_sample
```

## Issue 3: Absolute depth ensembling is documented but always raises

Affected code:
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/marigold/pipeline_marigold_depth.py#L806-L827

Problem:
The `ensemble_depth()` docstring says absolute predictions (`scale_invariant=False`, `shift_invariant=False`) skip alignment and only ensemble, but the post-ensemble normalization branch raises `ValueError("Unrecognized alignment.")` whenever `scale_invariant` is false.

Impact:
Any absolute-depth Marigold checkpoint config can run single predictions, but `ensemble_size > 1` crashes.

Reproduction:
```python
import torch
from diffusers import MarigoldDepthPipeline

depth = torch.rand(3, 1, 8, 8)
MarigoldDepthPipeline.ensemble_depth(
depth,
scale_invariant=False,
shift_invariant=False,
output_uncertainty=True,
reduction="mean",
)
```

Relevant precedent:
The method’s own docstring describes absolute-prediction ensembling as supported.
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/marigold/pipeline_marigold_depth.py#L685-L691

Suggested fix:
```python
if scale_invariant:
depth_max = depth.max()
depth_min = depth.min() if shift_invariant else 0
depth_range = (depth_max - depth_min).clamp(min=1e-6)
depth = (depth - depth_min) / depth_range
if output_uncertainty:
uncertainty /= depth_range
```

## Issue 4: Visualization helpers advertise list[np.ndarray] but list paths assume torch tensors

Affected code:
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/marigold/marigold_image_processing.py#L487-L538
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/marigold/marigold_image_processing.py#L543-L626
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/marigold/marigold_image_processing.py#L631-L670

Problem:
`visualize_normals()`, `visualize_intrinsics()`, and `visualize_uncertainty()` accept `list[np.ndarray]` in their annotations/docstrings, but their list branches call helpers that immediately use tensor-only methods like `.permute()`.

Impact:
Batch arrays work, but equivalent lists of arrays fail with `AttributeError`, which is a public API mismatch for post-processing utilities.

Reproduction:
```python
import numpy as np
from diffusers.pipelines.marigold import MarigoldImageProcessor

MarigoldImageProcessor.visualize_normals([np.zeros((4, 4, 3), dtype=np.float32)])
```

Relevant precedent:
`visualize_depth()` handles list elements individually, and the non-list branches of these helpers already know how to convert NumPy arrays.
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/marigold/marigold_image_processing.py#L415-L446

Suggested fix:
```python
elif isinstance(normals, list):
return [
out
for item in normals
for out in MarigoldImageProcessor.visualize_normals(item, flip_x=flip_x, flip_y=flip_y, flip_z=flip_z)
]
```

Apply the same recursive list handling to `visualize_intrinsics()` and `visualize_uncertainty()`.

## Duplicate-search status

Searched GitHub issues and PRs for `marigold`, the affected class/function/file names, and the specific failure modes above. I found broad Marigold integration/docs items, but no duplicate issues or PRs for these four findings.

## Test coverage status

Fast and slow tests exist for depth, normals, and intrinsics under `tests/pipelines/marigold/`. The gaps are the cases above: NumPy HWC latent reuse, generator-list batching with multi-step LCM, absolute-depth ensembling, and list-of-NumPy visualization inputs.

コントリビューションガイド

コントリビューションガイドを開く

調査の方向性

Start with the affected check_inputs(), batching, ensemble_depth(), and visualization methods in the three Marigold pipeline files and marigold_image_processing.py. Review the existing tests under tests/pipelines/marigold/ and add coverage for NumPy latent reuse, generator-list batching, absolute-depth ensembling, and list-of-NumPy visualization inputs. Done means all four reproductions work and the existing fast and slow Marigold tests pass.

索引モデルが issue の本文から書いたものです。

評価

技術スタック
python, pytorch
領域
computer-vision, machine-learning, testing-qa
issue の種類
バグ
難易度
4/5
見積もり時間
3〜5日
活発さ
静か
明瞭さ
明確に書かれている
初心者へのやさしさ
46/100

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。