[Bugfix][CPU] Take an attention group's query head count from its layers - #51852
Merged
bigPYJ1151 merged 4 commits intoAug 17, 2026
Merged
Conversation
ganeshr10
marked this pull request as ready for review
August 12, 2026 07:55
ganeshr10
requested review from
LucasWilkinson and
MatthewBonanni
as code owners
August 12, 2026 07:55
Contributor
|
Hi @ganeshr10, the pre-commit checks have failed. Please run: uv pip install pre-commit>=4.5.1
pre-commit install
pre-commit run --all-filesThen, commit the changes and push to your branch. For future commits, |
bigPYJ1151
reviewed
Aug 13, 2026
bigPYJ1151
reviewed
Aug 14, 2026
ganeshr10
force-pushed
the
fix-cpu-attn-per-layer-head-counts
branch
from
August 17, 2026 06:09
33cb73d to
839fde8
Compare
Contributor
|
Hi @ganeshr10, the pre-commit checks have failed. Please run: uv pip install pre-commit>=4.5.1
pre-commit install
pre-commit run --all-filesThen, commit the changes and push to your branch. For future commits, |
…s differ The CPU backend sizes one scheduler metadata blob per KV cache group from the model-wide query head count, so models that vary it per layer (e.g. Laguna) overrun the split-KV scratchpad and either segfault or hang. Build metadata per distinct head count and let each layer select the one matching its own. Signed-off-by: Ganesh R <Ganesh.R@amd.com> Change-Id: I671eadc2b5601f1a3af12f39391657a11df1c9f7
The layers a group covers are known by the time the builder is built, so read them there rather than on the first build(). This also drops the Optional from window_size, which mypy could not narrow inside the nested scheduler metadata helper. Run the head count test only on CPU, and in CPU CI. Signed-off-by: R <Ganesh.R@amd.com> Co-authored-by: Cursor <cursoragent@cursor.com> Change-Id: Iad3cd2ff8ad7dbc4101e5341c5673e2b0b300e9e Signed-off-by: R <Ganesh.R@amd.com> Co-authored-by: Cursor <cursoragent@cursor.com>
Attention groups are keyed on num_heads_q, so the layers in a group always agree on it and no single metadata blob has to cover several counts. Read that count with get_num_attention_heads_from_layers, which triton_attn and flashinfer already use for the same reason, and drop the per-count metadata dict. Signed-off-by: R <Ganesh.R@amd.com> Co-authored-by: Cursor <cursoragent@cursor.com> Change-Id: I80456459171fef0edd9d10430d98ebc9e9b56131
ganeshr10
force-pushed
the
fix-cpu-attn-per-layer-head-counts
branch
from
August 17, 2026 07:23
839fde8 to
f307f46
Compare
Member
|
/ci run |
|
✅ Triggered Buildkite CI #84171 for commit |
Member
|
/ci run |
|
✅ Triggered Buildkite CI #84175 for commit |
Alessandra005
pushed a commit
to Alessandra005/vllm
that referenced
this pull request
Aug 17, 2026
…ers (vllm-project#51852) Signed-off-by: Ganesh R <Ganesh.R@amd.com> Signed-off-by: R <Ganesh.R@amd.com> Co-authored-by: Cursor <cursoragent@cursor.com> Co-authored-by: Li, Jiang <jiang1.li@intel.com> Signed-off-by: Alessandra005 <aurib032@fiu.edu>
zyp2014
pushed a commit
to zyp2014/vllm
that referenced
this pull request
Aug 21, 2026
…ers (vllm-project#51852) Signed-off-by: Ganesh R <Ganesh.R@amd.com> Signed-off-by: R <Ganesh.R@amd.com> Co-authored-by: Cursor <cursoragent@cursor.com> Co-authored-by: Li, Jiang <jiang1.li@intel.com>
wyettzeng
pushed a commit
to wyettzeng/vllm
that referenced
this pull request
Aug 21, 2026
…ers (vllm-project#51852) Signed-off-by: Ganesh R <Ganesh.R@amd.com> Signed-off-by: R <Ganesh.R@amd.com> Co-authored-by: Cursor <cursoragent@cursor.com> Co-authored-by: Li, Jiang <jiang1.li@intel.com> Signed-off-by: Wyett <wyettzeng@gmail.com>
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.
Purpose
CPUAttentionMetadataBuildersizes the split-KV scratchpad from the model-wide query head count, but the kernel runs with each layer's own count. so models that vary it per layer (e.g. Laguna) index past the end of the scratchpad andthe decode segfaults or hangs. Attention groups are keyed on
num_heads_q, so take the count from the group's layers viaget_num_attention_heads_from_layers(), astriton_attnandflashinferalready do. Models with a uniform count are unaffected.Change-Id: I671eadc2b5601f1a3af12f39391657a11df1c9f7
Test Plan
pytest tests/v1/attention/test_group_head_counts.py— new unit test, mirroringtest_group_sliding_window.py: uniform heads, Laguna-style alternating heads, and a default that is not the most common count.[8, 16, 8, 16]against 2 KV heads and random weights (load_format=dummy, so no checkpoint is needed), decoding 512 tokens so split-KV reduction engages. Run both on unpatchedmainand with this change.Reproducer
Test Result
Unit tests: 6 passed (3 new, plus the 3 existing sliding-window cases).
Reproducer: hangs on current
mainand is killed at the timeout; passes in 1.5s with this change.Laguna-XS 300-token decode: previously segfaulted or hung, now completes in 47s with coherent output.
GSM8K 5-shot, full 1319 prompts. Before this change the same run died with an
execute_modeltimeout.Instrumenting the scratchpad bounds shows the overrun directly on unpatched
main: the region holds 6336 bytes per KV head while the split-KV write path needs 8384, because sizing assumes 6 query heads per KV head (48 heads / 8 KV heads) and the 64-head layers address it with a stride of 8.Models whose layers share one head count keep the existing single-blob path, so there is no extra allocation or lookup for them.
Essential Elements of an Effective PR Description Checklist
supported_models.mdandexamplesfor a new model.