huggingface / huggingface/diffusers
marigold model/pipeline review
- 主要言語
- 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