huggingface / huggingface/diffusers

marigold model/pipeline review

Open
#13,628 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Python
Stars
34.5k
Forks
7.3k
Avg merge
3d 3h
Merged PRs (30d)
91

Description

# `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.

Contributor guide

Open the contributing guide

Research direction

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.

Written by the indexing model from the issue text.

Assessment

Tech stack
python, pytorch
Domain
computer-vision, machine-learning, testing-qa
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
46/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.