huggingface / huggingface/diffusers

kolors model/pipeline review

Đang mở
#13,612 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ả

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

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 tệp Kolors bị ảnh hưởng: `src/diffusers/pipelines/kolors/__init__.py`, `pipeline_kolors.py`, `pipeline_kolors_img2img.py` và `pipeline_output.py`, sau đó chạy các bài kiểm thử Kolors mục tiêu. So sánh các đường dẫn liên quan với các tiền lệ và bản tái hiện được trích dẫn, lưu ý rằng việc collection cục bộ hiện đang thất bại vì torch đã cài đặt thiếu `torch._C._distributed_c10d`. Được xem là hoàn tất khi các vấn đề được liệt kê về export, dependency, embedding, offload và pipeline-call đã được xử lý, với coverage hồi quy đạt yêu cầu.

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.