huggingface / huggingface/diffusers

kandinsky3 model/pipeline review

Ouverte
#13,640 0 commentaires 0 réactions 0 personnes assignées Voir sur GitHub
Langage dominant
Python
Étoiles
34.5k
Forks
7.3k
Merge moyen
3 j 3 h
PR mergées (30 j)
91

Description

# `kandinsky3` model/pipeline review

Commit tested: `0f1abc4ae8b0eb2a3b40e82a310507281144c423`

Review performed against the repository review rules.

## Issue 1: `Kandinsky3Img2ImgPipeline` re-encodes latent image inputs

Affected code:
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/kandinsky3/pipeline_kandinsky3_img2img.py#L559-L565

Problem:
`VaeImageProcessor.preprocess()` returns 4-channel latent tensors unchanged, but the pipeline then always calls `self.movq.encode(image)`. A user-provided latent tensor is sent into a 3-channel image encoder and fails.

Impact:
Direct latent img2img inputs are unusable, and the latent branch in `prepare_latents()` is effectively unreachable from `__call__`.

Reproduction:
```python
import torch
from diffusers import Kandinsky3Img2ImgPipeline, VQModel
from diffusers.schedulers import DDPMScheduler

movq = VQModel(block_out_channels=[32], down_block_types=["DownEncoderBlock2D"], up_block_types=["UpDecoderBlock2D"], in_channels=3, out_channels=3, latent_channels=4, layers_per_block=1, norm_num_groups=8, num_vq_embeddings=12, vq_embed_dim=4)
pipe = Kandinsky3Img2ImgPipeline(None, None, None, DDPMScheduler(num_train_timesteps=4), movq)
pipe(prompt_embeds=torch.ones(1, 2, 4), attention_mask=torch.ones(1, 2, dtype=torch.long), image=torch.randn(1, 4, 8, 8), guidance_scale=1.0, num_inference_steps=1, output_type="latent")
```

Relevant precedent:
`VaeImageProcessor.preprocess()` intentionally returns latent-channel tensors unchanged:
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/image_processor.py#L712-L715

Suggested fix:
```python
if image.shape[1] == self.movq.config.latent_channels:
latents = image
else:
latents = self.movq.encode(image)["latents"]
latents = latents.repeat_interleave(num_images_per_prompt, dim=0)
```

## Issue 2: `strength=0.0` returns an empty batch

Affected code:
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/kandinsky3/pipeline_kandinsky3_img2img.py#L143-L150
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/kandinsky3/pipeline_kandinsky3_img2img.py#L562-L568

Problem:
The docs say `strength` is between 0 and 1, but `strength=0.0` produces an empty timestep tensor. That empty tensor is used as `latent_timestep`, and the pipeline returns an empty latent batch instead of preserving the input image or raising.

Impact:
A boundary value accepted by the public API silently returns `torch.Size([0, ...])`.

Reproduction:
```python
import torch
from diffusers import Kandinsky3Img2ImgPipeline, VQModel
from diffusers.schedulers import DDPMScheduler

movq = VQModel(block_out_channels=[32], down_block_types=["DownEncoderBlock2D"], up_block_types=["UpDecoderBlock2D"], in_channels=3, out_channels=3, latent_channels=4, layers_per_block=1, norm_num_groups=8, num_vq_embeddings=12, vq_embed_dim=4)
pipe = Kandinsky3Img2ImgPipeline(None, None, None, DDPMScheduler(num_train_timesteps=4), movq)
out = pipe(prompt_embeds=torch.ones(1, 2, 4), attention_mask=torch.ones(1, 2, dtype=torch.long), image=torch.rand(1, 3, 8, 8), strength=0.0, guidance_scale=1.0, num_inference_steps=2, output_type="latent")
print(out.images.shape) # torch.Size([0, 4, 8, 8])
```

Relevant precedent:
Stable Diffusion img2img validates the public strength range:
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/stable_diffusion/pipeline_stable_diffusion_img2img.py#L656-L669

Suggested fix:
Handle `strength == 0` explicitly by returning the initial latents/image without denoising, or reject it:
```python
if strength <= 0 or strength > 1:
raise ValueError(f"The value of strength should be in (0.0, 1.0], but is {strength}")
```

## Issue 3: `Kandinsky3UNet` crashes when `encoder_attention_mask` is omitted

Affected code:
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/models/unets/unet_kandinsky3.py#L149-L152
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/models/unets/unet_kandinsky3.py#L431-L433

Problem:
`encoder_attention_mask` defaults to `None`, but `Kandinsky3AttentionPooling.forward()` unconditionally calls `context_mask.to(...)`.

Impact:
Direct model users cannot rely on the optional mask default, and model-level tests do not catch it because the pipelines always pass masks.

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

m = Kandinsky3UNet(in_channels=4, time_embedding_dim=4, groups=2, attention_head_dim=4, layers_per_block=1, block_out_channels=(32, 64), cross_attention_dim=4, encoder_hid_dim=32)
m(torch.randn(1, 4, 8, 8), torch.tensor(1), encoder_hidden_states=torch.randn(1, 2, 32), return_dict=False)
```

Relevant precedent:
Generic attention processors accept `attention_mask=None`.

Suggested fix:
```python
def forward(self, x, context, context_mask=None):
if context_mask is not None:
context_mask = context_mask.to(dtype=context.dtype)
context = self.attention(context.mean(dim=1, keepdim=True), context, context_mask)
return x + context.squeeze(1)
```

## Issue 4: Tuple-valued UNet config annotations are not implemented

Affected code:
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/models/unets/unet_kandinsky3.py#L56-L58
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/models/unets/unet_kandinsky3.py#L88-L90

Problem:
`layers_per_block` and `cross_attention_dim` are typed as accepting tuples, but tuple values fail during construction. `layers_per_block` is copied as a tuple into every level, and `cross_attention_dim` is passed to `nn.Linear` as a tuple.

Impact:
The serialized/public config surface advertises values that cannot be loaded.

Reproduction:
```python
from diffusers import Kandinsky3UNet

base = dict(in_channels=4, time_embedding_dim=4, groups=2, attention_head_dim=4, block_out_channels=(32, 64), encoder_hid_dim=32)
Kandinsky3UNet(**base, layers_per_block=(1, 1), cross_attention_dim=4)
Kandinsky3UNet(**base, layers_per_block=1, cross_attention_dim=(4, 4))
```

Relevant precedent:
`UNet2DConditionModel` expands scalar-or-tuple config fields before constructing blocks:
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/models/unets/unet_2d_condition.py#L337-L344

Suggested fix:
Support tuple `layers_per_block`, and either implement per-block context projections or reject tuple `cross_attention_dim` explicitly:
```python
if isinstance(layers_per_block, int):
num_blocks = [layers_per_block] * len(block_out_channels)
else:
if len(layers_per_block) != len(block_out_channels):
raise ValueError("`layers_per_block` must match `block_out_channels`.")
num_blocks = list(layers_per_block)

if not isinstance(cross_attention_dim, int):
raise ValueError("`Kandinsky3UNet` currently supports only an integer `cross_attention_dim`.")
```

## Issue 5: Deprecated `callback` crashes when `callback_steps` is omitted

Affected code:
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/kandinsky3/pipeline_kandinsky3.py#L430-L431
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/kandinsky3/pipeline_kandinsky3.py#L555-L559
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/kandinsky3/pipeline_kandinsky3_img2img.py#L488-L489
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/kandinsky3/pipeline_kandinsky3_img2img.py#L613-L617

Problem:
`callback_steps` defaults to `None`, but the loop evaluates `i % callback_steps` whenever deprecated `callback` is passed.

Impact:
The deprecated callback API is still present until 1.0.0 but fails unless users also know to pass deprecated `callback_steps`.

Reproduction:
```python
import torch
from diffusers import Kandinsky3Pipeline
from diffusers.schedulers import DDPMScheduler

class ZeroUNet(torch.nn.Module):
@property
def dtype(self): return torch.float32
@property
def device(self): return torch.device("cpu")
def forward(self, sample, *args, **kwargs): return (torch.zeros_like(sample),)

pipe = Kandinsky3Pipeline(None, None, ZeroUNet(), DDPMScheduler(num_train_timesteps=4), None)
pipe(prompt_embeds=torch.ones(1, 2, 4), attention_mask=torch.ones(1, 2, dtype=torch.long), guidance_scale=1.0, height=8, width=8, num_inference_steps=1, output_type="latent", callback=lambda step, timestep, latents: None)
```

Relevant precedent:
Deprecated arguments should continue to work until the removal version.

Suggested fix:
```python
callback = kwargs.pop("callback", None)
callback_steps = kwargs.pop("callback_steps", 1 if callback is not None else None)
```

## Issue 6: `encode_prompt()` is decorated with `@torch.no_grad()`

Affected code:
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/kandinsky3/pipeline_kandinsky3.py#L91-L92
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/kandinsky3/pipeline_kandinsky3_img2img.py#L106-L107

Problem:
The review rules call out helper-level `@torch.no_grad()` as incorrect because `__call__` already owns inference no-grad behavior. Keeping it on `encode_prompt()` blocks advanced callers from using gradients for prompt embedding workflows.

Impact:
Public helper behavior is less flexible than related modern pipelines.

Reproduction:
```python
import torch
from types import SimpleNamespace
from diffusers import Kandinsky3Pipeline

class Tokenizer:
def __call__(self, prompt, max_length=None, **kwargs):
return SimpleNamespace(input_ids=torch.arange(max_length).unsqueeze(0), attention_mask=torch.ones(1, max_length, dtype=torch.long))

class TextEncoder(torch.nn.Module):
def __init__(self):
super().__init__()
self.emb = torch.nn.Embedding(128, 4)
@property
def dtype(self): return self.emb.weight.dtype
def forward(self, input_ids, attention_mask=None): return (self.emb(input_ids),)

pipe = Kandinsky3Pipeline(Tokenizer(), TextEncoder(), None, None, None)
print(pipe.encode_prompt("x", do_classifier_free_guidance=False, device="cpu")[0].requires_grad) # False
```

Relevant precedent:
`FluxPipeline.encode_prompt()` is not decorated, while `__call__` is:
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/flux/pipeline_flux.py#L311-L316
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/flux/pipeline_flux.py#L652-L654

Suggested fix:
Remove `@torch.no_grad()` from both `encode_prompt()` methods.

## Issue 7: Kandinsky3 conversion script constructs the UNet with a positional config dict

Affected code:
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/kandinsky3/convert_kandinsky3_unet.py#L79-L86

Problem:
`Kandinsky3UNet(config)` passes the dict as `in_channels`, so construction fails inside convolution/group norm setup.

Impact:
The checked-in converter cannot run from its CLI path.

Reproduction:
```python
from diffusers import Kandinsky3UNet
Kandinsky3UNet({})
```

Relevant precedent:
Top-level model constructors should receive config values as keyword arguments.

Suggested fix:
```python
config = {}
unet = Kandinsky3UNet(**config)
unet.load_state_dict(converted_state_dict, strict=True)
```

## Coverage and duplicate-search status

Public imports, lazy loading, auto-pipeline mappings, dummy objects, pipeline runtime paths, UNet config/runtime behavior, docs, fast tests, and slow tests were reviewed.

Slow tests are present for text2image and img2img in `tests/pipelines/kandinsky3/`. There are no standalone `tests/models` tests for `Kandinsky3UNet`; coverage is pipeline-only.

Attempted fast pytest targets in `.venv`, but collection failed before the tests ran because this Torch install lacks `torch._C._distributed_c10d`.

Duplicate search performed with `gh search issues --include-prs` against `huggingface/diffusers` for `kandinsky3`, `Kandinsky3UNet`, `convert_kandinsky3_unet`, `Kandinsky3Img2ImgPipeline strength`, `callback_steps`, `latent image`, `encode_prompt no_grad`, `layers_per_block`, `encoder_attention_mask`, and `context_mask`. I found related historical items, including https://github.com/huggingface/diffusers/issues/5963, https://github.com/huggingface/diffusers/pull/11080, https://github.com/huggingface/diffusers/pull/12474, and https://github.com/huggingface/diffusers/pull/12544, but no direct duplicate for the issues above.

Guide de contribution

Ouvrir le guide de contribution

Piste de recherche

Start with the supplied reproductions and the affected files under src/diffusers/pipelines/kandinsky3/, src/diffusers/models/unets/unet_kandinsky3.py, and the conversion script. Check each reported failure against the referenced precedents before making changes. Done means all seven Kandinsky3 issues are addressed and the supplied reproductions no longer fail.

Rédigé par le modèle d'indexation à partir du texte de l'issue.

Évaluation

Stack technique
python, pytorch
Domaine
machine-learning
Type d'issue
Bug
Difficulté
5/5
Temps estimé
Plus d'une semaine
Activité
Calme
Clarté
Plutôt claire
Accessibilité débutants
35/100

Recevez les nouvelles issues par e-mail

Un résumé court des issues GitHub adaptées aux débutants.