huggingface / huggingface/diffusers
kandinsky3 model/pipeline review
- Vorherrschende Sprache
- Python
- Sterne
- 34.5k
- Forks
- 7.3k
- Ø Merge
- 3 T. 3 Std.
- Gemergte PRs (30 T.)
- 91
Beschreibung
# `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.
Beitragsleitfaden
Rechercherichtung
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.
Vom Indexierungsmodell aus dem Issue-Text verfasst.
Bewertung
- Tech-Stack
- python, pytorch
- Bereich
- machine-learning
- Issue-Typ
- Bug
- Schwierigkeit
- 5/5
- Geschätzter Aufwand
- Über eine Woche
- Aktivitätsstatus
- Ruhig
- Klarheit
- Größtenteils klar
- Anfängerfreundlichkeit
- 35/100