huggingface / huggingface/diffusers

kolors model/pipeline review

Ouverte
#13,612 0 commentaires 0 réactions 0 personnes assignées Voir sur GitHub

Personne n'a encore pris cette issue.

Langage dominant
Python
Étoiles
34.5k
Forks
7.3k
Merge moyen
3 j 3 h
PR mergées (30 j)
91

Description

# `kolors` model/pipeline review

Commit tested: `0f1abc4ae8b0eb2a3b40e82a310507281144c423`

Review performed against the repository review rules.

Target files reviewed: `kolors/__init__.py`, `pipeline_kolors.py`, `pipeline_kolors_img2img.py`, `pipeline_output.py`, `text_encoder.py`, `tokenizer.py`, plus Kolors public exports, docs/examples references, and tests.

Duplicate search status: searched GitHub Issues and PRs for `kolors`, affected class/function/file names, and the specific failure modes below. Existing related items found: sentencepiece import issue #9034, Kolors from-single-file issue #10207 / PR #10215, and Kolors LoRA PR #11198. No duplicate found for the specific prompt-embedding, `max_sequence_length`, img2img offload, output export, or `original_rope` findings below.

Test note: targeted reproductions ran under `.venv`. Full fast-test collection with `python -m pytest tests/pipelines/kolors/test_kolors.py tests/pipelines/kolors/test_kolors_img2img.py -q` failed in this local environment because the installed torch build lacks `torch._C._distributed_c10d`.

## Issue 1: `KolorsPipelineOutput` is not exported from `diffusers.pipelines.kolors`

Affected code:
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/kolors/__init__.py#L15-L28
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/kolors/pipeline_output.py#L10-L20

Problem:
`KolorsPipelineOutput` is defined and referenced in docstrings as `~pipelines.kolors.KolorsPipelineOutput`, but `kolors/__init__.py` never adds `pipeline_output` to `_import_structure`.

Impact:
Public subpackage import fails and autodoc cross-references can resolve inconsistently.

Reproduction:
```python
from diffusers.pipelines.kolors import KolorsPipelineOutput
# ImportError: cannot import name 'KolorsPipelineOutput'
```

Relevant precedent:
`stable_diffusion`, `stable_diffusion_xl`, `qwenimage`, and `flux` export their pipeline output classes from the subpackage `__init__.py`.

Suggested fix:
```python
_import_structure["pipeline_output"] = ["KolorsPipelineOutput"]

# TYPE_CHECKING branch
from .pipeline_output import KolorsPipelineOutput
```

## Issue 2: Kolors subpackage lazy import still breaks when `sentencepiece` is missing

Affected code:
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/kolors/__init__.py#L18-L28
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/kolors/__init__.py#L32-L39

Problem:
The dependency guard uses `if not (is_transformers_available() and is_torch_available()) and is_sentencepiece_available()`. That only raises when torch/transformers are missing and sentencepiece is present. If sentencepiece is missing, Kolors exposes real lazy modules that import `sentencepiece`.

Impact:
Direct imports from `diffusers.pipelines.kolors` can fail with a raw lazy-module import error instead of the normal backend message. This is the same failure class as existing issue https://github.com/huggingface/diffusers/issues/9034, but the subpackage guard is still malformed here.

Reproduction:
```python
import builtins, importlib, sys
import diffusers.utils as utils

utils.is_torch_available = lambda: True
utils.is_transformers_available = lambda: True
utils.is_sentencepiece_available = lambda: False

for name in list(sys.modules):
if name == "diffusers.pipelines.kolors" or name.startswith("diffusers.pipelines.kolors."):
del sys.modules[name]

kolors = importlib.import_module("diffusers.pipelines.kolors")

real_import = builtins.__import__
def blocked_import(name, *args, **kwargs):
if name == "sentencepiece":
raise ModuleNotFoundError("No module named 'sentencepiece'")
return real_import(name, *args, **kwargs)

builtins.__import__ = blocked_import
try:
kolors.ChatGLMTokenizer
finally:
builtins.__import__ = real_import
```

Relevant precedent:
The parent package has the correct condition:
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/__init__.py#L473-L484

Suggested fix:
```python
if not (is_transformers_available() and is_torch_available() and is_sentencepiece_available()):
raise OptionalDependencyNotAvailable()
```

## Issue 3: `max_sequence_length` is validated but ignored by both pipeline calls

Affected code:
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/kolors/pipeline_kolors.py#L865-L875
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/kolors/pipeline_kolors_img2img.py#L1018-L1026

Problem:
`__call__` accepts and validates `max_sequence_length`, but does not pass it into `encode_prompt`, so tokenization always uses the default `256`.

Impact:
Users cannot shorten prompt encoding for memory/performance or test non-default sequence lengths through the public pipeline API.

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

class Dummy(KolorsPipeline):
@property
def _execution_device(self):
return torch.device("cpu")
@property
def do_classifier_free_guidance(self):
return False
def check_inputs(self, *args, **kwargs):
pass
def encode_prompt(self, **kwargs):
print(kwargs.get("max_sequence_length"))
raise RuntimeError("stop")

pipe = Dummy.__new__(Dummy)
pipe.default_sample_size = 8
pipe.vae_scale_factor = 8
try:
pipe(prompt="x", max_sequence_length=16)
except RuntimeError:
pass
# Prints None, not 16.
```

Relevant precedent:
QwenImage forwards the call-time value:
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/qwenimage/pipeline_qwenimage.py#L585-L630

Suggested fix:
```python
) = self.encode_prompt(
...
negative_pooled_prompt_embeds=negative_pooled_prompt_embeds,
max_sequence_length=max_sequence_length,
)
```

## Issue 4: `encode_prompt` mishandles zeroed negatives and precomputed prompt embeds

Affected code:
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/kolors/pipeline_kolors.py#L289-L292
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/kolors/pipeline_kolors.py#L351-L359
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/kolors/pipeline_kolors_img2img.py#L309-L312
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/kolors/pipeline_kolors_img2img.py#L371-L379

Problem:
When `force_zeros_for_empty_prompt=True`, the code zeros `negative_prompt_embeds` but leaves `negative_pooled_prompt_embeds=None`, then calls `.repeat(...)`. Separately, if users pass precomputed `prompt_embeds` and `negative_prompt_embeds`, only pooled embeds are repeated for `num_images_per_prompt`; sequence embeds stay at batch size 1.

Impact:
The zero-negative config path crashes. Precomputed embeddings with `num_images_per_prompt > 1` later fail in UNet attention because latent batch and text batch do not match.

Reproduction:
```python
import torch
from types import SimpleNamespace
from diffusers.pipelines.kolors.pipeline_kolors import KolorsPipeline

pipe = KolorsPipeline.__new__(KolorsPipeline)
pipe._internal_dict = SimpleNamespace(force_zeros_for_empty_prompt=True)
pipe.tokenizer = None
pipe.text_encoder = None

try:
pipe.encode_prompt(
prompt=None,
device=torch.device("cpu"),
prompt_embeds=torch.randn(1, 4, 8),
pooled_prompt_embeds=torch.randn(1, 8),
do_classifier_free_guidance=True,
)
except Exception as e:
print(type(e).__name__, e)

pipe._internal_dict = SimpleNamespace(force_zeros_for_empty_prompt=False)
out = pipe.encode_prompt(
prompt=None,
device=torch.device("cpu"),
prompt_embeds=torch.randn(1, 4, 8),
pooled_prompt_embeds=torch.randn(1, 8),
negative_prompt_embeds=torch.randn(1, 4, 8),
negative_pooled_prompt_embeds=torch.randn(1, 8),
do_classifier_free_guidance=True,
num_images_per_prompt=2,
)
print([tuple(t.shape) for t in out])
# prompt/negative sequence embeds remain (1, 4, 8), pooled embeds become (2, 8).
```

Relevant precedent:
SDXL zeros the pooled negative embed and always duplicates prompt embeds after encoding/reuse:
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/stable_diffusion_xl/pipeline_stable_diffusion_xl.py#L421-L499

Suggested fix:
```python
if do_classifier_free_guidance and negative_prompt_embeds is None and zero_out_negative_prompt:
negative_prompt_embeds = torch.zeros_like(prompt_embeds)
negative_pooled_prompt_embeds = torch.zeros_like(pooled_prompt_embeds)

bs_embed, seq_len, _ = prompt_embeds.shape
prompt_embeds = prompt_embeds.repeat(1, num_images_per_prompt, 1).view(
bs_embed * num_images_per_prompt, seq_len, -1
)

if do_classifier_free_guidance:
seq_len = negative_prompt_embeds.shape[1]
negative_prompt_embeds = negative_prompt_embeds.repeat(1, num_images_per_prompt, 1).view(
bs_embed * num_images_per_prompt, seq_len, -1
)
```

Apply in `pipeline_kolors.py`, then propagate copied blocks.

## Issue 5: `KolorsImg2ImgPipeline.__call__` drops pooled prompt embeds

Affected code:
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/kolors/pipeline_kolors_img2img.py#L1018-L1026
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/kolors/pipeline_kolors_img2img.py#L534-L541

Problem:
Img2img validates that precomputed `prompt_embeds` must be accompanied by pooled embeds, but then does not pass `pooled_prompt_embeds` or `negative_pooled_prompt_embeds` into `encode_prompt`.

Impact:
The public precomputed-embedding path for Kolors img2img is unusable.

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

class Dummy(KolorsImg2ImgPipeline):
@property
def _execution_device(self):
return torch.device("cpu")
@property
def do_classifier_free_guidance(self):
return True
def check_inputs(self, *args, **kwargs):
pass
def encode_prompt(self, **kwargs):
print("pooled passed:", kwargs.get("pooled_prompt_embeds") is not None)
print("negative pooled passed:", kwargs.get("negative_pooled_prompt_embeds") is not None)
raise RuntimeError("stop")

pipe = Dummy.__new__(Dummy)
pipe.default_sample_size = 8
pipe.vae_scale_factor = 8
try:
pipe(
prompt=None,
image=torch.zeros(1, 3, 64, 64),
prompt_embeds=torch.randn(1, 4, 8),
pooled_prompt_embeds=torch.randn(1, 8),
negative_prompt_embeds=torch.randn(1, 4, 8),
negative_pooled_prompt_embeds=torch.randn(1, 8),
)
except RuntimeError:
pass
# Both printed values are False.
```

Relevant precedent:
`KolorsPipeline.__call__` passes both pooled tensors:
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/kolors/pipeline_kolors.py#L865-L875

Suggested fix:
```python
) = self.encode_prompt(
...
prompt_embeds=prompt_embeds,
pooled_prompt_embeds=pooled_prompt_embeds,
negative_prompt_embeds=negative_prompt_embeds,
negative_pooled_prompt_embeds=negative_pooled_prompt_embeds,
max_sequence_length=max_sequence_length,
)
```

## Issue 6: Img2img keeps stale SDXL-only offload and LoRA assumptions

Affected code:
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/kolors/pipeline_kolors_img2img.py#L23-L24
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/kolors/pipeline_kolors_img2img.py#L142-L151
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/kolors/pipeline_kolors_img2img.py#L171-L171
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/kolors/pipeline_kolors_img2img.py#L617-L620

Problem:
`model_cpu_offload_seq` contains `"image_encoder-unet"` instead of `"image_encoder->unet"`. `prepare_latents` references nonexistent `self.text_encoder_2`. The class also inherits `StableDiffusionXLLoraLoaderMixin`, whose loadable modules include `text_encoder_2`, while Kolors has only one text encoder.

Impact:
Model CPU offload is not chained correctly for img2img, stale offload-hook paths crash, and img2img LoRA loading inherits the same two-text-encoder assumption that PR https://github.com/huggingface/diffusers/pull/11198 fixed for the base Kolors pipeline.

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

print(KolorsImg2ImgPipeline.model_cpu_offload_seq.split("->"))
print(KolorsImg2ImgPipeline._lora_loadable_modules)

pipe = KolorsImg2ImgPipeline.__new__(KolorsImg2ImgPipeline)
pipe.vae = SimpleNamespace(config=SimpleNamespace(latents_mean=None, latents_std=None))
pipe.final_offload_hook = object()

try:
pipe.prepare_latents(torch.zeros(1, 3, 8, 8), torch.tensor([1]), 1, 1, torch.float32, torch.device("cpu"))
except Exception as e:
print(type(e).__name__, e)
```

Relevant precedent:
`KolorsPipeline` already uses `StableDiffusionLoraLoaderMixin` and the correct offload sequence:
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/kolors/pipeline_kolors.py#L22-L23
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/kolors/pipeline_kolors.py#L152-L152

Suggested fix:
```python
from ...loaders import IPAdapterMixin, StableDiffusionLoraLoaderMixin

class KolorsImg2ImgPipeline(DiffusionPipeline, StableDiffusionMixin, StableDiffusionLoraLoaderMixin, IPAdapterMixin):
model_cpu_offload_seq = "text_encoder->image_encoder->unet->vae"

# In prepare_latents:
self.text_encoder.to("cpu")
```

## Issue 7: `ChatGLMConfig()` lacks the `original_rope` default required by `ChatGLMModel`

Affected code:
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/kolors/text_encoder.py#L31-L75
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/kolors/text_encoder.py#L762-L767

Problem:
`ChatGLMModel.__init__` reads `config.original_rope`, but `ChatGLMConfig.__init__` never defines it unless it arrives via pretrained-config kwargs.

Impact:
A fresh/synthetic `ChatGLMConfig` cannot instantiate `ChatGLMModel`, which breaks local tiny configs and normal config round-tripping expectations.

Reproduction:
```python
from diffusers.pipelines.kolors.text_encoder import ChatGLMConfig, ChatGLMModel

cfg = ChatGLMConfig(
num_layers=1,
hidden_size=8,
ffn_hidden_size=16,
kv_channels=4,
num_attention_heads=2,
padded_vocab_size=32,
seq_length=8,
)
ChatGLMModel(cfg, empty_init=False)
# AttributeError: 'ChatGLMConfig' object has no attribute 'original_rope'
```

Relevant precedent:
The tiny pretrained ChatGLM config works only because its remote config includes `original_rope=True`; the class default should still be self-contained.

Suggested fix:
```python
def __init__(..., prefix_projection=False, original_rope=False, **kwargs):
...
self.prefix_projection = prefix_projection
self.original_rope = original_rope
super().__init__(**kwargs)
```

## Issue 8: Slow tests are missing for Kolors

Affected code:
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/tests/pipelines/kolors/test_kolors.py#L42-L132
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/tests/pipelines/kolors/test_kolors_img2img.py#L46-L158
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/tests/pipelines/pag/test_pag_kolors.py#L47-L180

Problem:
Kolors has fast tests using `hf-internal-testing/tiny-random-chatglm3-6b`, but no `@slow` tests for the real `Kwai-Kolors/Kolors-diffusers` text2img/img2img behavior. The prompt explicitly requires missing slow tests to be reported.

Impact:
Real-checkpoint regressions in prompt encoding, scheduler defaults, dtype/device behavior, offload, IP-Adapter, and img2img conditioning can ship without coverage.

Reproduction:
```python
from pathlib import Path

paths = list(Path("tests/pipelines/kolors").glob("test_*.py")) + [Path("tests/pipelines/pag/test_pag_kolors.py")]
missing = [p.as_posix() for p in paths if "@slow" not in p.read_text(encoding="utf-8")]
print("\n".join(missing))
```

Relevant precedent:
Many mature pipeline families include both tiny fast tests and at least one real-checkpoint slow smoke test for main workflows.

Suggested fix:
Add `@slow` smoke tests for `KolorsPipeline` and `KolorsImg2ImgPipeline` using `Kwai-Kolors/Kolors-diffusers` with a small deterministic prompt/image, low steps, and either a numerical slice or shape/sanity assertion. Include an offload/IP-Adapter slow path if runtime budget allows.

Guide de contribution

Ouvrir le guide de contribution

Par où commencer

  1. Lisez l'issue en entier, puis le guide de contribution du projet.
  2. Signalez en commentaire que vous la prenez — cela évite que deux personnes fassent le même travail.
  3. Forkez le dépôt et travaillez sur une branche.
  4. Ouvrez une pull request qui référence le numéro de l'issue.

Piste de recherche

Commencez par les fichiers Kolors concernés : `src/diffusers/pipelines/kolors/__init__.py`, `pipeline_kolors.py`, `pipeline_kolors_img2img.py` et `pipeline_output.py`, puis exécutez les tests Kolors ciblés. Comparez les chemins pertinents avec les précédents et les reproductions cités, en notant que la collecte locale échoue actuellement parce que le torch installé ne contient pas `torch._C._distributed_c10d`. Le travail est considéré comme terminé lorsque les problèmes répertoriés liés à l’exportation, aux dépendances, aux embeddings, à l’offload et aux appels du pipeline sont corrigés, avec une couverture de régression réussie.

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.