[ROCm] Keep DeepSeek V4 on MRV1 with the wide eager attention region - #52488
Closed
Fangzhou-Ai wants to merge 1 commit into
Closed
[ROCm] Keep DeepSeek V4 on MRV1 with the wide eager attention region#52488Fangzhou-Ai wants to merge 1 commit into
Fangzhou-Ai wants to merge 1 commit into
Conversation
vllm-project#51430/vllm-project#51768 moved DSV4 to MRV2 and a narrow eager region. That is a large decode TPOT regression on ROCm. Default ROCm back to MRV1, wrap the full attention body in the eager break for MRV1 only, and drop the MRV1+PIECEWISE rejection. CUDA keeps MRV2 and the narrow region. Co-authored-by: Nick Hill <nickhill123@gmail.com> Co-authored-by: Cursor Grok 4.6 <cursoragent@cursor.com> Signed-off-by: fai <fangzhouai@gmail.com>
Fangzhou-Ai
requested review from
ProExpertProg,
WoosukKwon,
houseroad,
mgoin,
robertgshaw2-redhat,
tlrmchlsmth,
yewentao256,
youkaichao and
zyongye
as code owners
August 16, 2026 03:27
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
This was referenced Aug 16, 2026
Member
|
/ci run |
|
✅ Triggered Buildkite CI #84062 for commit |
Member
|
@Fangzhou-Ai this should hopefully already have been resolved by #52401, could you confirm? |
Member
|
Actually it appears that this PR is just a duplicate of that one? |
Contributor
Author
|
Thanks @njhill I think your solution should work, therefore I am closing this PR! |
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
3 participants
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
_prepare_and_attnin_prepare_and_attn_eager; MRV2 keeps the narrow_sparse_indexer_and_attnbreak. Drop the MRV1+PIECEWISE rejection once the wide region is restored for MRV1.Why this is not a duplicate of #52093 / #52094
Those PRs revert #51430 and #51768 on every platform. This PR scopes the same performance recovery to ROCm. CUDA is unchanged.
#52401 already takes this same runner-gated approach. This PR is the AMD-fork replacement for #52093/#52094 so those global reverts can close. If #52401 lands first, this PR can close as a duplicate.
Test plan
.venv/bin/python -m pytest tests/test_config.py::test_rocm_defaults_deepseek_v4_to_mrv1 tests/test_config.py::test_is_default_v2_model_runner_model tests/test_config.py::test_v2_model_runner_env_tri_state -v --noconftest— 23 passeddsv4_rocm_bench/run_inferencex_8k1k.sh). Expected: mean TPOT ~20.5 ms / ~186 tok/s (MRV1 + wide eager). Current MRV2 default on the same box: 24.70 ms / 154.81 tok/s (#52094 table).Contribution notes
Made with Cursor