huggingface / huggingface/diffusers

glm_image model/pipeline review

Open
#13,587 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Python
Stars
34.5k
Forks
7.3k
Avg merge
3d 3h
Merged PRs (30d)
91

Description

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

Contributor guide

Open the contributing guide

Research direction

Start with the affected GLM files: src/diffusers/models/transformers/transformer_glm_image.py, src/diffusers/pipelines/glm_image/pipeline_glm_image.py, its __init__.py and pipeline_output.py. Read tests/pipelines/glm_image/test_glm_image.py and the linked QwenImage/CogView4 precedents, then run the targeted GLM tests. Done means the five reported areas are corrected, portable fast and slow coverage exists, and the stale model and output documentation is updated.

Written by the indexing model from the issue text.

Assessment

Tech stack
python, pytorch
Domain
documentation, machine-learning, testing-qa
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.