Skip to content

Revert "[Bugfix] Guard DeepSeek V4 MRV1 piecewise CUDA graphs" (#51768) - #52094

Closed
Fangzhou-Ai wants to merge 2 commits into
vllm-project:mainfrom
Fangzhou-Ai:afz/revert-dsv4-mrv2-default
Closed

Revert "[Bugfix] Guard DeepSeek V4 MRV1 piecewise CUDA graphs" (#51768)#52094
Fangzhou-Ai wants to merge 2 commits into
vllm-project:mainfrom
Fangzhou-Ai:afz/revert-dsv4-mrv2-default

Conversation

@Fangzhou-Ai

@Fangzhou-Ai Fangzhou-Ai commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Restore Model Runner V1 as the DeepSeek-V4 default.
  • Remove the MRV1 piecewise-graph rejection added by #51768.
  • Stack this change on #52093, which first restores the wider eager attention region required for correctness.

Dependency

Stacked on #52093. Do not merge this PR first: reverting #51768 alone would re-enable the known-broken MRV1 piecewise path while #51430 remains active. Until #52093 merges, this PR intentionally contains both revert commits; afterward its diff reduces to the config/test revert.

Why

On gfx950 TP8, MRV2 preserved correctness but regressed steady decode performance. With the prerequisite eager-region revert, MRV1 piecewise execution is correct and restores long-context throughput.

No open PR reverting #51768 was found.

Test plan

Configuration: DeepSeek-V4-Pro, gfx950, TP=8, DP=1, prefix caching disabled, max_num_batched_tokens=8192, AITER MoE, FP8 KV cache, FULL_AND_PIECEWISE graphs.

Configuration Output throughput Mean TPOT Median ITL
MRV2 before reverts 154.81 tok/s 24.70 ms 23.65 ms
MRV1 + #52093 185.66 tok/s 20.53 ms 19.47 ms
Historical reference 186.99 tok/s 20.36 ms 19.23 ms

Additional results:

  • 40/40 successful InferenceX 8k/1k concurrency-4 requests.
  • Mean TTFT: 466.34 ms.
  • Correctness smoke: 1 + 1 returned 2.
  • Two selective ROCm runs measured 185.16 and 184.94 tok/s, corroborating the rollback result.

Pre-commit hooks run during commit passed, including Ruff, formatting, mypy, SPDX, and configuration validation.

Contribution notes

  • Duplicate check: no open PR reverting #51768 was found.
  • AI assistance was used for regression isolation, implementation, testing, and PR preparation.
  • The human submitter must review every changed line and independently validate the results before merge.

Fangzhou-Ai and others added 2 commits August 13, 2026 04:05
…ct#51430)"

Restore the wider eager region because the narrowed capture corrupts ROCm output and regresses long-context serving performance.

Co-authored-by: Cursor <cursoragent@cursor.com>
Signed-off-by: Fangzhou Ai <fangzhou.ai@amd.com>
…roject#51768)"

Restore Model Runner V1 after the prerequisite eager-region revert makes piecewise graph execution correct again and recovers ROCm decode throughput.

Co-authored-by: Cursor <cursoragent@cursor.com>
Signed-off-by: Fangzhou Ai <fangzhou.ai@amd.com>

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

@mergify mergify Bot added deepseek Related to DeepSeek models nvidia bug Something isn't working labels Aug 13, 2026
@WoosukKwon

Copy link
Copy Markdown
Collaborator

Sorry for introducing the regression. Do we understand why the regression happens though? We are deprecating MRv1 anyways, so MRV2 should be working well on AMD.

@Fangzhou-Ai

Copy link
Copy Markdown
Contributor Author

Superseded for the ROCm path by #52488 (MRV1 default + wide eager region on ROCm only; CUDA keeps MRV2 / narrow region). Same approach as #52401.

This global revert can close once #52488 or #52401 lands.

@github-project-automation github-project-automation Bot moved this to Done in NVIDIA Aug 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working deepseek Related to DeepSeek models nvidia

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants