Fix get_head_size() when config materializes head_dim as 0 - #1072
Open
zohaibm-amd wants to merge 1 commit into
Open
Fix get_head_size() when config materializes head_dim as 0#1072zohaibm-amd wants to merge 1 commit into
zohaibm-amd wants to merge 1 commit into
Conversation
Cherry-pick of the get_head_size() portion of upstream commit ceae5bc ("[ROCm][CI] Fix nixl tests", vllm-project#45219). DeepseekVLV2TextConfig materializes a missing head_dim as 0 rather than leaving it unset. The existing guard only rejected None, so 0 was passed through as the architecture head size while the true head size is hidden_size / num_attention_heads = 1280 / 10 = 128. The resulting zero-sized KV cache made the first inference request abort the engine with hipErrorLaunchFailure ("CUDA error: unspecified launch failure"). This is a latent bug rather than a recent stack regression: the guard has only ever tested for None, and the DeepSeek-VL2-tiny configs happened to survive earlier because different code consumed the head size. Verified on gfx1151 with the nightly's own benchmark arguments, before vs after, identical args: DeepSeek-VL2-tiny_4096 before: exit 1, 0 successful requests, 5x "unspecified launch failure" after: exit 0, 2 successful requests, prefill 22084 tok/s, decode 138 tok/s DeepSeek-VL2-tiny_VLM before: engine dead, 0 tok/s after: prefill 5167 tok/s, decode 154 tok/s Changes: - The upstream commit also carries nixl/CI test changes that are unrelated to this crash; only the get_head_size() fix and its regression test are taken here to keep the later upstream merge (AIESW-37860) conflict-free. - DeepSeek-VL2-tiny_VLM still fails its multimodal sanity check after this fix, but that check was already failing on the Jun 26 nightly while the benchmark itself passed, so it is a separate pre-existing issue. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
roberteg16
reviewed
Jul 31, 2026
roberteg16
approved these changes
Jul 31, 2026
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.
Summary
Cherry-picks the
get_head_size()portion of upstream commitceae5bcbda(vllm-project/vllm#45219) to fix the nightlyDeepSeek-VL2-tiny_VLMandDeepSeek-VL2-tiny_4096regressions (AIESW-39871).Root cause
DeepseekVLV2TextConfigmaterializes a missinghead_dimas0rather than leaving it unset. The existing guard inget_head_size()only rejectedNone, so0passed straight through as the architecture head size — while the true head size ishidden_size / num_attention_heads = 1280 / 10 = 128.Confirmed directly against the model config, no GPU needed:
The resulting zero-sized KV cache is what aborts the engine on the first inference request with
hipErrorLaunchFailure/CUDA error: unspecified launch failure.This is a latent bug being exposed, not a stack regression — the guard has only ever tested for
None.Verification
Run on gfx1151 (Strix Halo) with the nightly's own benchmark arguments, before vs after, identical args:
DeepSeek-VL2-tiny_4096unspecified launch failureDeepSeek-VL2-tiny_VLMUnit test (upstream's, included here) is non-vacuous — it fails
assert 0 == 128against unpatched code and passes with the fix:ruff checkandruff format --checkboth clean.Scope note
The upstream commit also carries nixl/CI test changes unrelated to this crash. Only the
get_head_size()fix and its regression test are taken here, to keep the later upstream merge (AIESW-37860) conflict-free.DeepSeek-VL2-tiny_VLMstill fails its multimodal sanity check after this fix (answers'100.'instead of'red'), but that check was already failing on the Jun 26 nightly while the benchmark itself passed — a separate, pre-existing issue, tracked independently of this crash.🤖 Generated with Claude Code