-
Notifications
You must be signed in to change notification settings - Fork 253
perf(agentx): refresh DeepSeek V4 MTP frontier on MI355X #2540
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
8c63451
2d5d0c2
323fc15
4d77e28
3c2131e
d813d64
8e6d1b2
cbbc4bb
96703c9
747b36a
11edf7a
86773f8
9e245f4
1de2b07
4c52de6
70a42ce
b244cb0
428e6b7
f894cc8
1b48a1a
9d0cdf9
8a9cc12
42fc554
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1227,13 +1227,13 @@ qwen3.5-fp8-mi355x-sglang-agentic-hicache: | |
| - { 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 collection sweep for MI355X. Keep the verified | ||
| # GPU-resident frontier, add denser c52/c60/c68 TEP guards, collect the same | ||
| # points with pinned Mooncake DRAM offload, and retain representative DEP8 and | ||
| # DEP4 cells even when dominated. The fast pass is a functionality gate; | ||
| # Pareto optimality is deliberately not required for inclusion. | ||
| dsv4-fp4-mi355x-vllm-agentic-mtp: | ||
| image: vllm/vllm-openai-rocm:nightly-821717118fc26667dd474b9b0ab81d29259dfc5c | ||
| image: vllm/vllm-openai-rocm:v0.27.0 | ||
| model: deepseek-ai/DeepSeek-V4-Pro | ||
| model-prefix: dsv4 | ||
| runner: cluster:mi355x-amds | ||
|
Comment on lines
1227
to
1239
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 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: This PR's title, 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 Proof: compare the PR title/body against the AGENTS.md template. The template requires Fix: append 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. |
||
|
|
@@ -1244,11 +1244,17 @@ dsv4-fp4-mi355x-vllm-agentic-mtp: | |
| agentic-coding: | ||
| - dram-utilization: 0.60 | ||
| search-space: | ||
| - { tp: 8, ep: 1, dp-attn: false, kv-offloading: none, spec-decoding: mtp, conc-list: [1, 4, 8, 16, 32, 40, 48] } | ||
| - { tp: 8, ep: 1, dp-attn: true, kv-offloading: none, spec-decoding: mtp, conc-list: [64], router: { name: vllm-router, version: "0.1.14" } } | ||
| # LMCache invalid-block recovery currently assumes one KV-cache group, | ||
| # while MTP creates two. Restore these points after the upstream hybrid | ||
| # KV recovery fix lands: https://github.com/vllm-project/vllm/pull/45497 | ||
| - { tp: 8, ep: 1, dp-attn: false, kv-offloading: none, spec-decoding: mtp, conc-list: [1, 2, 4, 6, 8, 12] } | ||
| - { tp: 8, ep: 8, dp-attn: false, kv-offloading: none, spec-decoding: mtp, conc-list: [16, 24, 32, 40, 48, 52, 56, 60, 64, 68] } | ||
| - { tp: 8, ep: 8, dp-attn: false, kv-offloading: dram, kv-offload-backend: { name: mooncake, version: "0.3.11.post1" }, spec-decoding: mtp, conc-list: [52, 60, 68] } | ||
| - { tp: 8, ep: 8, dp-attn: true, kv-offloading: none, spec-decoding: mtp, conc-list: [64, 128, 192, 224], router: { name: vllm-router, version: "0.1.14" } } | ||
| - { tp: 4, ep: 4, dp-attn: true, kv-offloading: none, spec-decoding: mtp, conc-list: [4, 8], router: { name: vllm-router, version: "0.1.14" } } | ||
| # DEP4 previously had only 12.09 GiB/GPU available for the native | ||
| # 1,048,576-token context versus a 14.88 GiB minimum. Retain two cells in | ||
| # the fast gate to reconfirm whether the current base can now initialize; | ||
| # do not weaken the AgentX context limit to make them pass. | ||
| # The stable-v0.27 Mooncake c52 gate completed its full 575-request | ||
| # warmup and measured profile without hybrid-KV or request errors. | ||
|
|
||
| dsr1-fp4-mi355x-sglang-disagg-mtp: | ||
| image: lmsysorg/sglang-rocm:v0.5.12-rocm720-mi35x-20260519 | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔴 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-configfrom{"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 runsspec-decoding: mtpwithNUM_SPEC_TOKENS=3, soTOKENS_PER_SEQ = 1 + NUM_SPEC_TOKENS = 4.Why it triggers: vLLM auto-derives its CUDA-graph capture-size ladder from
--max-num-seqsassuming one token per sequence. When speculative decoding is active andcudagraph_modeisFULL_DECODE_ONLY,adjust_cudagraph_sizes_for_spec_decode()rounds each auto-derived size up to the nearest multiple ofTOKENS_PER_SEQand dedups the resulting list. With no explicit override, this collapses the usable capture range down to roughlyMAX_NUM_SEQS / TOKENS_PER_SEQseqs — i.e.MAX_NUM_SEQS / 4here. 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 toFULL_DECODE_ONLYremoves that fallback path, and this PR's--compilation-configJSON only setsmodeandcudagraph_mode— no--max-cudagraph-capture-sizeflag, and no explicitcudagraph_capture_sizeslist. Both sibling recipes in the same directory,dsv4_fp4_b200_vllm_mtp.shanddsv4_fp4_b300_vllm_mtp.sh, hit this identical interaction (sameNUM_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 withMAX_CUDAGRAPH_CAPTURE_SIZE=0passed via--max-cudagraph-capture-size; B300 builds an explicitcudagraph_capture_sizeslist of every multiple ofTOKENS_PER_SEQup toMAX_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. WithNUM_SPEC_TOKENS=3,TOKENS_PER_SEQ=4. vLLM's default ladder formax_num_seqs=448includes 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 roughly448/4 = 112seqs. 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-derivedTOKENS_PER_SEQ=$((1 + NUM_SPEC_TOKENS)), computeMAX_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-configflag.