perf(agentx): refresh DeepSeek V4 MTP frontier on MI355X - #2540
Conversation
|
Thanks for the contribution! Please reach out to respective companies' CODEOWNER to fill in the latest PR_REVIEW_CHECKLIST.md before pinging core maintainer on Slack for review. In order for the signoff PR check bot to trigger, you must follow the PR_REVIEW_CHECKLIST.md template correctly, including the phrase For PR verification, add the PR authors are responsible for ensuring that after merging, all GitHub Action jobs fully pass. A lot of the time, failures are just flakes and simply re-running the failed jobs will fix it. See GitHub's docs on re-running failed jobs 感谢你的贡献!请联系相应公司的 CODEOWNER 填写最新的 PR_REVIEW_CHECKLIST.md,然后再在 Slack 上联系核心维护者进行审阅。为了触发 signoff PR 检查机器人,你必须正确遵循 PR_REVIEW_CHECKLIST.md 模板,包括保留英文语句 如需进行 PR 验证,请为此 PR 添加 PR 作者有责任确保合并后所有 GitHub Action 任务完全通过。 很多时候失败只是偶发抖动(flake),重新运行失败的任务即可解决。参见 GitHub 关于重新运行失败任务的文档 |
|
Claude finished @cquil11's task in 2m 25s —— View job PR Review
LGTM - no blocking issues found Verification details:
|
There was a problem hiding this comment.
Beyond the inline finding on FULL_DECODE_ONLY capture-size collapse, I also checked whether MAX_NUM_SEQS=$((2 * CONC)) (not divided by DP world size) under-sizes per-rank scheduling capacity on the DP8+EP8 band — it doesn't: each DP rank runs its own engine and this only reserves generous per-rank headroom, so it's not a correctness issue.
Extended reasoning...
The finder flagged that MAX_NUM_SEQS = 2CONC is not divided by the DP world size (TP) for the DP8+EP8/dp-attn band, which could suggest under- or over-provisioning per-rank scheduling slots. Verification: in DP-attention mode each DP rank runs an independent vLLM engine instance behind the router, and --max-num-seqs is a per-engine (i.e. per-rank) cap, not a global one. Since the router distributes the aggregate CONC across ranks, using the full un-divided 2CONC as each rank's cap only over-provisions scheduling headroom (matching the script's own comment about 2x headroom for AgentX subagent fan-out); it does not create an under-provisioning correctness bug. This is a much smaller consideration than the FULL_DECODE_ONLY cudagraph capture-size bug already posted as an inline comment on this PR, which is a real and non-trivial issue at the new high-concurrency DP8+EP8 cells this PR adds.
| --kv-cache-dtype fp8 | ||
| "${PARALLEL_ARGS[@]}" | ||
| "${EP_ARGS[@]}" | ||
| --gpu-memory-utilization 0.8 | ||
| --gpu-memory-utilization 0.9 | ||
| --block-size 256 | ||
| --max-num-batched-tokens 8192 | ||
| --moe-backend aiter | ||
| --compilation-config '{"mode":3,"cudagraph_mode":"FULL_AND_PIECEWISE"}' | ||
| --compilation-config '{"mode":3,"cudagraph_mode":"FULL_DECODE_ONLY"}' | ||
| --speculative-config "$SPEC_CONFIG" | ||
| --tokenizer-mode deepseek_v4 | ||
| --tool-call-parser deepseek_v4 | ||
| --reasoning-parser deepseek_v4 | ||
| --enable-auto-tool-choice | ||
| --enable-prefix-caching | ||
| --enable-prompt-tokens-details | ||
| --no-disable-hybrid-kv-cache-manager | ||
| --max-num-seqs "$MAX_NUM_SEQS" | ||
| "${OFFLOAD_ARGS[@]}" |
There was a problem hiding this comment.
🔴 Switching --compilation-config to FULL_DECODE_ONLY (line 411-412) without also passing a capture-size fix-up means vLLM's adjust_cudagraph_sizes_for_spec_decode() rounds captured decode-batch sizes up to multiples of (1+NUM_SPEC_TOKENS)=4 and dedups, collapsing capture coverage to MAX_NUM_SEQS/4 seqs — the largest decode batches at the new high-concurrency cells (c40/c56/c224, MAX_NUM_SEQS up to 448) silently fall back to eager execution. The sibling B200/B300 MTP recipes in this same directory (dsv4_fp4_b200_vllm_mtp.sh:239-285, dsv4_fp4_b300_vllm_mtp.sh:261-286) hit this exact MTP+FULL_DECODE_ONLY interaction and fix it with --max-cudagraph-capture-size / an explicit cudagraph_capture_sizes list sized to MAX_NUM_SEQS*(1+NUM_SPEC_TOKENS); this script needs the same treatment.
Extended reasoning...
The bug: this PR switches --compilation-config from {"mode":3,"cudagraph_mode":"FULL_AND_PIECEWISE"} to {"mode":3,"cudagraph_mode":"FULL_DECODE_ONLY"} (dsv4_fp4_mi355x_vllm_mtp.sh:411-412) but does not carry over the capture-size mitigation that this exact mode+MTP combination requires. Every row in this recipe's search space runs spec-decoding: mtp with NUM_SPEC_TOKENS=3, so TOKENS_PER_SEQ = 1 + NUM_SPEC_TOKENS = 4.
Why it triggers: vLLM auto-derives its CUDA-graph capture-size ladder from --max-num-seqs assuming one token per sequence. When speculative decoding is active and cudagraph_mode is FULL_DECODE_ONLY, adjust_cudagraph_sizes_for_spec_decode() rounds each auto-derived size up to the nearest multiple of TOKENS_PER_SEQ and dedups the resulting list. With no explicit override, this collapses the usable capture range down to roughly MAX_NUM_SEQS / TOKENS_PER_SEQ seqs — i.e. MAX_NUM_SEQS / 4 here. Any decode batch larger than that silently falls back to eager execution instead of using a captured graph.
Why the existing code doesn't prevent it: the previous mode, FULL_AND_PIECEWISE, uses piecewise CUDA graphs for the portion outside the captured range, so this collapse was masked. Switching to FULL_DECODE_ONLY removes that fallback path, and this PR's --compilation-config JSON only sets mode and cudagraph_mode — no --max-cudagraph-capture-size flag, and no explicit cudagraph_capture_sizes list. Both sibling recipes in the same directory, dsv4_fp4_b200_vllm_mtp.sh and dsv4_fp4_b300_vllm_mtp.sh, hit this identical interaction (same NUM_SPEC_TOKENS=3) and each carry an in-repo comment describing it verbatim ("vLLM's FULL_DECODE_ONLY ladder tops out at MAX_NUM_SEQS/(1+N) seqs and the largest decode batches fall back to eager"). B200 fixes it with MAX_CUDAGRAPH_CAPTURE_SIZE=0 passed via --max-cudagraph-capture-size; B300 builds an explicit cudagraph_capture_sizes list of every multiple of TOKENS_PER_SEQ up to MAX_NUM_SEQS*TOKENS_PER_SEQ. Neither mitigation is present in the MI355X script under review.
Step-by-step proof (TP8+EP8, DP-attn cell at c224): MAX_NUM_SEQS = 2*CONC = 448. With NUM_SPEC_TOKENS=3, TOKENS_PER_SEQ=4. vLLM's default ladder for max_num_seqs=448 includes sizes up through 448 (in seqs, i.e. tokens under the 1-token/seq assumption). adjust_cudagraph_sizes_for_spec_decode() rounds every size up to the next multiple of 4 and dedups — so a would-be capture point at, say, 400 seqs rounds to 400 tokens = 100 seqs worth of MTP-verified batch, and no capture point exists above roughly 448/4 = 112 seqs. Any decode batch above ~112 seqs at this cell (which is exactly the new high-concurrency point this PR adds) runs eager instead of a captured graph, degrading measured decode throughput at precisely the topology (DP8+EP8, up to c224) that the PR states is 'the intended throughput topology.'
Impact: this is a pure benchmarking recipe whose only deliverable is throughput numbers; an unintended eager fallback at the largest, newest concurrency cells silently produces degraded/misleading numbers rather than a crash, which is worse for a benchmark refresh whose stated purpose is establishing an accurate frontier.
Fix: mirror either sibling's mitigation — add NUM_SPEC_TOKENS-derived TOKENS_PER_SEQ=$((1 + NUM_SPEC_TOKENS)), compute MAX_CUDAGRAPH_CAPTURE_SIZE=$((MAX_NUM_SEQS * TOKENS_PER_SEQ)), and pass it via --max-cudagraph-capture-size (matching the B200 recipe's simpler approach) alongside the existing --compilation-config flag.
| - { tp: 8, ep: 1, kv-offloading: none, conc-list: [1, 2, 4, 8, 16, 32] } | ||
| - { tp: 8, ep: 1, kv-offloading: dram, kv-offload-backend: { name: hicache }, conc-list: [16, 32, 48, 64] } | ||
|
|
||
| # MTP twin of dsv4-fp4-mi355x-vllm-agentic. The topologies, KV-offload backends, | ||
| # and concurrency points are identical; speculative decoding is enabled on the | ||
| # current immutable ROCm nightly because the base config's retired nightly tag | ||
| # is no longer available from Docker Hub. Throughput uses the three-token golden | ||
| # synthetic acceptance length, while eval-only runs retain real verification. | ||
| # DeepSeek-V4-Pro FP4 MTP refresh for MI355X. The TP8, fit-validated TP4, and | ||
| # DP8+EP8 bands mirror the B200/B300 MTP grids, with extra intermediate cells to | ||
| # resolve the MI355X knee. TP8+EP8 isolates expert-sharding without changing the | ||
| # attention topology. GPU-resident KV remains the only supported MTP path until | ||
| # vLLM's multi-KV-group recovery fix lands. | ||
| dsv4-fp4-mi355x-vllm-agentic-mtp: | ||
| image: vllm/vllm-openai-rocm:nightly-821717118fc26667dd474b9b0ab81d29259dfc5c | ||
| image: vllm/vllm-openai-rocm:v0.26.0 | ||
| model: deepseek-ai/DeepSeek-V4-Pro | ||
| model-prefix: dsv4 | ||
| runner: cluster:mi355x-amds |
There was a problem hiding this comment.
🟡 AGENTS.md requires bilingual PR titles/descriptions (' / <中文标题>' plus a mirroring '## 中文说明' section), but this PR's title and description are English-only. Please add the Chinese title suffix and a '## 中文说明' section mirroring the Summary/Search space/Validation content.
Extended reasoning...
AGENTS.md line 7 states an explicit, named repository convention: "PR and GitHub-issue titles & descriptions must be bilingual — include a Simplified Chinese version in addition to English. Title format: <English title> / <中文标题>. In the PR/issue body, follow the English content with its Chinese translation (e.g. a ## 中文说明 section mirroring the summary...)."
This PR's title, perf(agentx): refresh DeepSeek V4 MTP frontier on MI355X, has no / <中文标题> suffix, and the description contains only English ## Summary, ## Search space, and ## Validation sections — there is no ## 中文说明 (or equivalent) section mirroring that content in Chinese.
Since this is a named, verbatim rule in the repo's own contributor guidance rather than a subjective style preference, it's a legitimate finding even though it concerns PR metadata rather than a specific line in configs/amd-master.yaml (the anchor line is arbitrary — the metadata field itself is what's non-compliant). No code in the diff prevents or enforces this; it's purely a PR-authoring step that was skipped.
Proof: compare the PR title/body against the AGENTS.md template. The template requires <English title> / <中文标题>; the actual title is perf(agentx): refresh DeepSeek V4 MTP frontier on MI355X with nothing after it. The template requires the body's English content to be followed by "its Chinese translation (e.g. a ## 中文说明 section mirroring the summary)"; the actual body ends after ## Validation with no Chinese section at all.
Fix: append / <中文标题> to the PR title with a Simplified Chinese translation of "refresh DeepSeek V4 MTP frontier on MI355X", and add a ## 中文说明 section at the end of the description that mirrors the Summary, Search space, and Validation content in Chinese.
This is metadata-only and has no bearing on the correctness of the config or benchmark script changes, so it should not block merging — it's a nit that the author (or a bot) can fix by editing the PR title/description directly.
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=31341602956 |
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=31348279044 |
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=31350586530 |
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=31354195084 |
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=31380353616 |
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=31384643909 |
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=31389074477 |
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=31393413090 |
|
/stage-results 31357800541 |
|
@cquil11 |
|
/stage-results 31357800541 |
|
@cquil11 staged run 31357800541: https://inferencemax-app-git-staging-semianalysisai.vercel.app/inference?i_dates=2026-08-10~r31357800541 This run remains available across future @cquil11 已将运行 31357800541 发布到预发布环境:https://inferencemax-app-git-staging-semianalysisai.vercel.app/inference?i_dates=2026-08-10~r31357800541 后续的 |
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=31416072428 |
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=31417073980 |
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=31421586014 |
1 similar comment
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=31421586014 |
|
/stage-results 31421586014 |
|
@cquil11 staged run 31421586014: https://inferencemax-app-git-staging-semianalysisai.vercel.app/inference?i_dates=2026-08-10~r31421586014 This run remains available across future @cquil11 已将运行 31421586014 发布到预发布环境:https://inferencemax-app-git-staging-semianalysisai.vercel.app/inference?i_dates=2026-08-10~r31421586014 后续的 |
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=31455195909 |
|
/reuse-sweep-run 31421586014 |
Summary
Search space
The previous official run peaked near 66k total tok/s and its only DP-attention point replicated the full model on each GPU (EP1). The new 40-point matrix mirrors the B200/B300 low-concurrency and high-throughput ranges while adding MI355X intermediate cells to resolve its local knee.
DP4+EP4 was tested and intentionally excluded: the target+MTP model loaded at 224.83 GiB/GPU, but only 12.09 GiB/GPU remained for KV versus 14.88 GiB required for the committed 1,048,576-token model length. The workload is not shortened to force an invalid topology.
Validation
vllm:server metrics, ~72.7k input +414 output tok/s averageDraft while cluster validation is in progress.