[AgentX] Tune DSV4 GB300 MTP / 调优 DSV4 GB300 MTP - #2571
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 关于重新运行失败任务的文档 |
aa7d626 to
39a457c
Compare
|
Claude finished @ivanium's task in 3m 51s —— View job Review of PR #2571
LGTM - no blocking issues found Verified: |
There was a problem hiding this comment.
LGTM — the reported issues are minor nits (changelog blank-line separator, a duplicable search-space entry, and a missing env var on one sibling config) that don't affect correctness of the sweep.
Extended reasoning...
Overview
This PR tunes GB300 AgentX MTP benchmark recipes for DeepSeek-V4-Pro: scheduler/CUDA-graph/memory settings across four existing recipe YAMLs, a new disagg 2P1D DEP8/DEP16 c1024 recipe, wiring changes in configs/nvidia-master.yaml's search-space definitions, and an appended perf-changelog.yaml entry. All changes are benchmark-harness config (SLURM recipes, env vars, vLLM engine args, sweep matrix entries) — no application/serving code paths are touched.
Security risks
None identified. These are benchmark configuration files consumed by an internal SLURM-based sweep harness; there's no user input handling, auth, or crypto surface in scope.
Level of scrutiny
This warrants light scrutiny: it's config-only tuning following the repo's well-established recipe pattern (topology, env vars, gpu-memory-utilization, cudagraph capture sizes), consistent with many prior similar PRs in this repo's history (e.g. the recent Kimi K3 DSpark and GLM-5.2 tuning PRs). The three findings from the bug-hunting pass are all nits: a missing blank-line separator in the changelog, a mergeable duplicate search-space entry, and a missing env var on one sibling config that only affects benchmark-harness flakiness (not served-model correctness). None of them block the sweep from running correctly or produce incorrect results.
Other factors
The PR description states 236 tests passed and changelog validation passed. No CODEOWNER-sensitive code is touched, and there are no unresolved reviewer comments in the timeline. Given the nature and low severity of the findings, human review is not required to safely merge this.
| pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/2475 | ||
| - config-keys: | ||
| - dsv4-fp4-gb300-dynamo-vllm-agentic-mtp-agg |
There was a problem hiding this comment.
🟡 perf-changelog.yaml is append-only; every one of the other 714 entries in the file is preceded by a blank separator line, but the new entry this PR appends (line 5821) runs directly after the prior entry's pr-link at line 5820 with no blank line. AGENTS.md explicitly calls this file 'byte-sensitive' and requires preserving 'separator whitespace' when appending, so please add a blank line before the new - config-keys: entry.
Extended reasoning...
perf-changelog.yaml is documented in AGENTS.md line 21 as append-only and byte-sensitive: "Preserve all existing bytes and separator whitespace, and append only at the tail." Every one of the 714 pre-existing - config-keys: entries in the file is separated from its predecessor by exactly one blank line — this is a consistent, mechanically-enforced convention throughout the entire file, not an incidental style choice.
The new entry appended by this PR (for PR #2571, covering the GB300 DeepSeek-V4-Pro AgentX MTP tuning) breaks that pattern. Concretely:
5818 pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/2475
5819
5820 - config-keys:
...
5820 pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/2475
5821 - config-keys:
5822 - dsv4-fp4-gb300-dynamo-vllm-agentic-mtp-agg
(Verified with cat -A: line 5820, the pr-link of the prior PR #2475 entry, is immediately followed by line 5821, - config-keys: of the new entry — no blank line in between. Every other transition in the file, e.g. the boundary a few lines above at 5814→5816 and 5804→5806, has a blank line inserted between the previous entry's pr-link and the next entry's - config-keys:.)
This does not break YAML parsing — a list of mappings in a block sequence does not require blank lines between items, so the file still loads correctly and any tooling that reads perf-changelog.yaml programmatically will be unaffected. The impact is purely about matching the established formatting convention that the rest of the file (and the maintainers' explicit written instruction in AGENTS.md) enforces.
The fix is a one-line change: insert a blank line between line 5820 (pr-link: .../pull/2475) and line 5821 (- config-keys:) so the new entry is visually and structurally consistent with all 714 entries before it.
This is a formatting/convention nit rather than a functional defect — it does not affect changelog validation, parsing, or any downstream consumer of the file, so it should not block merging, but it is worth fixing to keep the byte-sensitive changelog internally consistent.
| - spec-decoding: mtp | ||
| conc-list: [4] | ||
| prefill: | ||
| num-worker: 1 | ||
| tp: 8 | ||
| ep: 1 | ||
| dp-attn: false | ||
| additional-settings: | ||
| - "CONFIG_FILE=recipes/vllm/deepseek-v4/agentic/agg-gb300-tp8-mtp-agentic.yaml" | ||
| # The aggregate worker also performs decode; keep the decode worker | ||
| # count at zero so result aggregation counts eight GPUs only once. | ||
| decode: | ||
| num-worker: 0 | ||
| tp: 8 | ||
| ep: 1 | ||
| dp-attn: false |
There was a problem hiding this comment.
🟡 The pre-existing conc-list: [1] entry and the newly added conc-list: [4] entry in dsv4-fp4-gb300-dynamo-vllm-agentic-mtp-agg (configs/nvidia-master.yaml:7381-7412) are byte-identical aside from the concurrency value — both point at agg-gb300-tp8-mtp-agentic.yaml with the same prefill/decode topology. Following the file's own conc-list: [a, b, ...] convention (e.g. conc-list: [10, 15, 30, 60]), these should be merged into one entry with conc-list: [1, 4] to drop the ~15 duplicated lines and avoid future drift.
Extended reasoning...
This is a DRY/quality finding, not a functional bug. The PR adds a new search-space entry for dsv4-fp4-gb300-dynamo-vllm-agentic-mtp-agg at conc-list: [4] (configs/nvidia-master.yaml:7397-7412), but the entry it added is a byte-for-byte copy of the entry immediately above it for conc-list: [1] (lines 7381-7396) — same prefill: {num-worker: 1, tp: 8, ep: 1, dp-attn: false}, same additional-settings: CONFIG_FILE=recipes/vllm/deepseek-v4/agentic/agg-gb300-tp8-mtp-agentic.yaml, the same "aggregate worker also performs decode" comment, and the same decode: {num-worker: 0, tp: 8, ep: 1, dp-attn: false}. The only distinguishing field between the two entries is the single concurrency value in conc-list.
This file already has an established idiom for exactly this situation: sweeping several concurrencies against one fixed topology via a single conc-list array rather than duplicating the whole search-space block per concurrency. Examples elsewhere in configs/nvidia-master.yaml include conc-list: [10, 15, 30, 60] and conc-list: [1, 2, 3, 4, 5, 6, 7, 8, 10, 12]. The two entries at issue clearly qualify for that idiom, since merging them changes nothing about what gets benchmarked — the sweep runner iterates conc-list and dispatches one run per value regardless of whether the values live in one array or two separate entries.
Why the existing review/CI process would not have caught this: nothing here is incorrect — the config is valid YAML, produces a fully valid sweep matrix, and both concurrency points get benchmarked exactly as intended. Schema validation and the sweep runner have no reason to flag duplicate blocks as an error, since duplication is not a validity condition, only a maintainability one.
Impact if left as-is: no functional impact on this PR — both concurrencies still run correctly. The downside is purely maintenance risk: since the prefill/decode block is now duplicated across two independent search-space entries, any future change (e.g. bumping gpu-memory-utilization, changing the recipe path, adjusting the additional-settings) has to be applied in both places, and it is easy to update one and forget the other, silently causing the two concurrency points to run different configurations without any error being raised.
Step-by-step proof of duplication:
- Read configs/nvidia-master.yaml lines 7381-7396 (pre-existing
conc-list: [1]entry):spec-decoding: mtp,prefill: {num-worker: 1, tp: 8, ep: 1, dp-attn: false, additional-settings: [CONFIG_FILE=...agg-gb300-tp8-mtp-agentic.yaml]},decode: {num-worker: 0, tp: 8, ep: 1, dp-attn: false}. - Read lines 7397-7412 (new
conc-list: [4]entry): identicalspec-decoding, identicalprefillblock byte-for-byte (same additional-settings string), identical comment, identicaldecodeblock. - Diff the two blocks field-by-field: every key/value pair matches except the
conc-listscalar ([1]vs[4]). - Compare against the file's own convention at, e.g., line 63 (
conc-list: [10, 15, 30, 60]) — a single entry driving a multi-point sweep against one fixed topology, confirming that the file expects duplicate topologies-at-different-concurrencies to be expressed as one entry with an array, not two entries. - Conclude: merging into
conc-list: [1, 4]under the shared prefill/decode block is functionally equivalent and removes the drift risk.
Suggested fix: delete the conc-list: [4] entry (lines 7397-7412) and change conc-list: [1] to conc-list: [1, 4] on the pre-existing entry above it. The adjacent conc-list: [8] entry (tp4, different recipe file) is correctly a separate entry since it uses a different topology.
| tensor-parallel-size: 8 | ||
| pipeline-parallel-size: 1 | ||
| disable-custom-all-reduce: true | ||
| enable-cumem-allocator: true | ||
| attention-config: '{"backend":"FLASHINFER_MLA_SPARSE_DSV4","use_prefill_query_quantization":true,"use_fp4_indexer_cache":true}' | ||
| max-model-len: 1048576 | ||
| max-num-seqs: 32 | ||
| max-num-seqs: 16 | ||
| max-num-batched-tokens: 8192 | ||
| trust-remote-code: true | ||
| no-enable-flashinfer-autotune: true | ||
| block-size: 256 | ||
| compilation-config: '{"cudagraph_mode":"FULL_DECODE_ONLY","mode":0}' | ||
| max-cudagraph-capture-size: 128 | ||
| compilation-config: '{"cudagraph_mode":"FULL_DECODE_ONLY","cudagraph_capture_sizes":[4,8,12,16,20,24,28,32,36,40,44,48,52,56,60,64],"mode":0}' | ||
| speculative-config: '{"method":"mtp","num_speculative_tokens":3,"rejection_sample_method":"synthetic","synthetic_acceptance_length":2.49}' | ||
| gpu-memory-utilization: 0.90 | ||
| gpu-memory-utilization: 0.94 | ||
| stream-interval: 10 | ||
| no-disable-hybrid-kv-cache-manager: true | ||
| tokenizer-mode: "deepseek_v4" |
There was a problem hiding this comment.
🟡 This PR adds AIPERF_DATASET_MMAP_CACHE_ENABLED: "false" (to avoid concurrent aiperf readers observing a mismatched mmap data/index pair) to the benchmark.env of all four other GB300 AgentX MTP configs it touches, but agg-gb300-tp8-mtp-agentic.yaml is skipped even though this same PR tunes it (max-num-seqs, compilation-config, gpu-memory-utilization) and newly wires it into the sweep via nvidia-master.yaml. It still uses the identical AIPERF_DATASET_MMAP_CACHE_DIR mechanism and remains exposed to the same race; add the same env var here for consistency.
Extended reasoning...
This PR introduces AIPERF_DATASET_MMAP_CACHE_ENABLED: "false" with the comment "Avoid concurrent readers observing a mismatched mmap data/index pair" into the benchmark.env block of four configs: agg-gb300-tp4-mtp-agentic.yaml, disagg-gb300-1p1d-dep4-dep8-c256-mtp-agentic.yaml, disagg-gb300-2p1d-dep8-dep12-c1152-mtp-agentic.yaml, and the brand-new disagg-gb300-2p1d-dep8-dep16-c1024-mtp-agentic.yaml. All five configs, including agg-gb300-tp8-mtp-agentic.yaml, share the same benchmark harness invocation (bash /infmax-workspace/benchmarks/multi_node/agentic_srt.sh) and the same AIPERF_DATASET_MMAP_CACHE_DIR: "/aiperf_mmap_cache" setting, which is what makes them susceptible to the underlying aiperf dataset mmap-cache race in the first place.
agg-gb300-tp8-mtp-agentic.yaml is not merely an untouched bystander file: this PR actively edits it (VLLM_V2_WARMUP_MAX_NUM_SEQS 32→16, max-num-seqs 32→16, expanded compilation-config cudagraph capture sizes, gpu-memory-utilization 0.90→0.94, dropped enable-cumem-allocator), and configs/nvidia-master.yaml gains a brand-new search-space entry (spec-decoding: mtp, prefill.tp: 8, additional-settings: CONFIG_FILE=recipes/vllm/deepseek-v4/agentic/agg-gb300-tp8-mtp-agentic.yaml) that pulls this exact recipe into the sweep. So the PR both tunes this config and newly exercises it in CI, yet its benchmark.env (around line 111-126 in the diff / ~146 in the merged file) still only has AIPERF_DATASET_MMAP_CACHE_DIR without the paired AIPERF_DATASET_MMAP_CACHE_ENABLED: "false" guard that all four sibling configs received.
Concrete walk-through of the gap:
agg-gb300-tp4-mtp-agentic.yamlbenchmark.env (post-diff): has bothAIPERF_DATASET_MMAP_CACHE_ENABLED: "false"andAIPERF_DATASET_MMAP_CACHE_DIR: "/aiperf_mmap_cache".disagg-gb300-1p1d-dep4-dep8-c256-mtp-agentic.yaml,disagg-gb300-2p1d-dep8-dep12-c1152-mtp-agentic.yaml, and the newdisagg-gb300-2p1d-dep8-dep16-c1024-mtp-agentic.yaml: same pairing added.agg-gb300-tp8-mtp-agentic.yamlbenchmark.env (post-diff): onlyAIPERF_DATASET_MMAP_CACHE_DIR: "/aiperf_mmap_cache"remains — theENABLED: "false"line was never added, despite this file being edited elsewhere in the same diff for scheduler/cudagraph tuning.- Because the tp8-agg config runs the identical
agentic_srt.shbenchmark client against the same mmap cache directory mechanism, it remains exposed to the intermittent race where a concurrent reader can observe a data file and index file from two different write generations (i.e., a torn/mismatched mmap pair), which the ENABLED:false flag was added everywhere else specifically to prevent.
This looks like a simple "missed one file" inconsistency during a multi-file find-and-fix pass rather than an intentional decision — there's no comment or rationale anywhere explaining why tp8-agg should be treated differently, and the mechanism it uses is byte-for-byte the same as its siblings.
Fix: add the same two lines to agg-gb300-tp8-mtp-agentic.yaml's benchmark.env, mirroring the other four configs exactly:
# Avoid concurrent readers observing a mismatched mmap data/index pair.
AIPERF_DATASET_MMAP_CACHE_ENABLED: "false"
AIPERF_DATASET_MMAP_CACHE_DIR: "/aiperf_mmap_cache"Impact/severity: this is a benchmark-harness reliability issue (a flaky aiperf dataset reader race), not a change in served-model correctness — a hit would produce an occasional re-runnable benchmark flake rather than a silent wrong result or a hard failure. Given that scope, this is a nit: worth fixing for consistency across the sibling configs newly wired into the same sweep, but not a merge blocker.
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=31583491445 |
15557ef to
ce61fdc
Compare
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=31585588203 |
ce61fdc to
7e5fb67
Compare
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=31585956705 |
Consolidate the GB300 AgentX MTP bring-up, runner selection, Dynamo transport hardening, AIPerf integration, and six-point aggregate/P-D sweep. Keep DeepGEMM AMX FP4 Mega MoE on expert-parallel P/D engines while restoring automatic MoE backend selection for pure aggregate TP. Disable the vLLM cumem allocator for aggregate TP, retain the 8192-token budget, size max-num-seqs as max(16, 4*concurrency) with matching MTP CUDA graph coverage, and use gpu-memory-utilization 0.94. 中文:整合 GB300 AgentX MTP 的启动流程、运行节点选择、Dynamo 传输加固、AIPerf 集成和六点聚合/P-D 扫描。在专家并行 P/D 引擎上保留 DeepGEMM AMX FP4 Mega MoE,并为纯聚合 TP 恢复自动 MoE 后端选择;聚合 TP 禁用 vLLM cumem 分配器并保留 8192-token 预算,同时按 max(16, 4*并发) 设置 max-num-seqs、配置对应的 MTP CUDA Graph 覆盖范围,并将 gpu-memory-utilization 设为 0.94。
Route GB300 multi-node AgentX recipes through the shared throughput-only synthetic acceptance injector and add regression coverage for eval-only behavior.\n\n中文:让 GB300 多节点 AgentX 配方使用仅吞吐量启用的共享合成接受率注入器,并添加仅评估模式的回归测试。
1f70b31 to
9926c05
Compare
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=31624709550 |
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=31633154542 |
1 similar comment
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=31633154542 |
|
/stage-results 31624709550 |
|
@ivanium staged run 31624709550: https://inferencemax-app-git-staging-semianalysisai.vercel.app/inference?i_dates=2026-08-12~r31624709550 This run remains available across future |
|
/stage-results 31633154542 |
|
@ivanium staged run 31633154542: https://inferencemax-app-git-staging-semianalysisai.vercel.app/inference?i_dates=2026-08-12~r31633154542 This run remains available across future |
Tune the GB300 AgentX MTP configs.
调优 GB300 AgentX MTP 配置。
Tests / 测试: 236 passed; changelog validation passed.