[AMD] [AGENTX] dsv4-fp4-mi355x-vllm-agentic: DEP tuning + LMCache refresh (MTP variant) - #2381
[AMD] [AGENTX] dsv4-fp4-mi355x-vllm-agentic: DEP tuning + LMCache refresh (MTP variant)#2381seungrokj wants to merge 29 commits into
Conversation
New MTP agentic recipe for DeepSeek-V4-Pro FP4 on MI355X vLLM, derived from the non-MTP recipe with --speculative-config method=mtp num_speculative_tokens=3. Throughput runs pin synthetic MTP acceptance to the dsv4-pro golden AL; EVAL_ONLY uses real target verification (synthetic acceptance corrupts the eval score). Carries the shared fixes: EVAL_FRAMEWORK=lm-eval, per-DP-rank max-num-seqs, DEP8 max-num-batched-tokens MNBT*2, GPU_MEM_UTIL=0.85, lmcache v0.5.2. amd-master.yaml: add dsv4-fp4-mi355x-vllm-agentic-mtp entry (TP8/DPA GPU-resident conc 48; other arms commented for later sweeps). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
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 关于重新运行失败任务的文档 |
…2381 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
seungrokj
changed the title
seungrokj
added
AMD
full-sweep-fail-fast
evals-only
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| # MTP: cudagraph capture sizes are in TOKENS. With num_speculative_tokens=N, | ||
| # every uniform decode batch of S seqs verifies S*(1+N) tokens, so capture the | ||
| # explicit multiples (1+N), 2*(1+N), ..., MAX_NUM_SEQS*(1+N) -- one graph per | ||
| # decode batch of 1..MAX_NUM_SEQS seqs. vLLM rounds configured sizes up to | ||
| # multiples of (1+N) and dedups (adjust_cudagraph_sizes_for_spec_decode), so a | ||
| # plain 1..MAX_NUM_SEQS list would collapse to coverage of only | ||
| # MAX_NUM_SEQS/(1+N) seqs and drop the largest decode batches to eager. | ||
| NUM_SPEC_TOKENS=3 | ||
| TOKENS_PER_SEQ=$((1 + NUM_SPEC_TOKENS)) |
There was a problem hiding this comment.
🔴 TOKENS_PER_SEQ is computed at line 421 (with a comment explaining that explicit cudagraph_capture_sizes of num_seqs*TOKENS_PER_SEQ must be passed for each num_seqs in 1..MAX_NUM_SEQS to avoid vLLM's adjust_cudagraph_sizes_for_spec_decode dropping the largest decode batches to eager) but is never actually applied — the --compilation-config on line 462 has no cudagraph_capture_sizes key. This is an incomplete port of the sibling dsv4_fp4_b300_vllm_mtp.sh recipe, which builds and embeds this list via a loop; at the only active arm (conc=48), the largest decode batches will silently fall back to eager execution, undercutting the MTP throughput this benchmark is meant to measure.
Extended reasoning...
What the bug is: Lines 413-419 of dsv4_fp4_mi355x_vllm_mtp.sh contain a comment — copied verbatim from benchmarks/single_node/agentic/dsv4_fp4_b300_vllm_mtp.sh — explaining that with MTP speculative decoding, cudagraph capture sizes must be specified in tokens, not sequences: since every uniform decode batch of S sequences verifies S*(1+N) tokens (where N = num_speculative_tokens), the recipe needs to pass explicit capture sizes 1*(1+N), 2*(1+N), ..., MAX_NUM_SEQS*(1+N) via --compilation-config's cudagraph_capture_sizes field. Otherwise vLLM's adjust_cudagraph_sizes_for_spec_decode rounds/dedups the default capture-size list to multiples of (1+N), which collapses coverage and drops the largest decode batches to eager execution.
Line 421 computes TOKENS_PER_SEQ=$((1 + NUM_SPEC_TOKENS)) in preparation for this — but that's the last time the variable is referenced anywhere in the file. The actual --compilation-config passed to vllm serve on line 462 is just the plain '{"mode":3,"cudagraph_mode":"FULL_DECODE_ONLY"}', with no cudagraph_capture_sizes key at all.
Code path / proof: Compare directly against the sibling recipe this was derived from, benchmarks/single_node/agentic/dsv4_fp4_b300_vllm_mtp.sh (lines ~254-279 in that file): it carries the identical explanatory comment, computes the identical TOKENS_PER_SEQ = 1 + NUM_SPEC_TOKENS, and then actually uses it — a for ((num_seqs=1; num_seqs<=MAX_NUM_SEQS; num_seqs++)) loop builds a CUDA_GRAPH_CAPTURE_SIZES string of num_seqs*TOKENS_PER_SEQ values, which is embedded into COMPILATION_CONFIG's cudagraph_capture_sizes field and passed to vllm serve. The MI355X MTP script under review copied the comment and the TOKENS_PER_SEQ line but dropped both the loop and the resulting compilation-config field — a clear incomplete copy-paste. A grep -n "TOKENS_PER_SEQ\|compilation-config" over the file confirms only two hits: the dead assignment at line 421 and the plain config at line 462, with no cudagraph_capture_sizes anywhere.
Concretely, for the only active arm in configs/amd-master.yaml (dp-attn: true, TP8, conc-list: [48]), MAX_NUM_SEQS = 2*48/8 = 12 per DP rank, and TOKENS_PER_SEQ = 1 + 3 = 4. The comment's own stated intent is to capture token-sizes 4, 8, 12, ..., 48 (one graph per decode batch of 1..12 sequences). Without explicit sizes, vLLM's adjust_cudagraph_sizes_for_spec_decode instead rounds/dedups its own default size list to multiples of 4, which per the comment's own logic collapses coverage and drops the largest decode batches to eager execution — silently degrading exactly the throughput this MTP recipe exists to measure.
Why nothing else catches this: There's no functional/type check that would fail — TOKENS_PER_SEQ is just an unused shell variable (a shellcheck SC2034 case), and the server will start and serve requests fine with the plain compilation-config; it just does so less efficiently for the largest decode batches, which won't surface as a CI failure, only as suppressed throughput numbers in benchmark output.
Fix: Port the loop from dsv4_fp4_b300_vllm_mtp.sh — build a CUDA_GRAPH_CAPTURE_SIZES array/string over num_seqs in 1..MAX_NUM_SEQS multiplying by TOKENS_PER_SEQ, and embed it as a cudagraph_capture_sizes key in the --compilation-config JSON passed to vllm serve on line 462.
Severity: I'm marking this normal rather than nit. While it won't crash the server or fail CI, the entire purpose of this new recipe is to measure MTP throughput, and this omission causes the benchmark to systematically under-measure that throughput at its one active concurrency by falling back to eager execution for the largest decode batches — a real, silent measurement defect in the recipe's core deliverable, not a cosmetic issue.
| LMCACHE_PID="" | ||
|
|
||
| cleanup_lmcache_server() { | ||
| if [[ -n "$LMCACHE_PID" ]] && kill -0 "$LMCACHE_PID" 2>/dev/null; then | ||
| kill "$LMCACHE_PID" 2>/dev/null || true | ||
| wait "$LMCACHE_PID" 2>/dev/null || true | ||
| fi | ||
| } | ||
|
|
||
| trap cleanup_lmcache_server EXIT | ||
|
|
||
| cleanup_agentic_services() { | ||
| local exit_code=$? | ||
| trap - EXIT INT TERM | ||
| set +e | ||
| stop_background_process_tree "$ROUTER_PID" "vLLM router" | ||
| stop_background_process_tree "$SERVER_PID" "vLLM server" 60 | ||
| stop_background_process_tree "$MOONCAKE_MASTER_PID" "Mooncake master" | ||
| exit "$exit_code" | ||
| } | ||
| trap cleanup_agentic_services EXIT |
There was a problem hiding this comment.
🟡 The lmcache-backend branch registers 'trap cleanup_lmcache_server EXIT' (line 236) to reap the standalone LMCache MP server, but it is immediately overwritten by 'trap cleanup_agentic_services EXIT' (line 247) — bash EXIT traps don't stack, so only the latter runs and LMCACHE_PID is never killed, leaking the process (and its DRAM pool + ports 5555/8080) on every exit. This is dormant today since the only active amd-master.yaml arm uses kv-offloading:none, but will trigger the moment a commented-out lmcache arm is enabled; fix by folding LMCACHE_PID teardown into cleanup_agentic_services. Note the identical bug already exists in the sibling dsv4_fp4_mi355x_vllm.sh this file was derived from.
Extended reasoning...
The bug: In the lmcache KV_OFFLOAD_BACKEND branch, line 236 registers trap cleanup_lmcache_server EXIT to kill the standalone LMCache MP server (LMCACHE_PID) on exit. Eleven lines later, line 247 registers trap cleanup_agentic_services EXIT. Bash traps do not stack per signal — only the most recently registered handler for a given signal fires. So cleanup_lmcache_server becomes dead code from the moment it's overwritten, and only cleanup_agentic_services ever runs on exit.
Why the surviving handler doesn't cover it: cleanup_agentic_services calls stop_background_process_tree on ROUTER_PID, SERVER_PID, and MOONCAKE_MASTER_PID only. LMCACHE_PID is never referenced inside it. The LMCache MP server is started later ("${LMCACHE_CMD[@]}" ... & LMCACHE_PID=$!) as a direct child of the script, not a descendant of any of the three tracked process trees, so none of the existing cleanup calls can reap it either directly or transitively.
Trigger path: Any run with KV_OFFLOAD_BACKEND=lmcache hits this on every exit path — normal completion, INT (routes to exit 130), and TERM (routes to exit 143) all funnel through the single surviving EXIT trap. cleanup_agentic_services runs, disables all traps (trap - EXIT INT TERM), stops the three tracked processes, and exits — leaving the LMCache server process running as an orphan.
Step-by-step proof:
- Script enters the
lmcache)case arm, definescleanup_lmcache_server, and runstrap cleanup_lmcache_server EXIT(line 236). At this point, EXIT →cleanup_lmcache_server. - Immediately after, the script defines
cleanup_agentic_servicesand runstrap cleanup_agentic_services EXIT(line 247). Bash's trap table now maps EXIT →cleanup_agentic_servicesonly; the line-236 registration is gone with no way to recover it (traps replace, they don't compose). - Later (line ~350) the script launches
"${LMCACHE_CMD[@]}" > "$LMCACHE_LOG" 2>&1 &and capturesLMCACHE_PID=$!. - Script eventually exits (success, or via the INT/TERM handlers calling
exit). The EXIT trap firescleanup_agentic_services, which doestrap - EXIT INT TERM, stops router/server/mooncake-master, and callsexit "$exit_code". cleanup_lmcache_servernever executes;LMCACHE_PIDwas never signaled. The LMCache server process, holding its large CPU KV pool and bound to ports 5555/8080, survives the script's exit as an orphan.
Impact: On a shared/reused node, the orphaned LMCache server continues holding host DRAM (the recipe sizes this pool from TOTAL_CPU_DRAM_GB, intended to be a large fraction of ~2.7 TiB) and keeps ports 5555/8080 bound, which would block or corrupt a subsequent lmcache run on the same node (port-bind failure or unexpected interaction with a stale server).
Why it doesn't fail this PR today: The sole active search-space arm for dsv4-fp4-mi355x-vllm-agentic-mtp in configs/amd-master.yaml is kv-offloading: none — every lmcache arm is commented out. So the lmcache) case branch, and this trap-overwrite bug, cannot execute in the current config. It's genuinely dormant, but it is new code introduced by this PR and copied verbatim from the pre-existing bug in the sibling benchmarks/single_node/agentic/dsv4_fp4_mi355x_vllm.sh.
Fix: Fold LMCACHE_PID teardown into cleanup_agentic_services (e.g. add a stop_background_process_tree "$LMCACHE_PID" "LMCache server" call, guarded by the variable being set) and drop the now-redundant standalone cleanup_lmcache_server/first trap ... EXIT registration, so there is exactly one EXIT handler that tears down every backgrounded process.
| agentic-coding: | ||
| - dram-utilization: 0.80 | ||
| search-space: | ||
| #- { tp: 8, ep: 1, dp-attn: false, kv-offloading: none, conc-list: [1, 4, 8, 16, 32, 40, 48] } | ||
| #- { tp: 8, ep: 1, dp-attn: true, kv-offloading: none, conc-list: [64, 72], router: { name: vllm-router, version: "0.1.14" } } | ||
| #- { tp: 8, ep: 1, dp-attn: false, kv-offloading: dram, kv-offload-backend: { name: lmcache, version: "9229067cec0b3a63bb8a39368d101db7ac0bc3c1" }, conc-list: [32, 40, 48] } | ||
| #- { tp: 8, ep: 1, dp-attn: true, kv-offloading: dram, kv-offload-backend: { name: lmcache, version: "v0.5.2" }, conc-list: [76] } | ||
| - { tp: 8, ep: 1, dp-attn: true, kv-offloading: none, conc-list: [48] } |
There was a problem hiding this comment.
🟡 The active dp-attn:true search-space row (line 1555) is missing the router: { name: vllm-router, version: "0.1.14" } field that every other dp-attn agentic row in this file carries (including this entry's own commented-out equivalent on line 1552). The launch script hardcodes USE_VLLM_ROUTER=true and deploys vllm-router 0.1.14 whenever DP_ATTENTION=true, so the router really is running — but since generate_sweep_configs.py only pulls router metadata from this YAML field, the published result for this run will be missing router attribution.
Extended reasoning...
The new dsv4-fp4-mi355x-vllm-agentic-mtp entry's only active search-space row (configs/amd-master.yaml:1555, { tp: 8, ep: 1, dp-attn: true, kv-offloading: none, conc-list: [48] }) omits the router field. This is inconsistent with every other dp-attn: true agentic row in the file, including:
- The sibling non-MTP entry
dsv4-fp4-mi355x-vllm-agentic's equivalentdp-attn: truerow (line 1531), which carriesrouter: { name: vllm-router, version: "0.1.14" }. - This entry's own commented-out
dp-attn: truerow (line 1552), which also carries the samerouterfield — strong evidence the field was dropped when the commented row was copied into the active one, rather than omitted intentionally.
The code path that makes this matter: utils/matrix_logic/generate_sweep_configs.py's component_metadata() (lines 194-201) resolves router metadata via benchmark.get('router', config.get('router')) — i.e. only from the top-level recipe entry or the per-row router field in the YAML. The MTP entry has neither a top-level router: key nor a router field on its active row, so component_metadata() returns no router info. That flows into ROUTER_METADATA (benchmark-tmpl.yml:173), which process_result.py (lines 77-79) only writes into the published result's router key when present.
Meanwhile, the new launch script benchmarks/single_node/agentic/dsv4_fp4_mi355x_vllm_mtp.sh sets USE_VLLM_ROUTER=true and runs agentic_pip_install --quiet "vllm-router==$VLLM_ROUTER_VERSION" (VLLM_ROUTER_VERSION=0.1.14) purely based on DP_ATTENTION=true, independent of the YAML router field. So the run genuinely deploys vllm-router 0.1.14 as a component of the serving stack, but the published benchmark result for this arm will have no router key at all, unlike every sibling dp-attn agentic result.
Concrete walkthrough:
- Sweep config generation reads the active row
{ tp: 8, ep: 1, dp-attn: true, kv-offloading: none, conc-list: [48] }— norouterkey present, and thedsv4-fp4-mi355x-vllm-agentic-mtpentry has no benchmark-levelrouter:either. component_metadata()computesROUTER = benchmark.get('router', config.get('router'))→Nonefor this row.ROUTER_METADATAenv var is left unset for the job.- The launch script independently sees
DP_ATTENTION=trueand setsUSE_VLLM_ROUTER=true, installs and startsvllm-router==0.1.14, and actually routes traffic through it. process_result.pychecksget_optional_component_metadata('ROUTER_METADATA'), finds nothing, and omitsrouterfrom the published result JSON.- Result: dashboards/analysis for this arm silently lack router attribution that every other dp-attn agentic arm (including the sibling non-MTP entry) records, even though the underlying infra is identical.
Fix: add router: { name: vllm-router, version: "0.1.14" } to the active row at line 1555, matching the commented-out row directly above it and the sibling non-MTP entry.
Impact assessment: this is metadata/attribution-only — the router field is Optional throughout validation.py, nothing crashes, the run itself is unaffected, and benchmark numbers (throughput/latency) are computed independent of this field. It does not block the sweep or change measured performance, so this is a real but low-severity oversight worth a one-line fix before merge.
Override the per-DP-rank sizing for the MTP recipe; use 2*CONC unconditionally for better throughput. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=30365204442 |
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=30366386876 |
1 similar comment
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=30366386876 |
…tp + vllm-router Add spec-decoding: mtp to all search-space arms so the launcher's SPEC_SUFFIX routes to dsv4_fp4_mi355x_vllm_mtp.sh (without it, SPEC_DECODING is none and the non-MTP script runs); add the vllm-router 0.1.14 to the active DP-attention arm. Fix a missing closing brace on the active-arm flow mapping. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=30368590483 |
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=30369745857 |
…k max-num-seqs Restore MAX_NUM_SEQS=2*CONC/TP under DP-attention (2*CONC in pure-TP). At 2*CONC (=96) the MTP cudagraph capture list grew to 512 sizes and the activation budget left only 23.36 GiB for KV cache -- below the 24.06 GiB needed for one max-len request, so engine init failed in _check_enough_kv_cache_memory. Per-rank sizing shrinks the capture/activation footprint so KV cache fits. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=30372160051 |
…86 for MTP KV headroom At 0.85 the ~23.9 GiB KV budget falls below the 24.06 GiB one 1M-context request needs once the MTP draft layer's per-token KV is counted, so engine init dies on the tighter eval-only relaunch. 0.86 adds ~2.6 GiB (nearly all to KV) while staying ~8 GiB under the free-mem hard-fail ceiling. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=30414612657 |
seungrokj
removed
the
evals-only
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=30416717474 |
…oad-backend key Remove the accidentally duplicated kv-offload-backend key on the vllm-native dram arm. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=30455805721 |
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=30455957352 |
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=30491144291 |
… versions, raise GPU_MEM_UTIL to 0.90 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=30501346390 |
…n to 0.60 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ram arm Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=30502809347 |
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=30503199791 |
…5.2 and set DP-attn max-num-seqs to CONC Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
@seungrokj the AgentX/AIPerf harness has been updated, please merge origin/main into your branch and refresh your submission. Additional tuning may be necessary depending on the config. I apologize for any inconvenience. This is an automated message. |
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=30510355442 |
…witch to kvnone pure-TP sweep Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=30524634428 |
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=30527169624 |
2 participants
Summary
Adds an MTP (multi-token-prediction) variant of the DeepSeek-V4-Pro FP4 MI355X vLLM agentic recipe.
benchmarks/single_node/agentic/dsv4_fp4_mi355x_vllm_mtp.sh(new)dsv4_fp4_mi355x_vllm.sh, adds--speculative-configwithmethod=mtp,num_speculative_tokens=3.synthetic_acceptance_length=2.49);EVAL_ONLYaccuracy runs use real target verification (synthetic acceptance bypasses verification and zeroes the eval score).EVAL_FRAMEWORK=lm-eval; per-DP-rank--max-num-seqs(2*CONC/TPunder DP-attn); DEP8--max-num-batched-tokens=MNBT*2(16384, avoids the rocBLASLt/TensileLite segfault at M=65536);GPU_MEM_UTIL=0.85(MI355X ~32 GiB/GPU carveout → 0.95 OOMs at init); lmcachev0.5.2, chunk-size 1024, undivided CPU pool.configs/amd-master.yamldsv4-fp4-mi355x-vllm-agentic-mtpentry; active arm TP8/DPA GPU-resident (kv-offloading: none) conc 48 (other arms commented for later sweeps).Test plan
dsv4-fp4-mi355x-vllm-agentic-mtp--speculative-configMTP (no init OOM at 0.85)🤖 Generated with Claude Code