[feat][nv] Add AgentX Kimi-K3 b200 pp2 sweep - #2672
Conversation
8285674 to
59e8448
Compare
xinli-sw
changed the title
Nine points on TP8 x PP2 with DCP8 and a Mooncake external tier, concurrency 1 through 96. Concurrency 1 to 32 drafts with DSpark level 7, 48 and above runs unspeculated.
59e8448 to
7d9e746
Compare
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Additional findings (outside current diff — PR may have been updated during review):
-
🟡
perf-changelog.yaml:6164-6171— The new perf-changelog.yaml entry forkimik3-fp4-b200-dynamo-vllm-agentic-dsparksetspr-linkto pull/2618, but this change is being introduced by this PR (#2672). Please update the link to point to #2672 (or the final merge PR number) to keep the changelog traceable.Extended reasoning...
This PR adds a new entry to
perf-changelog.yaml(lines 6164-6171) documenting thekimik3-fp4-b200-dynamo-vllm-agentic-dsparkconfig change: "Add new B200 multi-node configs with PP2, DCP8." That description exactly matches what this PR does — it renames/rewrites the aggregated Kimi-K3 B200 agentic recipes from a TP8/DP2 DEP layout to a TP8/PP2/DCP8 Mooncake-backed layout, and adds several new concurrency-point YAML files (c1, c2, c4, c8, c16, c32, c48, c72, c96). Yet the entry'spr-linkfield points tohttps://github.com/SemiAnalysisAI/InferenceX/pull/2618, not this PR (#2672, per pr-metadata).Established convention: every other entry in
perf-changelog.yamllinks to the PR that actually introduced it. The three entries immediately preceding this new one link to #2656, #2658, and #2590 respectively — each of those corresponds to the PR that made that specific change (visible in the repo's recent commit history: "Add Kimi K3 FP4 B200 PP2 agentic sweep", "perf(dsv4): update... (#2656)", etc.). Agrepfor2618across the repository turns up no other reference to that PR number anywhere in the diff or codebase, which rules out it being some intentional cross-reference to a related prior PR — it reads as a stale/copy-pasted number left over from an earlier draft of this changelog entry.Why this isn't caught by existing tooling:
perf-changelog.yamlis a free-form changelog with no schema validation tyingpr-linkto the PR number that actually authored the diff; nothing in CI cross-checks this field against the current PR, so a copy-paste error here would silently pass all checks.Step-by-step proof:
- This PR's metadata reports PR number 2672.
- The new changelog entry (lines 6164-6171) is added entirely by this PR's diff — it doesn't exist on
mainbefore this change. - Its
descriptionfield ("Add new B200 multi-node configs with PP2, DCP8") is a precise 1:1 match for thenvidia-master.yamlchanges in this same diff (pipeline-parallel-size: 2, decode-context-parallel-size: 8, new Mooncake-backed recipe files). - Its
pr-linknonetheless points at pull/2618 — a different, unrelated, lower-numbered PR. - Every neighboring entry's
pr-linkmatches the PR that introduced that entry's change, confirming the file-wide convention this entry breaks.
Impact: Purely a documentation/traceability issue — nothing functional breaks, no benchmark behavior is affected. But
pr-linkexists specifically so future readers can trace a perf change back to its origin PR; as written it would send them to an unrelated PR (#2618) instead of this one. Given the PR is currently marked[wip], the number may simply not have been updated yet to the final merge PR — but as written today it is inconsistent with the file's own convention.Fix: Update
pr-linkon this entry tohttps://github.com/SemiAnalysisAI/InferenceX/pull/2672(or whatever the actual merge PR number ends up being).
| if [[ "$IS_AGENTIC" == "1" && $MODEL_PREFIX == "kimik3" ]]; then | ||
| # Pin the tested renderer so branch movement cannot change generated rank | ||
| # commands between sweep points. | ||
| git clone --branch main --single-branch https://github.com/NVIDIA/srt-slurm.git "$SRT_REPO_DIR" || exit 1 | ||
| cd "$SRT_REPO_DIR" || exit 1 | ||
| git checkout 217f9438 || exit 1 | ||
| mkdir -p recipes/vllm/kimi-k3/agentic || exit 1 | ||
| cp -rT "$GITHUB_WORKSPACE/benchmarks/multi_node/srt-slurm-recipes/vllm/kimi-k3/agentic" \ | ||
| recipes/vllm/kimi-k3/agentic || exit 1 | ||
| elif [[ $MODEL_PREFIX == "dsv4" && $FRAMEWORK == "dynamo-sglang" ]]; then |
There was a problem hiding this comment.
🔴 The new kimik3 IS_AGENTIC block in launch_b200-nscale-slurm.sh (lines 67-76) clones vanilla NVIDIA/srt-slurm main @ 217f9438, but the new agg-b200-tp8pp2-mooncake-c*-agentic.yaml recipes it copies in are direct multi-node vLLM (frontend/backend type vllm, pipeline-parallel-size 2 across 2 nodes) — the same shape as agg-b200-tp8pp2-agentic.yaml, which documents that this requires the functionstackx/srt-slurm-nv fork's klaud/direct-vllm-multinode branch, since srt-slurm PR #278 only validates single-node on mainline. The two sibling launchers (launch_b200-dgxc.sh, launch_h200-dgxc-slurm.sh) both clone that fork for this exact IS_AGENTIC+kimik3 case; cloning mainline here instead means srtctl apply will likely fail to render/launch these 2-node PP2 recipes on b200-nscale.
Extended reasoning...
The bug: runners/launch_b200-nscale-slurm.sh adds a new branch for IS_AGENTIC == 1 && MODEL_PREFIX == kimik3 (lines 67-76) that clones https://github.com/NVIDIA/srt-slurm.git on main and checks out 217f9438. It then copies the new agg-b200-tp8pp2-mooncake-c1..c96-agentic.yaml recipes into that checkout. All of these new recipes are direct multi-node vLLM profiles: frontend.type: vllm, backend.type: vllm, dynamo.install: false, pipeline-parallel-size: 2, and resources.agg_nodes: 2 (16 GPUs total, i.e. a 2-node headless vLLM serve launch, not a Dynamo frontend/backend split).\n\nWhy mainline can't render this: the pre-existing sibling recipe agg-b200-tp8pp2-agentic.yaml (same TP8xPP2 2-node vLLM shape) documents in its header that direct vLLM multi-node serving requires the functionstackx/srt-slurm-nv fork's klaud/direct-vllm-multinode branch, because upstream srt-slurm PR #278 'validates single-node only.' perf-changelog.yaml corroborates this: the launcher clones that fork branch specifically because it 'carries PR #278 plus the multi-node extension (vLLM-native --master-addr/--nnodes/--node-rank serve, headless non-leader ranks) for the 2-node TP8xPP2 topology.' Both other launchers that already handle this exact case — launch_b200-dgxc.sh (lines 168-177, with the comment 'Kimi K3 aggregate profiles use the srt-slurm fork that supports direct multi-node vLLM') and launch_h200-dgxc-slurm.sh (lines 82-85) — clone functionstackx/srt-slurm-nv at klaud/direct-vllm-multinode (commit df5baa93), never vanilla NVIDIA/srt-slurm.\n\nWhy existing code doesn't prevent it: the new branch in launch_b200-nscale-slurm.sh was evidently adapted from the pre-existing dsv4/kimik2.6 branches immediately below it in the same if/elif chain, which correctly clone mainline because those recipes use Dynamo's frontend/backend split (which mainline supports). The kimik3 case was special-cased for but the clone target was left as mainline instead of being pointed at the fork the other two clusters use — nothing in the script validates that the cloned renderer actually supports the recipe shape it's about to copy in.\n\nImpact: every point in the new c1..c96 sweep (the entire purpose of this PR, now retargeted from cluster:b200-dgxc to cluster:b200-nscale in configs/nvidia-master.yaml) would fail at srtctl apply, because mainline's renderer lacks the vLLM-native --master-addr/--nnodes/--node-rank headless-worker extension needed to generate correct per-rank launch commands for a 2-node PP2 job. This is a concrete launch failure, not a cosmetic one — the job would either fail to render, or render as a broken single-node command.\n\nStep-by-step proof:\n1. A sweep point sets MODEL_PREFIX=kimik3, PRECISION=fp4, IS_AGENTIC=1, CONFIG_FILE=recipes/vllm/kimi-k3/agentic/agg-b200-tp8pp2-mooncake-c8-agentic.yaml (from configs/nvidia-master.yaml's kimik3-fp4-b200-dynamo-vllm-agentic-dspark scenario, now pointed at runner: cluster:b200-nscale).\n2. launch_b200-nscale-slurm.sh hits the new elif [[ == "kimik3" && == "fp4" ]] for model-path resolution, then the new if [[ "" == "1" && == "kimik3" ]] clone branch: git clone --branch main ... NVIDIA/srt-slurm.git, git checkout 217f9438.\n3. The recipe (pipeline-parallel-size: 2, agg_nodes: 2, backend.type: vllm) is copied into recipes/vllm/kimi-k3/agentic/ inside that mainline checkout.\n4. srtctl apply -f runs against the mainline renderer at 217f9438, which — per PR #278's own scope note and the fork's existence — only knows how to render single-node vLLM launch commands, not the 2-node --master-addr/--nnodes/--node-rank headless-worker commands this PP2 recipe needs.\n5. The job either fails to render/submit, or submits an incorrect single-node command against a 2-node allocation — either way the sweep point fails, exactly as it would if launch_b200-dgxc.sh cloned mainline instead of the fork for the same recipe shape.\n\nFix: in the new IS_AGENTIC==1 && MODEL_PREFIX==kimik3 branch, clone functionstackx/srt-slurm-nv at klaud/direct-vllm-multinode (commit df5baa93) — matching launch_b200-dgxc.sh/launch_h200-dgxc-slurm.sh — instead of NVIDIA/srt-slurm main.
| echo "Cloning srt-slurm repository..." | ||
| SRT_REPO_DIR="srt-slurm" | ||
| rm -rf "$SRT_REPO_DIR" | ||
| if [[ $MODEL_PREFIX == "dsv4" && $FRAMEWORK == "dynamo-sglang" ]]; then | ||
| if [[ "$IS_AGENTIC" == "1" && $MODEL_PREFIX == "kimik3" ]]; then | ||
| # Pin the tested renderer so branch movement cannot change generated rank | ||
| # commands between sweep points. | ||
| git clone --branch main --single-branch https://github.com/NVIDIA/srt-slurm.git "$SRT_REPO_DIR" || exit 1 | ||
| cd "$SRT_REPO_DIR" || exit 1 | ||
| git checkout 217f9438 || exit 1 | ||
| mkdir -p recipes/vllm/kimi-k3/agentic || exit 1 | ||
| cp -rT "$GITHUB_WORKSPACE/benchmarks/multi_node/srt-slurm-recipes/vllm/kimi-k3/agentic" \ | ||
| recipes/vllm/kimi-k3/agentic || exit 1 | ||
| elif [[ $MODEL_PREFIX == "dsv4" && $FRAMEWORK == "dynamo-sglang" ]]; then | ||
| git clone --branch main --single-branch https://github.com/NVIDIA/srt-slurm.git "$SRT_REPO_DIR" || exit 1 | ||
| cd "$SRT_REPO_DIR" || exit 1 | ||
| # Pin the srt-slurm revision used by these checked-in recipes. |
There was a problem hiding this comment.
🟡 The kimik3 clone branch in launch_b200-nscale-slurm.sh is gated on IS_AGENTIC==1 && MODEL_PREFIX==kimik3 (line 67); a hypothetical non-agentic kimik3 run would fall through to the final else, which checks out the kimi-k2.6 srt-slurm pin and copies recipes/vllm/kimi-k2.6 instead of kimi-k3, rather than hitting the explicit 'Unsupported model prefix/precision' guard used for other invalid combos. No shipped config can trigger this today (the only cluster:b200-nscale kimik3 entry is scenario-type agentic-coding, and IS_AGENTIC is derived deterministically from scenario-type in the workflow templates), and if it were ever triggered the run would fail loudly at srtctl apply (missing kimi-k3 CONFIG_FILE in the k2.6-only checkout) rather than silently misconfiguring the job — so this is a defensive-coding nit, not a blocking issue.
Extended reasoning...
What the code does today: runners/launch_b200-nscale-slurm.sh:64-79 clones srt-slurm and picks which recipe directory to copy in based on model/framework/agentic flags. The PR adds a new first branch, [[ "$IS_AGENTIC" == "1" && $MODEL_PREFIX == "kimik3" ]], which checks out 217f9438 and copies benchmarks/multi_node/srt-slurm-recipes/vllm/kimi-k3/agentic into recipes/vllm/kimi-k3/agentic. The remaining branches only match dsv4, so any kimik3 job with IS_AGENTIC != 1 would fall through to the final else, which checks out the kimi-k2.6 pin (c180328b...) and copies recipes/vllm/kimi-k2.6 — the wrong recipe tree for a kimik3 job. Unlike the model-prefix/precision guard a few lines earlier (else ... echo "Unsupported model prefix/precision" ... exit 1), there's no equivalent fail-fast here for an unsupported agentic combination.
Why I'm filing this as a nit rather than agreeing with the full refutation: one verifier refuted the finding outright, arguing it's not triggerable and not silent. I re-checked both claims against the current repo state and they hold up: .github/workflows/benchmark-multinode-tmpl.yml:253 and benchmark-tmpl.yml:185 both set IS_AGENTIC: ${{ inputs.scenario-type == 'agentic-coding' && '1' || '0' }}, so IS_AGENTIC is not an independently-settable input — it's derived purely from scenario-type. And the only configs/nvidia-master.yaml entry that targets cluster:b200-nscale for kimik3 (kimik3-fp4-b200-dynamo-vllm-agentic-dspark, the one this PR modifies) declares only scenario-type: agentic-coding under scenarios:. The sibling kimik3-fp4-b200-dynamo-vllm-agentic entry runs on cluster:b200-dgxc, a different launcher entirely. So today there is genuinely no config path that reaches this launcher with MODEL_PREFIX=kimik3 and IS_AGENTIC != 1.
Why I'm still filing it (not abstaining): the refutation is correct about non-triggerability today, but that's exactly why this is a nit-level "add a guard for robustness" comment rather than a "this breaks a real run" comment — it isn't a reason to drop it entirely. The PR is explicitly introducing new dispatch logic in a shared, non-model-specific script; a future kimik3 config that omits agentic-coding (or a copy/paste of an existing kimik3 entry with scenario-type changed) would silently pick up the k2.6 recipe directory instead of erroring immediately the way the prefix/precision guard does. That's a latent gap in code this PR is actively touching, and it's cheap to close.
Step-by-step proof of the gap (hypothetical, not currently reachable): (1) a future nvidia-master.yaml entry sets model-prefix: kimik3, runner: cluster:b200-nscale, precision: fp4, framework: dynamo-vllm, but under a non-agentic-coding scenario-type (e.g. a plain throughput scenario). (2) The multinode workflow template computes IS_AGENTIC=0 for that run per the deterministic mapping above. (3) launch_b200-nscale-slurm.sh model-path block at lines 46-48 still matches (kimik3/fp4), so MODEL_PATH/SRT_SLURM_MODEL_PREFIX get set correctly for kimik3. (4) At the clone dispatch, the new IS_AGENTIC==1 && kimik3 branch is skipped (condition false), the two dsv4 elifs are skipped, and execution reaches the final else, checking out kimi-k2.6's pin and copying recipes/vllm/kimi-k2.6. (5) Downstream, CONFIG_FILE (from the kimik3 scenario's additional-settings) still points at a recipes/vllm/kimi-k3/... path, which doesn't exist in the k2.6-only checkout, so srtctl apply -f fails — a confusing late failure rather than the clear early "Unsupported model prefix/precision" message that exists for other invalid combinations.
Suggested fix: either drop the IS_AGENTIC==1 condition from the kimik3 clone branch (clone the kimik3 recipes whenever MODEL_PREFIX==kimik3, agentic or not) or add an explicit non-agentic kimik3 elif/fail-fast alongside it, mirroring the existing "Unsupported model prefix/precision" guard's exit-1 pattern so an invalid future config fails immediately and clearly instead of falling into the k2.6 branch.
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=32225002756 |
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=32225002756 |
1 similar comment
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=32225002756 |
|
/stage-results 32225002756 |
|
@functionstackx staged run 32225002756: https://inferencemax-app-git-staging-semianalysisai.vercel.app/inference?i_dates=2026-08-19~r32225002756 This run remains available across future |
|
/reuse-sweep-run |
2 participants
No description provided.