Test DeepSeek-V4 GB200 Dynamo AgentX recipes / 测试 DeepSeek-V4 GB200 Dynamo AgentX 配方#2302
Test DeepSeek-V4 GB200 Dynamo AgentX recipes / 测试 DeepSeek-V4 GB200 Dynamo AgentX 配方#2302ivanium wants to merge 5 commits into
Conversation
Add the aggregate TP8 and four disaggregated DP/EP topologies validated by PR #2260. Use current NVIDIA/srt-slurm main for upstream per-node DP, health-count, and Mooncake fixes while retaining the still-required multi-node TP port patch. Configure the TP8 path to use FlashInfer all-reduce. Limit this validation sweep to TP8 c16 and representative 1P1D DEP8/DEP8 c256. 中文：新增 DeepSeek-V4 GB200 Dynamo AgentX 聚合式 TP8 配置及四种分离式 DP/EP 拓扑，均已通过 PR #2260 验证。改用 NVIDIA/srt-slurm 最新 main 分支以复用已上游的单节点内 DP 启动、健康检查计数和 Mooncake 修复，仅保留多节点 TP 仍需的端口补丁；同时为 TP8 启用 FlashInfer all-reduce。本次验证扫描仅运行 TP8 c16 和代表性的 1P1D DEP8/DEP8 c256。
Remove the redundant local srt-slurm patch now that main unsets VLLM_PORT for multi-node vLLM endpoints. 中文：依赖上游的 VLLM_PORT 处理。当前 srt-slurm main 已会为跨节点 vLLM 端点取消设置 VLLM_PORT，因此移除冗余的本地补丁。
|
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 关于重新运行失败任务的文档 |
Point the performance changelog at the upstream-hosted replacement PR. 中文：更新替代 PR 链接。将性能变更日志指向由上游仓库分支创建的新 PR。
|
Claude finished @ivanium's task in 6m 24s —— View job PR Review
SummaryFound 1 blocking issue and 2 warnings (details in inline comments):
The rest looks good: new recipe YAMLs are well-documented, the |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| - config-keys: | ||
| - dsv4-fp4-gb200-dynamo-vllm-agentic-agg | ||
| - dsv4-fp4-gb200-dynamo-vllm-agentic-3p1d-dep8-dep16 | ||
| description: | ||
| - "Add GB200 Dynamo-vLLM AgentX aggregate TP8 at conc [16] and disaggregated 3P/1D DEP8/DEP16 at conc [1536]." | ||
| - "Use NVIDIA/srt-slurm main with its upstream per-node DP, multi-node TP VLLM_PORT, Dynamo health-count, and Mooncake master fixes. TP8 uses FlashInfer all-reduce and disables vLLM custom all-reduce." | ||
| - "Image: vllm/vllm-openai:nightly-dev-arm64-cu13.0.1-c188b96" | ||
| pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/2302 |
There was a problem hiding this comment.
🔴 BLOCKING: The check-changelog job is failing and gating the entire sweep (all matrix jobs skipped) because this branch is stale relative to main.
Why it matters: utils/process_changelog.py validates with a two-dot git diff origin/main <head> (process_changelog.py:21). Since this branch forked, main gained a changelog entry for PR #2293 (dsv4-fp4-mi355x-sglang-disagg). That entry exists on main but not on this branch, so the diff shows it as a deletion and the validator hard-fails with Deletions are not allowed in perf-changelog.yaml — see the failed run. No benchmark validation can run until this is fixed.
Fix: Merge the latest origin/main into this branch (keeping this new entry at the very end of the file, after the #2293 entry), then re-trigger the sweep.
| if [[ "${IS_AGENTIC:-0}" == "1" ]]; then | ||
| # AgentX was tuned against the canonical DeepSeek-V4-Pro | ||
| # checkpoint, which is staged on compute-visible Lustre. | ||
| export MODEL_PATH="/mnt/lustre01/models/DeepSeek-V4-Pro/" | ||
| else | ||
| # Existing fixed-sequence GB200 recipes use the NVIDIA ModelOpt | ||
| # NVFP4 checkpoint. | ||
| export MODEL_PATH="/mnt/lustre01/models/DeepSeek-V4-Pro-NVFP4/" | ||
| fi |
There was a problem hiding this comment.
🟡 WARNING: This checkpoint switch is keyed on IS_AGENTIC alone, so it also silently changes the model for the two existing GB200 agentic configs — dsv4-fp4-gb200-dynamo-vllm-agentic-2p1d-dep8-dep8 and dsv4-fp4-gb200-dynamo-vllm-agentic-3p2d-tep8-tp8 (both dsv4 + fp4 + agentic-coding, so they hit this branch too).
Why it matters: Those configs' published results were measured against the ModelOpt NVFP4 checkpoint (DeepSeek-V4-Pro-NVFP4/). After this PR they will load DeepSeek-V4-Pro/ instead, so their numbers can shift under unchanged config keys, and the perf-changelog.yaml entry only lists the two new keys — the change to the existing keys would be undocumented.
Fix: Either scope this to the new recipes (e.g., key off the CONFIG_FILE/topology rather than IS_AGENTIC broadly), or — if moving all GB200 AgentX runs to the canonical checkpoint is intentional — add the two existing config keys to the perf-changelog entry and confirm they were re-validated against it.
| if [[ "$IS_AGENTIC" == "1" ]]; then | ||
| # Agentic multi-node uses the same pinned cquil11/srt-slurm-nv commit as | ||
| # launch_gb300-nv.sh — everything the agentic recipes need is there: | ||
| # - BenchmarkType.CUSTOM + benchmark.command + benchmark.env | ||
| # (the hook that hands off to benchmarks/multi_node/agentic_srt.sh) | ||
| # - DynamoConfig.wheel (recipes pin the ai-dynamo wheel) | ||
| # - srtctl apply --no-preflight (model path /mnt/numa1 is compute-node | ||
| # local NVMe, invisible to the login-node runner) | ||
| # - benchmark_stage srun_options propagation (container-remap-root | ||
| # must reach the agentic_srt.sh srun) | ||
| git clone https://github.com/cquil11/srt-slurm-nv.git "$SRT_REPO_DIR" | ||
| cd "$SRT_REPO_DIR" | ||
| git checkout de59739b172e507e15ebf145bfe305f606e82fbf | ||
| mkdir -p recipes/vllm/deepseek-v4/agentic | ||
| # Agentic recipes use NVIDIA/srt-slurm:main. The per-node DP launcher, | ||
| # matching Dynamo health counts, multi-node TP VLLM_PORT handling, and | ||
| # Mooncake master compatibility are all upstream. | ||
| git clone --branch main --single-branch https://github.com/NVIDIA/srt-slurm.git "$SRT_REPO_DIR" || exit 1 | ||
| cd "$SRT_REPO_DIR" || exit 1 | ||
|
|
||
| mkdir -p recipes/vllm/deepseek-v4/agentic || exit 1 | ||
| cp -rT "$GITHUB_WORKSPACE/benchmarks/multi_node/srt-slurm-recipes/vllm/deepseek-v4/agentic" \ | ||
| recipes/vllm/deepseek-v4/agentic | ||
| recipes/vllm/deepseek-v4/agentic || exit 1 |
There was a problem hiding this comment.
🟡 WARNING: Two concerns with moving the agentic path from the pinned fork (cquil11/srt-slurm-nv@de59739b) to an unpinned NVIDIA/srt-slurm:main:
-
Existing recipes ride along. The existing GB200 agentic recipes (
disagg-gb200-2p1d-dep8-dep8-agentic.yaml,disagg-gb200-3p2d-tep8-tp8-agentic.yaml) were validated against the fork and still usedynamo: wheel: "1.3.0.dev20260618"plus thevllm/vllm-openai:v0.23.0container, while the new recipes usedynamo: hash:. The removed comment explicitly called outDynamoConfig.wheelandBenchmarkType.CUSTOMas fork features. If upstreammaindoesn't accept thewheel:field (or any other fork-only schema those recipes rely on), the existing 2p1d/3p2d sweeps will fail to parse atsrtctl apply. Please confirm both existing recipes were re-run on upstreammain, not just the two new ones. -
No pin = reproducibility drift. The previous code pinned an exact commit;
--branch mainfloats, so upstream can break or shift results for every GB200 agentic sweep without any change in this repo. Other launcher branches pin a branch/commit (aflowers/vllm-gb200-v0.20.0,sa-submission-q2-2026).
Fix: pin a known-good upstream commit:
| if [[ "$IS_AGENTIC" == "1" ]]; then | |
| # Agentic multi-node uses the same pinned cquil11/srt-slurm-nv commit as | |
| # launch_gb300-nv.sh — everything the agentic recipes need is there: | |
| # - BenchmarkType.CUSTOM + benchmark.command + benchmark.env | |
| # (the hook that hands off to benchmarks/multi_node/agentic_srt.sh) | |
| # - DynamoConfig.wheel (recipes pin the ai-dynamo wheel) | |
| # - srtctl apply --no-preflight (model path /mnt/numa1 is compute-node | |
| # local NVMe, invisible to the login-node runner) | |
| # - benchmark_stage srun_options propagation (container-remap-root | |
| # must reach the agentic_srt.sh srun) | |
| git clone https://github.com/cquil11/srt-slurm-nv.git "$SRT_REPO_DIR" | |
| cd "$SRT_REPO_DIR" | |
| git checkout de59739b172e507e15ebf145bfe305f606e82fbf | |
| mkdir -p recipes/vllm/deepseek-v4/agentic | |
| # Agentic recipes use NVIDIA/srt-slurm:main. The per-node DP launcher, | |
| # matching Dynamo health counts, multi-node TP VLLM_PORT handling, and | |
| # Mooncake master compatibility are all upstream. | |
| git clone --branch main --single-branch https://github.com/NVIDIA/srt-slurm.git "$SRT_REPO_DIR" || exit 1 | |
| cd "$SRT_REPO_DIR" || exit 1 | |
| mkdir -p recipes/vllm/deepseek-v4/agentic || exit 1 | |
| cp -rT "$GITHUB_WORKSPACE/benchmarks/multi_node/srt-slurm-recipes/vllm/deepseek-v4/agentic" \ | |
| recipes/vllm/deepseek-v4/agentic | |
| recipes/vllm/deepseek-v4/agentic || exit 1 | |
| if [[ "$IS_AGENTIC" == "1" ]]; then | |
| # Agentic recipes use NVIDIA/srt-slurm. The per-node DP launcher, | |
| # matching Dynamo health counts, multi-node TP VLLM_PORT handling, and | |
| # Mooncake master compatibility are all upstream. | |
| git clone https://github.com/NVIDIA/srt-slurm.git "$SRT_REPO_DIR" || exit 1 | |
| cd "$SRT_REPO_DIR" || exit 1 | |
| git checkout <validated-upstream-commit-sha> || exit 1 | |
| mkdir -p recipes/vllm/deepseek-v4/agentic || exit 1 | |
| cp -rT "$GITHUB_WORKSPACE/benchmarks/multi_node/srt-slurm-recipes/vllm/deepseek-v4/agentic" \ | |
| recipes/vllm/deepseek-v4/agentic || exit 1 |
ivanium
changed the title
There was a problem hiding this comment.
This PR adds new multi-node GB200 AgentX recipes and modifies shared launcher/routing logic, including a change to configs/nvidia-master.yaml (a CODEOWNER-protected file), so it's worth a human look before merging.
What was reviewed: the two new srt-slurm recipe YAMLs (aggregate TP8 and disaggregated 3P1D DEP8/DEP16), the launch_gb200-nv.sh model-path routing and srt-slurm source switch for agentic runs, the benchmark_lib.sh aiperf conv-aware-routing opt-out, and the new nvidia-master.yaml/perf-changelog.yaml entries.
Extended reasoning...
Overview
The PR adds two new DeepSeek-V4 GB200 Dynamo AgentX recipes (aggregate TP8 and disaggregated 3P1D DEP8/DEP16), registers them in configs/nvidia-master.yaml and perf-changelog.yaml, and modifies two shared scripts: benchmarks/benchmark_lib.sh (adds an opt-out env var for aiperf's dynamo conv-aware-routing flag) and runners/launch_gb200-nv.sh (switches the agentic srt-slurm clone from a pinned fork to NVIDIA/srt-slurm:main, and branches MODEL_PATH resolution for DSV4 FP4 based on IS_AGENTIC).
Security risks
No security-sensitive surface — this is internal benchmark/CI infrastructure (Slurm job launching, recipe YAML, shell env plumbing) with no auth, crypto, or externally-reachable inputs. The git clone URLs and container images are pinned to specific orgs/tags rather than user-controlled input.
Level of scrutiny
Higher than typical for this repo: the diff touches shared launcher logic (launch_gb200-nv.sh) and the aiperf routing default in benchmark_lib.sh, both of which affect existing non-agentic GB200 runs in addition to the new recipes. It also modifies configs/nvidia-master.yaml, which is CODEOWNER-protected (@ankur-singh @kedarpotdar-nv @InferenceX/core). Combined with the size and multi-file nature of the change (new multi-node recipes with many tuned env vars, a repo-source switch for the agentic srt-slurm path, and conditional model-path logic), this warrants a human pass rather than auto-approval.
Other factors
The bug-hunting system found no bugs, and several candidate issues (leftover NCCL symm-mem env var, an odd tokenizer string, --no-preflight usage, combining kv-cache-memory with gpu-memory-utilization) were raised and ruled out as not real bugs. The PR description/comments show no CODEOWNER sign-off yet per the standard recipe-PR checklist, which is itself a signal that human review is expected before merge.
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=29960434846 |
1 participant
No description provided.