huggingface / huggingface/diffusers
kolors model/pipeline review
- Lenguaje dominante
- Python
- Estrellas
- 34.5k
- Forks
- 7.3k
- Merge medio
- 3 d 3 h
- PR fusionados (30 d)
- 91
Descripción
# `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.
Guía de contribución
Línea de trabajo
Start with the affected Kolors files: `src/diffusers/pipelines/kolors/__init__.py`, `pipeline_kolors.py`, `pipeline_kolors_img2img.py`, and `pipeline_output.py`, then run the targeted Kolors tests. Compare the relevant paths with the cited precedents and reproductions, noting that local collection currently fails because the installed torch lacks `torch._C._distributed_c10d`. Done means the listed export, dependency, embedding, offload, and pipeline-call issues are addressed with passing regression coverage.
Escrito por el modelo de indexación a partir del texto del issue.
Evaluación
- Stack tecnológico
- python, pytorch
- Área
- machine-learning
- Tipo de issue
- Error
- Dificultad
- 5/5
- Tiempo estimado
- Más de una semana
- Estado de actividad
- Tranquilo
- Claridad
- Bastante claro
- Aptitud para principiantes
- 35/100