huggingface / huggingface/diffusers

kandinsky3 model/pipeline review

Đang mở
#13,640 0 bình luận 0 reaction 0 người được giao Xem trên GitHub
Ngôn ngữ chính
Python
Star
34.5k
Fork
7.3k
Merge trung bình
3 ngày 3 giờ
Pull request đã merge (30 ngày)
91

Mô tả

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

Hướng dẫn đóng góp

Mở hướng dẫn đóng góp

Hướng nghiên cứu

Bắt đầu với các bản tái hiện được cung cấp và các tệp bị ảnh hưởng trong src/diffusers/pipelines/kandinsky3/, src/diffusers/models/unets/unet_kandinsky3.py và tập lệnh chuyển đổi. Kiểm tra từng lỗi được báo cáo đối chiếu với các tiền lệ được tham chiếu trước khi thực hiện thay đổi. Được xem là hoàn tất khi cả bảy vấn đề của Kandinsky3 đã được xử lý và các bản tái hiện được cung cấp không còn thất bại.

Do mô hình lập chỉ mục viết ra từ nội dung của issue.

Đánh giá

Công nghệ
python, pytorch
Lĩnh vực
machine-learning
Loại issue
Lỗi
Độ khó
5/5
Thời gian dự kiến
Hơn một tuần
Mức độ hoạt động
Ít trao đổi
Độ rõ ràng
Khá rõ ràng
Mức phù hợp với người mới
35/100

Nhận issue mới trong hộp thư của bạn

Bản tóm tắt ngắn những issue GitHub phù hợp với người mới.