Repository navigation
Fix odd head_dim validation for RoPE configurations - #48524
Merged
Rocketknight1 merged 4 commits intoSep 24, 2026
Merged
Conversation
somuai
force-pushed
the
fix-odd-head-dim-rope-validation
branch
from
September 4, 2026 12:04
a615f6c to
df4b4db
Compare
Contributor
Author
|
All CI matrix test shards (109/109) are passing green, modular template consistency checks passed with zero diffs, and bandit/ruff checks are clean. Rebased onto upstream main and ready for review @Rocketknight1. Thank you! |
3 of 5 tasks
somuai
force-pushed
the
fix-odd-head-dim-rope-validation
branch
2 times, most recently
from
September 22, 2026 13:56
0a4c5ae to
bfc80a0
Compare
Validate that rotary dimensions are even in RotaryEmbeddingConfigMixin.validate_rope. RoPE requires an even dimension to compute pairwise 2D rotations; an odd dimension results in dimension mismatch crashes during the forward pass. Fixes huggingface#48101 Signed-off-by: Soumyajit Ghosh <[email protected]>
Only reject the case that actually crashes: an odd head_dim that is fully rotated. Odd partial rotary dims are rounded up by `arange(0, dim, 2)` and still fit inside the head, so they work today and shouldn't be rejected. Drop the try/except scaffolding and the per-layer cross product; skip heterogeneous configs instead. Bump a few ConfigTester hidden sizes that produced head_dim=1/3 and would crash in a real forward. Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
Rocketknight1
force-pushed
the
fix-odd-head-dim-rope-validation
branch
from
September 23, 2026 15:03
0be17ac to
a385086
Compare
Rocketknight1
approved these changes
Sep 23, 2026
Rocketknight1
left a comment
Rocketknight1
left a comment
Member
There was a problem hiding this comment.
Simplified the checks a little, kept the core of it, and sorry for the review delay! Approving now
Rocketknight1
enabled auto-merge
September 23, 2026 15:08
|
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. |
Contributor
Author
|
Thank you for the review, refinements, and approval @Rocketknight1! |
…to fix-odd-head-dim-rope-validation
Contributor
|
[For maintainers] Suggested jobs to run (before merge) run-slow: diffllama, kyutai_speech_to_text, nomic_bert |
…Hub checkpoints Signed-off-by: Soumyajit Ghosh <[email protected]>
auto-merge was automatically disabled
September 23, 2026 19:18
Head branch was pushed to by a user without write access
Contributor
CI recapDashboard: View test results in Grafana |
Rocketknight1
enabled auto-merge
September 24, 2026 11:48
Merged
1 of 3 tasks
arde171
pushed a commit
to djl11/Liger-Kernel
that referenced
this pull request
Oct 7, 2026
…pe (linkedin#1513) Transfomers change: huggingface/transformers#48524, leads to failing tests because of odd head_dim. ## Summary <!--- This is a required section; please describe the main purpose of this proposed code change. ---> HuggingFace's newer config validation (validate_rope) requires an even rotary dimension, so constructing LlamaConfig(head_dim=41) now raises StrictDataclassClassValidationError. test_functional_correctness builds that config in its fixture, so the weird-shape case [*-9-7-41-41-41] fails during setup (before the kernel runs) on transformers versions that enforce the validator -- this is what breaks the daily H100 correctness CI. Odd head_dim is unsupported across the stack: the CuTeDSL backend rejects it (test_rope_cutedsl_rejects_odd_head_dimension) and the sibling test_correctness block in this same file already documents this and uses an even head_dim. Switch the weird shape from head_dim=41 to head_dim=92, matching that convention while preserving non-power-of-2 masking coverage (bsz=9, seq_len=7, 41 q/kv heads). <!--- ## Details This is an optional section; is there anything specific that reviewers should be aware of? ---> ## Testing Done <!--- This is a required section; please describe how this change was tested. ---> <!-- Replace BLANK with your device type. For example, A100-80G-PCIe Complete the following tasks before sending your PR, and replace `[ ]` with `[x]` to indicate you have done them. --> - Hardware Type: <BLANK> - [ ] run `make test` to ensure correctness - [x] run `make checkstyle` to ensure code style - [ ] run `make test-convergence` to ensure convergence Co-authored-by: Copilot <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Fixes #48101
As discussed and approved by @Rocketknight1 in #48101, model configurations specifying an odd attention head_dim (or an odd effective rotary dimension
int(head_dim * partial_rotary_factor)) previously passed configuration validation, but subsequently crashed during forward pass with a PyTorch dimension mismatch error (e.g.RuntimeError: The size of tensor a (65) must match the size of tensor b (66) at non-singleton dimension 3).Rotary Position Embeddings operate on pairs of 2D coordinates across orthogonal rotation planes and mathematically require an even dimension.
This PR adds compact validation:
RotaryEmbeddingConfigMixin.validate_rope: validates that any model configuration with RoPE parameters has an even rotary dimensionint(head_dim * partial_rotary_factor) % 2 == 0whenhead_dimis present. BecausePreTrainedConfiginherits this mixin,@strict-powered class validation automatically discovers and executesvalidate_ropeon config instantiation across all RoPE-based models (LLaMA, Qwen2, Mixtral, Gemma, DeepSeek, Mistral, etc.) in a single compact location.tests/utils/test_modeling_rope_utils.py: adds parameterized unit tests covering oddhead_dimand odd rotary dimensions frompartial_rotary_factoracross standard and nested RoPE configurations.Before submitting
Who can review?
@Rocketknight1