huggingface / huggingface/diffusers
kolors model/pipeline review
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
- Lisez l'issue en entier, puis le guide de contribution du projet.
- Signalez en commentaire que vous la prenez — cela évite que deux personnes fassent le même travail.
- Forkez le dépôt et travaillez sur une branche.
- 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