huggingface / huggingface/diffusers

glm_image model/pipeline review

Offen
#13,587 0 Kommentare 0 Reaktionen 0 zugewiesene Personen Auf GitHub ansehen
Vorherrschende Sprache
Python
Sterne
34.5k
Forks
7.3k
Ø Merge
3 T. 3 Std.
Gemergte PRs (30 T.)
91

Beschreibung

# `glm_image` model/pipeline review

Commit tested: `0f1abc4ae8b0eb2a3b40e82a310507281144c423`

Review performed against the repository review rules.

Duplicate-search status: searched `huggingface/diffusers` Issues and PRs for `glm_image`, `GLM-Image`, `GlmImagePipeline`, `GlmImageTransformer2DModel`, `attention_mask`, `prompt_embeds dtype device`, `check_inputs width`, transformer version gating, and slow-test coverage. I found no duplicates for the findings below. Related but not duplicates: PR #12974 adjusted GLM transformer-version gating, PR #13007 added batch support, PR #13344 added model tests, and issue #13227 tracks an MPS loading corruption issue.

## Issue 1: Attention masks do not actually mask padded text tokens

Affected code:
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/models/transformers/transformer_glm_image.py#L317-L325
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/glm_image/pipeline_glm_image.py#L518-L543
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/glm_image/pipeline_glm_image.py#L972-L999

Problem:
`GlmImageAttnProcessor` converts a boolean padding mask into a dense float `0/1` tensor. SDPA treats float masks as additive attention bias, so `0` does not block a token. The pipeline also discards the glyph encoder padding mask after constructing padded glyph embeddings, so batched variable-length glyph prompts have no valid text mask passed to the transformer.

Impact:
Masked or padded text tokens can still affect image tokens. This can make batched outputs differ from equivalent single-prompt outputs and makes the public `attention_mask` argument misleading. It also prevents the bool-mask varlen path described in the review rules.

Reproduction:
```python
import torch
from diffusers.models.attention_processor import Attention
from diffusers.models.transformers.transformer_glm_image import GlmImageAttnProcessor

attn = Attention(query_dim=4, heads=1, dim_head=4, out_dim=4, bias=False, processor=GlmImageAttnProcessor())
with torch.no_grad():
attn.to_q.weight.zero_()
attn.to_k.weight.zero_()
attn.to_v.weight.copy_(torch.eye(4))
attn.to_out[0].weight.copy_(torch.eye(4))

encoder_hidden_states = torch.tensor([[[0., 0., 0., 0.], [1000., 0., 0., 0.]]])
hidden_states = torch.zeros(1, 1, 4)
attention_mask = torch.tensor([[True, False]])

image_out, _ = attn(
hidden_states=hidden_states,
encoder_hidden_states=encoder_hidden_states,
attention_mask=attention_mask,
)
print(image_out[0, 0, 0].item()) # current: ~155.66; expected: 0 if token 2 is masked
```

Relevant precedent:
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/models/transformers/transformer_qwenimage.py#L946-L952
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/qwenimage/pipeline_qwenimage.py#L695-L704

Suggested fix:
```python
# In GlmImageAttnProcessor.__call__, keep a bool key mask instead of a float QK matrix.
if attention_mask is not None:
attention_mask = attention_mask.to(device=query.device, dtype=torch.bool)
cached_seq_length = key.shape[1] - text_seq_length - image_seq_length
cache_mask = torch.ones((batch_size, cached_seq_length), device=query.device, dtype=torch.bool)
image_mask = torch.ones((batch_size, image_seq_length), device=query.device, dtype=torch.bool)
attention_mask = torch.cat([cache_mask, attention_mask, image_mask], dim=1)
```
Also return a glyph padding mask from `_get_glyph_embeds` and pass it through the conditional and unconditional transformer calls.

## Issue 2: Width validation accepts invalid resolutions and silently truncates latents

Affected code:
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/glm_image/pipeline_glm_image.py#L633-L642

Problem:
`check_inputs` validates height with `vae_scale_factor * patch_size * 2`, but validates width only with `patch_size * 2`. With default GLM settings this accepts widths divisible by `4`, even though the pipeline later floors latent width by `width // vae_scale_factor`.

Impact:
Invalid widths pass validation and produce latents for a smaller decoded width than requested.

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

class Config:
patch_size = 2

class Transformer:
config = Config()

pipe = object.__new__(GlmImagePipeline)
pipe.vae_scale_factor = 8
pipe.transformer = Transformer()
pipe._callback_tensor_inputs = ["latents", "prompt_embeds"]

pipe.check_inputs(prompt="x", height=32, width=20, callback_on_step_end_tensor_inputs=["latents"])
latents = pipe.prepare_latents(1, 4, 32, 20, torch.float32, torch.device("cpu"), torch.Generator().manual_seed(0))
print(latents.shape[-1] * pipe.vae_scale_factor) # 16, not requested width 20
```

Relevant precedent:
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/cogview4/pipeline_cogview4.py#L328-L329

Suggested fix:
```python
multiple_of = self.vae_scale_factor * self.transformer.config.patch_size * 2
if (height is not None and height % multiple_of != 0) or (width is not None and width % multiple_of != 0):
raise ValueError(f"`height` and `width` have to be divisible by {multiple_of} but are {height} and {width}.")
```

## Issue 3: Precomputed conditioning tensors are not moved or cast

Affected code:
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/glm_image/pipeline_glm_image.py#L584-L600
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/glm_image/pipeline_glm_image.py#L833-L836

Problem:
When users pass `prompt_embeds`, `negative_prompt_embeds`, or prior token tensors directly, the pipeline returns/uses them as-is instead of normalizing them to the execution device and dtype.

Impact:
Precomputed embeddings can fail at the transformer with dtype or device mismatches. Prior token tensors generated on CPU can also fail when the pipeline is on an accelerator.

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

pipe = object.__new__(GlmImagePipeline)
prompt_embeds = torch.randn(1, 2, 4, dtype=torch.float64)

out, _ = pipe.encode_prompt(
prompt=None,
do_classifier_free_guidance=False,
prompt_embeds=prompt_embeds,
device=torch.device("cpu"),
dtype=torch.float32,
)
print(out.dtype, out is prompt_embeds) # torch.float64 True
```

Relevant precedent:
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/cogview4/pipeline_cogview4.py#L216-L218

Suggested fix:
```python
if prompt_embeds is None:
prompt_embeds = self._get_glyph_embeds(prompt, max_sequence_length, device, dtype)
else:
prompt_embeds = prompt_embeds.to(device=device, dtype=dtype)

if negative_prompt_embeds is not None:
negative_prompt_embeds = negative_prompt_embeds.to(device=device, dtype=dtype)

if prior_token_ids is not None:
prior_token_ids = prior_token_ids.to(device=device)
if prior_token_image_ids is not None:
prior_token_image_ids = [x.to(device=device) for x in prior_token_image_ids]
if source_image_grid_thw is not None:
source_image_grid_thw = [x.to(device=device) for x in source_image_grid_thw]
```

## Issue 4: Transformers version gates are inconsistent with required GLM classes

Affected code:
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/glm_image/pipeline_glm_image.py#L36-L40
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/glm_image/__init__.py#L20-L27
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/tests/pipelines/glm_image/test_glm_image.py#L29-L37

Problem:
The pipeline file only imports real `GlmImageProcessor` / `GlmImageForConditionalGeneration` for `transformers >= 5.0.0.dev0`, but the package init tries `>= 4.57.4` and the fast tests require only `> 4.57.4`.

Impact:
With `transformers==4.57.6`, the pipeline is importable but uses `ProcessorMixin` / `PreTrainedModel` placeholders, and the test decorator would allow tests whose GLM classes are not imported.

Reproduction:
```python
import transformers
from diffusers.utils import is_transformers_version
from diffusers.pipelines.glm_image.pipeline_glm_image import GlmImageProcessor, GlmImageForConditionalGeneration
from transformers import ProcessorMixin, PreTrainedModel

print(transformers.__version__)
print(is_transformers_version(">", "4.57.4")) # True in this env
print(is_transformers_version(">=", "5.0.0.dev0")) # False
print(hasattr(transformers, "GlmImageProcessor")) # False
print(GlmImageProcessor is ProcessorMixin, GlmImageForConditionalGeneration is PreTrainedModel)
```

Relevant precedent:
Related prior version-gating PR: https://github.com/huggingface/diffusers/pull/12974

Suggested fix:
```python
GLM_IMAGE_TRANSFORMERS_MIN_VERSION = "5.0.0.dev0" # or the first released transformers version with these classes

if is_transformers_available() and is_transformers_version(">=", GLM_IMAGE_TRANSFORMERS_MIN_VERSION):
from transformers import GlmImageForConditionalGeneration, GlmImageProcessor
else:
raise OptionalDependencyNotAvailable()
```
Use the same predicate in pipeline lazy imports and tests.

## Issue 5: Slow tests are missing, and current fast coverage is not portable

Affected code:
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/tests/pipelines/glm_image/test_glm_image.py#L36-L38
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/tests/pipelines/glm_image/test_glm_image.py#L90
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/docs/source/en/api/models/glm_image_transformer2d.md#L14
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/glm_image/pipeline_output.py#L10-L12

Problem:
There are no GLM Image slow tests. The pipeline “fast” tests are decorated with `@require_torch_accelerator` even though they run on CPU, and they load the processor from `zai-org/GLM-Image` instead of an internal tiny fixture. The model docs still contain a TODO, and the pipeline output docstring says CogView3.

Impact:
CPU CI can skip the pipeline fast suite, slow end-to-end loading of `zai-org/GLM-Image` is untested, and docs have stale placeholders.

Reproduction:
```python
from pathlib import Path

test_text = Path("tests/pipelines/glm_image/test_glm_image.py").read_text(encoding="utf-8")
model_doc = Path("docs/source/en/api/models/glm_image_transformer2d.md").read_text(encoding="utf-8")

print("@slow present:", "@slow" in test_text)
print("fast class requires accelerator:", "@require_torch_accelerator" in test_text)
print("fast test downloads full GLM processor:", "zai-org/GLM-Image" in test_text)
print("model docs still contain TODO:", "TODO" in model_doc)
```

Relevant precedent:
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/tests/pipelines/qwenimage/test_qwenimage.py#L117

Suggested fix:
Use an `hf-internal-testing` tiny GLM processor fixture for fast tests, remove the accelerator requirement from CPU-only fast tests, add at least one `@slow` smoke test for `GlmImagePipeline.from_pretrained("zai-org/GLM-Image", ...)`, and replace the stale TODO/CogView3 doc text.

Test status: a tiny CPU `GlmImageTransformer2DModel` forward pass succeeded. Targeted pytest collection for GLM model/pipeline tests failed in this local `.venv` because the installed Torch build lacks `torch._C._distributed_c10d`, so I could not use pytest results as signal for this audit.

Beitragsleitfaden

Beitragsleitfaden öffnen

Rechercherichtung

Beginne mit den betroffenen GLM-Dateien: src/diffusers/models/transformers/transformer_glm_image.py, src/diffusers/pipelines/glm_image/pipeline_glm_image.py, der zugehörigen __init__.py und pipeline_output.py. Lies tests/pipelines/glm_image/test_glm_image.py und die verknüpften QwenImage/CogView4-Präzedenzfälle und führe anschließend die gezielten GLM-Tests aus. Fertig ist die Arbeit, wenn die fünf gemeldeten Bereiche korrigiert sind, portable Abdeckung für schnelle und langsame Ausführung vorhanden ist und die veraltete Modell- und Output-Dokumentation aktualisiert wurde.

Vom Indexierungsmodell aus dem Issue-Text verfasst.

Bewertung

Tech-Stack
python, pytorch
Bereich
documentation, machine-learning, testing-qa
Issue-Typ
Bug
Schwierigkeit
5/5
Geschätzter Aufwand
Über eine Woche
Aktivitätsstatus
Ruhig
Klarheit
Größtenteils klar
Anfängerfreundlichkeit
35/100

Neue Issues direkt in Ihr Postfach

Eine kurze Übersicht über anfängerfreundliche GitHub-Issues.