Skip to content

fix failed test cases for glm5_next - #48497

Merged
vasqu merged 4 commits into
huggingface:mainfrom
kaixuanliu:glm5_next
Sep 4, 2026
Merged

vasqu merged 4 commits into
huggingface:mainfrom
kaixuanliu:glm5_next

Conversation

@kaixuanliu

@kaixuanliu kaixuanliu commented Sep 3, 2026 •

Copy link
Copy Markdown
Contributor

CPU CI GPU run-slow

Fix 3 failed cases for glm5-next:

FAILED tests/models/glm5_next/test_modeling_glm5_next.py::Glm5NextModelTest::test_flash_attn_2_inference_equivalence - ZeroDivisionError: division by zero
FAILED tests/models/glm5_next/test_modeling_glm5_next.py::Glm5NextModelTest::test_flash_attn_2_inference_equivalence_right_padding - ZeroDivisionError: division by zero
FAILED tests/models/glm5_next/test_modeling_glm5_next.py::Glm5NextModelTest::test_model_parallelism - RuntimeError: Expected all tensors to be on the same device, but found at least two devices, xpu:1 and xpu:0!

@ydshieh pls help review, thx!

Signed-off-by: kaixuanliu <kaixuan.liu@intel.com>
Signed-off-by: kaixuanliu <kaixuan.liu@intel.com>
Signed-off-by: kaixuanliu <kaixuan.liu@intel.com>
Comment on lines +172 to +179
@unittest.skip("Glm5Next text DSA attention does not support flash_attention_2 in the common full-model test.")
def test_flash_attn_2_inference_equivalence(self):
pass

@unittest.skip("Glm5Next text DSA attention does not support flash_attention_2 in the common full-model test.")
def test_flash_attn_2_inference_equivalence_right_padding(self):
pass

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm is it because vision side does support it but text doesnt? I'm a bit confused because the common test imo should detect to skip this by itself

The 0 division error is something else tho, probably our prepare head dim function (which we should ignore in this case as it's a unique case)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, the explicit skips were too broad. The real issue was that the common FA2 test calls _prepare_config_headdim before it instantiates the model and reaches the existing _supports_flash_attn skip logic. For GLM5 Next, text_config.head_dim intentionally mirrors qk_rope_head_dim, which is 0 in this test config, while the actual QK dim is qk_head_dim.

I replaced the manual skips with a model-test _prepare_config_headdim override that temporarily uses text_config.qk_head_dim, so the common helper no longer hits the 0-division and the common FA2 skip logic handles this as expected.

Signed-off-by: kaixuanliu <kaixuan.liu@intel.com>
@github-actions

github-actions Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

[For maintainers] Suggested jobs to run (before merge)

run-slow: glm5_next

@github-actions

github-actions Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

CI recap

Dashboard: View test results in Grafana
Latest run: 33717730867:2
Result: success | Jobs: 5 | Tests: 339 | Failures: 0 | Duration: 5m 27s

@vasqu vasqu left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, thanks for fixing 🫡

@vasqu
vasqu enabled auto-merge September 4, 2026 12:38
@vasqu
vasqu added this pull request to the merge queue Sep 4, 2026
@HuggingFaceDocBuilderDev

Copy link
Copy Markdown

The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.

Merged via the queue into huggingface:main with commit 3197876 Sep 4, 2026
38 checks passed
sbucaille pushed a commit to sbucaille/transformers that referenced this pull request Sep 16, 2026
* fix failed test cases for glm5_next

Signed-off-by: kaixuanliu <kaixuan.liu@intel.com>

* update comment

Signed-off-by: kaixuanliu <kaixuan.liu@intel.com>

* update

Signed-off-by: kaixuanliu <kaixuan.liu@intel.com>

* update

Signed-off-by: kaixuanliu <kaixuan.liu@intel.com>

---------

Signed-off-by: kaixuanliu <kaixuan.liu@intel.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants