Conversation
|
Thanks for the contribution!
中文感谢你的贡献!
|
There was a problem hiding this comment.
Looks good — this is a mechanical addition of a new GB200 Dynamo+SGLang AgentX benchmark family (6 recipe YAMLs + matching master-config entries + changelog), plus a small, additive conditional in launch_gb200-nv.sh that preserves existing behavior for non-agentic/dynamo-vllm dsv4 paths.
What was reviewed: the new IS_AGENTIC/FRAMEWORK branching for MODEL_PATH and uses_watchtower_shared_fs() (additive, existing dsv4/dynamo-vllm behavior unchanged, no defaulted vars); the recipe/master-config/changelog linkage for all 6 new files; decode max-running-requests vs. cuda-graph-max-bs-decode sizing in the disagg c400 recipe (left at 256 despite higher concurrency — flagged as a candidate but ruled out); and the widened shared-fs routing for AgentX dsv4 jobs (consistent with the existing dynamo-vllm precedent, ruled out as a new risk category).
Extended reasoning...
Overview
The PR adds a new GB200 Dynamo+SGLang AgentX recipe family for DSV4-Pro: six new recipe YAMLs (two aggregate TP8, four disaggregated 1P1D/2P1D DEP8/DEP16), two matching nvidia-master.yaml config-key entries, two perf-changelog.yaml append-only entries, and a small change to runners/launch_gb200-nv.sh that (a) points MODEL_PATH at a node-local NVMe checkpoint specifically for MODEL_PREFIX=dsv4, PRECISION=fp4, FRAMEWORK=dynamo-sglang, IS_AGENTIC=1 jobs, and (b) extends uses_watchtower_shared_fs() to route those same jobs onto the compute-visible shared filesystem, matching the existing dynamo-vllm dsv4 precedent.
Security risks
None identified. This is internal benchmark infrastructure config (image tags, model paths, cluster routing flags) with no user input, auth, or crypto surface. Paths and image references are hardcoded, not interpolated from untrusted sources.
Level of scrutiny
Moderate: most of the diff (recipe YAMLs, master-config, changelog) is mechanical and follows established patterns already present elsewhere in the repo (e.g., the existing gb300 dsv4 dynamo-sglang agentic entries). The bash script change is small (18 lines) and additive — it nests new conditionals inside existing elif branches rather than replacing them, so prior behavior for non-agentic and dynamo-vllm dsv4 jobs is preserved, consistent with the repo's no-silent-default-fallback convention. I traced the two candidate concerns already flagged (decode CUDA-graph capture size vs. max-running-requests in the c400 disagg recipe, and the widened shared-fs routing) and confirmed both are either intentional or consistent with pre-existing precedent, which is why they were ruled out.
Other factors
The perf-changelog entries carry placeholder pull/XXX pr-link values, but grepping the file shows this is an established, tolerated pattern already present elsewhere in perf-changelog.yaml (e.g., line 5598) prior to this PR, so it isn't a new deviation worth raising. No CHANGES_REQUESTED reviews or unresolved third-party objections are evident in the timeline, and the bug hunt exited via dry_streak with no findings. Given the additive, pattern-following nature of the change and the narrow, well-scoped bash edits, I have high confidence this does not need further human scrutiny beyond what's already been examined.
This review covers commit 893b871, which is no longer the latest commit on this pull request; later commits are not covered by it.
|
View unofficial run (performance): https://inferencex.semianalysis.com/inference?unofficialRun=36691720714 View unofficial run (accuracy): https://inferencex.semianalysis.com/evaluation?unofficialRun=36691720714 |
|
As a PR reviewer and CODEOWNER, I have reviewed this and have:
Additional detail section:
Signed: |
functionstackx
left a comment
There was a problem hiding this comment.
the TTFT doesn't seem tuned well and it seems like from 20s to 100s e2el there is no points which would be unfair to vllm submissions & other submissions to have no points on 20s to 100s
https://inferencex.semianalysis.com/inference?unofficialRun=35278105888
✅✅✅ Verdict: PASS ✅✅✅Passed and not applicable checks✅ Check 0 (CODEOWNER): PASS — ✅ Check 1 (Passing sweep on in-PR commit): PASS — on the pinned head ✅ Check 2 (Evals pass): PASS — ➖ Check 3 (Recipe linked/merged/complete): N/A — disaggregated/multi-node submission (all recipes under ✅ Check 4 (Reuse command): PASS — ✅ Check 5 (Latest checklist template): PASS — every item of the current ✅ Check 6 (Upstream image, engine-first): PASS — both new entries use ✅ Check 7 (No deprecated models/scenarios): PASS — ✅ Check 8 (No architecture hacks): PASS — no ✅ Check 9 (Spec-decode via chat template): PASS — DSPARK configs are driven by ✅ Check 10 (No engine patches): PASS — no ✅ Check 11 (Agentic spec-decode golden AL): PASS — recipes carry no hard-coded AL; ➖ Check 12 (Append-only): N/A — neither new Assessed commit: |
|
I will fix that |
|
thanks @nvpohanh ! appreipcate it! <3 |
09bcd32 to
5163e42
Compare
8642cc5 to
a0c4ade
Compare
8d04085 to
956b6bc
Compare
|
Sorry, over the weekend, there was 2 major refactors to clean up the technical debt accumalated over the past 11 months of moving at the speed of light. We don't see any major refactors in the forthseeable future besides cleaning up AMD multinode AgentX pile of bash. As much, due to the refactors, u would need to ask your agent to rebase from remote main@latest. Thank you in advance for ur understanding |
956b6bc to
1937a4a
Compare
dd0021f to
620c475
Compare
|
Heads-up: #3576 (merged) replaced the bash launchers with a Python launcher, so this PR will conflict when you merge |
9b39841 to
38ae07c
Compare
[by Codex]
Add GB200 Dynamo+SGLang AgentX configurations for
deepseek-ai/DeepSeek-V4-Pro-0813using its bundled DSpark head and HiCache.The sweep covers TP8 aggregate concurrency 1 and 4; 1P1D DEP8/DEP16 concurrency 64 and 128; 1P1D DEP16/DEP32 concurrency 256; and 2P1D DEP16/DEP32 concurrency 768, 1024, and 1280. The branch is rebased onto current
main, with its GB200 checkpoint and compute-visible workspace behavior migrated to the current Python launcher.AI model disclosure
中文
为
deepseek-ai/DeepSeek-V4-Pro-0813添加 GB200 Dynamo+SGLang AgentX 配置,使用其内置的 DSpark 草稿头和 HiCache。本次 sweep 覆盖 TP8 聚合模式并发 1 和 4;1P1D DEP8/DEP16 并发 64 和 128;1P1D DEP16/DEP32 并发 256;以及 2P1D DEP16/DEP32 并发 768、1024 和 1280。分支已 rebase 到当前
main,并将 GB200 检查点及计算节点可见工作区逻辑迁移到当前 Python launcher。AI 模型披露