Test Kimi Agentx - #2637
Conversation
Port the validated aggregate and disaggregated DCP8 Mooncake configurations to InferenceX using the rebuilt vLLM image. Co-authored-by: OpenAI Codex <codex@openai.com> Signed-off-by: Wei Zhao <51183510+wzhao18@users.noreply.github.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 关于重新运行失败任务的文档 |
Co-authored-by: OpenAI Codex <codex@openai.com> Signed-off-by: Wei Zhao <51183510+wzhao18@users.noreply.github.com>
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| # Kimi-K3 GB300 p90-ITL Pareto points. Each DCP8 worker spans two 4-GPU | ||
| # trays: 1P1D uses 16 GPUs and 1P2D uses 24 GPUs. | ||
| kimik3-fp4-gb300-dynamo-vllm-agentic-dspark-mooncake-dcp8-disagg: | ||
| image: vllm/vllm-openai:nightly-dev-arm64-cu13.0.1-75c2eef | ||
| model: moonshotai/Kimi-K3 | ||
| model-prefix: kimik3 | ||
| runner: cluster:gb300-nv | ||
| precision: fp4 | ||
| framework: dynamo-vllm | ||
| router: { name: dynamo-router, version: "ba83080ecd31c1ce918559e576d3c5bc9e092ff1" } | ||
| kv-p2p-transfer: nixl | ||
| multinode: true | ||
| disagg: true | ||
| scenarios: | ||
| agentic-coding: | ||
| # 0.1664 resolves to 150 GB of host DRAM per 4-GPU server node on | ||
| # cluster:gb300-nv, matching the recipe's Mooncake segment size. | ||
| - dram-utilization: 0.1664 | ||
| search-space: | ||
| - spec-decoding: mtp | ||
| kv-offloading: dram | ||
| kv-offload-backend: { name: mooncake, version: "0.3.12.post1" } | ||
| conc-list: [64] |
There was a problem hiding this comment.
🔴 This PR adds 5 new Kimi-K3 GB300 agentic recipes and 3 new master-config keys in configs/nvidia-master.yaml but does not add a corresponding perf-changelog.yaml entry, which AGENTS.md marks as a non-negotiable requirement for every recipe addition. Please append a changelog entry (config-keys/scenario-type/description/PR-link) following the pattern used by the immediately preceding recipe PRs (#2612, #2619, #2620, #2621).
Extended reasoning...
AGENTS.md, under 'Non-negotiable benchmark invariants', states verbatim: 'Every change that can affect benchmark performance and every recipe addition or modification requires a new perf-changelog.yaml entry. The file is append-only and byte-sensitive.' This PR adds 5 new Kimi-K3 GB300 agentic recipe YAMLs (agg-gb300-dcp8-dspark4-maxseq2-mooncake-agentic.yaml, agg-gb300-dcp8-nospec-mooncake-agentic.yaml, and three disagg-gb300-*-mooncake-agentic.yaml variants) plus 3 brand-new master-config keys in configs/nvidia-master.yaml (kimik3-fp4-gb300-dynamo-vllm-agentic-dspark-mooncake-dcp8-disagg, ...-dcp8-agg, and ...-mooncake-dcp8-agg, at lines 8578-8600 and following). This is squarely 'recipe addition' territory, yet perf-changelog.yaml is not among the 7 files touched by the diff.\n\nThe code path that triggers this is simple: a reviewer or the ingest tooling diffs the PR's file list against the AGENTS.md checklist and expects a changelog entry alongside any new master-config scenario key. Nothing in the PR's CI (the recipe-reminder bot comment, the CODEOWNER checklist) auto-generates or enforces this entry — it's a manually-authored file that recipe authors are expected to append to themselves, so nothing here prevents the omission from slipping through.\n\nThe established pattern in this exact repository is consistent: the tail of perf-changelog.yaml shows that every recent sibling recipe PR — #2612, #2619, #2620, and #2621 (visible in this repo's own recent commit history, e.g. 'Add Kimi-K3 GB300 DCP Pareto configs', '[AgentX] Mirror GB300 MTP tuning...', 'perf(agentx): retune B200 vLLM MTP aggregate...', '[NV][AgentX] Add GB200 GLM-5.2 FP4 MTP sweep...') — appended a corresponding config-keys/scenario-type/description/PR-link block. This PR breaks that pattern.\n\nProof by walkthrough:\n1. Open AGENTS.md and locate the 'Non-negotiable benchmark invariants' section — it unconditionally requires a changelog entry for 'every recipe addition or modification'.\n2. List this PR's changed files: the 5 new recipe YAMLs, configs/nvidia-master.yaml, and runners/launch_gb300-nv.sh — 7 files total, none of which is perf-changelog.yaml.\n3. Diff configs/nvidia-master.yaml and confirm 3 new top-level scenario keys were added (kimik3-fp4-gb300-dynamo-vllm-agentic-dspark-mooncake-dcp8-disagg, -dcp8-agg, -mooncake-dcp8-agg), each of which is a 'recipe addition' per the AGENTS.md definition.\n4. Check the tail of perf-changelog.yaml on main and observe the last four recipe-adding PRs (#2612, #2619, #2620, #2621) each appended an entry for their new keys.\n5. Confirm no such entry exists for the 3 new Kimi-K3 keys introduced here — the pattern is broken.\n\nFix: append a perf-changelog.yaml entry documenting the 3 new config keys (config-keys, scenario-type, description, and this PR's link), following the exact block format used by the #2612/#2619/#2620/#2621 entries, and take care to preserve the file's append-only/byte-sensitive formatting conventions when editing.
| no-enable-flashinfer-autotune: true | ||
| max-cudagraph-capture-size: 512 | ||
| stream-interval: 10 | ||
| max_num_batched_tokens: 16384 |
There was a problem hiding this comment.
🟡 In the prefill vllm_config of all three new disagg Kimi-K3 recipes, the key is snake_case max_num_batched_tokens (line 151) while every other key in these files — and the sibling agg recipes added in this same PR — uses kebab-case max-num-batched-tokens, per AGENTS.md's kebab-case requirement. Same typo at disagg-gb300-1p1d-dcp8-dcp8-dspark7-mooncake-agentic.yaml:151 and disagg-gb300-1p2d-dcp8-dcp8-dspark4-mooncake-agentic.yaml:151; please rename to kebab-case for consistency (and consider setting the equivalent key explicitly on decode too, as the sibling deepseek-v4 disagg recipes do).
Extended reasoning...
What's wrong: In the prefill vllm_config block of the three new disagg Kimi-K3 recipes, the batched-tokens key is spelled max_num_batched_tokens (underscores) instead of max-num-batched-tokens (kebab-case):
prefill:
...
max_num_batched_tokens: 16384 # line 151 — should be max-num-batched-tokensThis is the only snake_case key in these files — every other key in the same vllm_config blocks (served-model-name, enable-prefix-caching, tensor-parallel-size, etc.) uses kebab-case, and AGENTS.md explicitly states "YAML uses kebab-case fields."
Where it manifests: The identical typo appears in all three new disagg files added by this PR — disagg-gb300-1p1d-dcp8-dcp8-dspark4-mooncake-agentic.yaml:151, disagg-gb300-1p1d-dcp8-dcp8-dspark7-mooncake-agentic.yaml:151, and disagg-gb300-1p2d-dcp8-dcp8-dspark4-mooncake-agentic.yaml:151.
Proof it's an accidental typo, not a deliberate choice: The two sibling aggregate recipes added in this very same PR — agg-gb300-dcp8-dspark4-maxseq2-mooncake-agentic.yaml:98 and agg-gb300-dcp8-nospec-mooncake-agentic.yaml:95 — both spell it correctly as kebab-case max-num-batched-tokens. So the same author got it right twice in this PR and only slipped in the three disagg-prefill blocks. A repo-wide grep confirms snake_case appears nowhere else across the 30+ other vLLM recipes in srt-slurm-recipes.
Why existing tooling doesn't catch it: There's no schema validation on these YAML vllm_config maps — they're passed through more or less verbatim to become CLI flags for the vLLM engine, so a misspelled key doesn't fail fast; it just silently produces a different (or missing) flag.
Runtime impact — deliberately not overstated: All three verifiers agreed this is real and worth flagging, but pushed back on the initial finder's claim that this would "silently fall back to vLLM's default instead of 16384." vLLM's FlexibleArgumentParser normalizes underscores to dashes in CLI flag names, so if srt-slurm renders this key straight into a --max_num_batched_tokens=16384 flag, vLLM would very likely still parse it as --max-num-batched-tokens and honor the value. Without visibility into the srt-slurm YAML-to-CLI renderer (cloned at runtime from ), a hard failure or silent default fallback can't be proven. What is proven is the convention violation and the internal inconsistency within the same author's PR.
Fix: Rename max_num_batched_tokens to max-num-batched-tokens at all three call sites for consistency and to guarantee the value is honored regardless of how the loader keys off it. Separately, note that none of the three files set this key at all on the decode role, unlike the existing deepseek-v4 disagg recipes (e.g. disagg-gb300-1p1d-dep4-dep8-c256-mtp-agentic.yaml) which set it explicitly on both prefill and decode — worth a look, though it's plausible decode intentionally relies on vLLM's default here.
Given the fix is a straightforward rename with no demonstrated runtime break, this is a consistency nit rather than a blocking issue.
| prefill: | ||
| # MultiConnector: Nixl (P->D handshake transfer) + Mooncake (external DRAM KV). | ||
| kv-transfer-config: '{"kv_connector":"MultiConnector","kv_role":"kv_both","kv_load_failure_policy":"recompute","kv_connector_extra_config":{"connectors":[{"kv_connector":"NixlConnector","kv_role":"kv_both","kv_load_failure_policy":"fail","kv_buffer_device":"cuda","kv_connector_extra_config":{"enforce_handshake_compat":false,"enable_cross_layers_blocks":false}},{"kv_connector":"MooncakeStoreConnector","kv_role":"kv_both","kv_load_failure_policy":"recompute","kv_connector_extra_config":{"load_async":true,"lookup_async":true,"enable_cross_layers_blocks":false,"enable_offload":false}}]}}' | ||
| served-model-name: "moonshotai/Kimi-K3" | ||
| enable-prefix-caching: true | ||
| prefix-match-unit: 128 | ||
| load-format: fastsafetensors | ||
| kv-cache-dtype: fp8 | ||
| tensor-parallel-size: 8 | ||
| # The point of this recipe: DCP reuses the 8 TP ranks and shards the MLA | ||
| # KV cache across them instead of replicating it, giving the prefill role | ||
| # ~7.5x the KV pool it has under plain TP8. | ||
| decode-context-parallel-size: 8 | ||
| enable-cumem-allocator: true | ||
| trust-remote-code: true | ||
| no-enable-flashinfer-autotune: true | ||
| max-cudagraph-capture-size: 512 | ||
| stream-interval: 10 | ||
| max_num_batched_tokens: 16384 | ||
| language-model-only: true | ||
| attention-backend: "TOKENSPEED_MLA" | ||
| attention-config: '{"mla_prefill_backend": "TRTLLM_RAGGED", "use_prefill_query_quantization": true}' | ||
| speculative-config: '{"model":"Inferact/Kimi-K3-DSpark","attention_backend":"TOKENSPEED_MLA","method":"dspark","num_speculative_tokens":4,"draft_sample_method":"probabilistic","rejection_sample_method":"block"}' |
There was a problem hiding this comment.
🟡 In all three new Kimi-K3 disagg recipes, vllm_config.prefill and vllm_config.decode are written out in full and are byte-identical except for max-cudagraph-capture-size, one decode-only compilation-config key, and max_num_batched_tokens vs max-num-seqs. The ~600-char kv-transfer-config JSON and a dozen other keys get hand-duplicated across 6 blocks (2 per file x 3 files); the repo already has a YAML anchor+merge idiom for this exact case (e.g. disagg-gb200-1p1d-dep8-dep8-mtp-agentic.yaml:71-96, and this PR already applies it one level up via &kimi_env).
Extended reasoning...
What the bug is: In each of the three new disagg recipes (disagg-gb300-1p1d-dcp8-dcp8-dspark4-mooncake-agentic.yaml:133-177, disagg-gb300-1p1d-dcp8-dcp8-dspark7-mooncake-agentic.yaml, and disagg-gb300-1p2d-dcp8-dcp8-dspark4-mooncake-agentic.yaml, same line range), vllm_config.prefill and vllm_config.decode are two full, independently-written maps. Diffing them shows they are byte-identical apart from three fields: max-cudagraph-capture-size (512 vs 1024), an extra decode-only compilation-config key, and max_num_batched_tokens (prefill) vs max-num-seqs (decode). Every other key — including the ~600-character kv-transfer-config MultiConnector JSON string, served-model-name, enable-prefix-caching, prefix-match-unit, load-format, kv-cache-dtype, tensor-parallel-size, decode-context-parallel-size, enable-cumem-allocator, trust-remote-code, no-enable-flashinfer-autotune, stream-interval, language-model-only, attention-backend, attention-config, and speculative-config — is duplicated verbatim between the two blocks.
Why this isn't already prevented: YAML has no native "extend a mapping" operator outside of the merge key (<<) combined with anchors (&name), and nothing in CI or a schema check enforces DRY-ness across sibling recipe blocks — a recipe is just data, so duplicating it is syntactically valid and produces a working config. The PR author clearly knows the anchor+merge idiom exists, because they use it one level up in the very same files: prefill_environment: &kimi_env / decode_environment: <<: *kimi_env (visible right above vllm_config: in each file). They simply didn't extend that pattern down into vllm_config. The wider repo also already has this exact precedent for prefill/decode config duplication: benchmarks/multi_node/srt-slurm-recipes/vllm/deepseek-v4/agentic/disagg-gb200-1p1d-dep8-dep8-mtp-agentic.yaml:71-96 defines prefill: &dep8_config and decode: <<: *dep8_config, overriding only the handful of differing keys.
Impact: This is a pure maintainability/quality issue, not a functional bug — the recipes work as written. The cost is that any future change to the shared settings (most acutely the giant kv-transfer-config JSON, which is exactly the kind of value that's easy to fumble when hand-copying) must be applied consistently across 6 near-duplicate blocks (2 per file × 3 files) instead of 3 canonical ones. A missed edit in one of the 6 copies would silently produce a prefill/decode mismatch (e.g. a stale kv_role or connector setting) that only surfaces at runtime as a KV-transfer failure, which is exactly the class of hard-to-diagnose bug the anchor+merge convention is meant to prevent.
How to fix — step-by-step proof of the refactor:
- In
disagg-gb300-1p1d-dcp8-dcp8-dspark4-mooncake-agentic.yaml, changeprefill:toprefill: &k3_dcp8_configand keep all of its current keys (the "prefill" values become the shared baseline, matching every non-differing key seen in the decode block). - Replace the
decode:block's body with<<: *k3_dcp8_configplus only the fields that actually differ:max-cudagraph-capture-size: 1024,compilation-config: '{"cudagraph_mode":"FULL_DECODE_ONLY","mode":0}', andmax-num-seqs: 256(droppingmax_num_batched_tokenssince decode doesn't use it). - Repeat steps 1-2 for the other two disagg files. After the change, each file has ~20 fewer duplicated lines and the ~600-char
kv-transfer-configstring exists once per file instead of twice — verified by re-diffing the two blocks post-refactor and confirming they resolve to the same merged values as today (YAML merge-key semantics guarantee<<: *anchorplus explicit overrides is equivalent to the fully-duplicated map, so this is a behavior-preserving refactor).
All three verifiers independently confirmed the duplication and the precedent with no refutations, and the finding is scoped as a [quality] cleanup per the source bug names, so it's filed as a non-blocking nit rather than something that should hold up the merge.
| # Built vLLM image containing the Kimi-K3 DCP, DSpark, and PMU fixes. | ||
| # Measured Pareto family: OCI jobs 471040 (c1) and 471041 (c2), using DSpark | ||
| # K=7 with synthetic acceptance length 3.84. | ||
| name: "kimi-k3-vllm-disagg-gb300-1p1d-dcp8-dcp8-dspark7-mooncake-agentic" | ||
|
|
There was a problem hiding this comment.
🟡 The new disagg-gb300-1p1d-dcp8-dcp8-dspark7-mooncake-agentic.yaml recipe (DSpark K=7) is added but no CONFIG_FILE in configs/nvidia-master.yaml ever references it, so no sweep will launch it and its measurement work is invisible in the benchmark matrix. Either wire it into a kimik3 search-space or drop the file.
Extended reasoning...
What the bug is
This PR adds five new Kimi-K3 GB300 agentic recipe files and wires four of them into configs/nvidia-master.yaml via CONFIG_FILE entries under three new search-space scenarios (kimik3-fp4-gb300-dynamo-vllm-agentic-dspark-mooncake-dcp8-disagg, ...-dcp8-agg, and ...-mooncake-dcp8-agg). The fifth file, disagg-gb300-1p1d-dcp8-dcp8-dspark7-mooncake-agentic.yaml, is never referenced anywhere in the master config.
Proof
A repo-wide search confirms this:
$ grep -rn "dspark7" --include="*.yaml" .
./benchmarks/.../disagg-gb300-1p1d-dcp8-dcp8-dspark7-mooncake-agentic.yaml:4:name: "kimi-k3-vllm-disagg-gb300-1p1d-dcp8-dcp8-dspark7-mooncake-agentic"
$ grep -n "dspark7" configs/nvidia-master.yaml
(no output)
The only hit is the recipe declaring its own name field -- nothing in nvidia-master.yaml selects it via CONFIG_FILE. Compare this to the disagg scenario that was wired in, which lists exactly two CONFIG_FILE entries (both ...dspark4...) under its search-space, plus the two agg scenarios that reference agg-...-dspark4-maxseq2... and agg-...-nospec.... All four other new recipe files have a home; this one does not.
Why nothing else catches it
runners/launch_gb300-nv.sh's agentic branch does "cp -rT .../kimi-k3/agentic recipes/vllm/kimi-k3/agentic", copying the entire directory into the srt-slurm checkout regardless of which files are actually used. That copy step succeeds and produces no error, so CI has no signal that a file went unreferenced -- a recipe is only ever launched when some search-space entry names it in CONFIG_FILE, and nothing in the diff does that for the K=7 variant.
Impact
The DSpark K=7 recipe (measured against OCI jobs 471040/471041, synthetic acceptance length 3.84) represents real prior benchmarking work that is now dead code: it will sit in the repo permanently without ever being exercised by a sweep, and nobody scanning nvidia-master.yaml for kimik3 coverage will know K=7 was ever measured or intended to be tracked.
Suggested fix
Either add a search-space entry (or extend the existing disagg scenario) with CONFIG_FILE: recipes/vllm/kimi-k3/agentic/disagg-gb300-1p1d-dcp8-dcp8-dspark7-mooncake-agentic.yaml so it is actually launched, or remove the file if K=7 was superseded by the K=4 config that did get wired in.
Severity
This is pure benchmark-matrix housekeeping -- nothing crashes, no incorrect data is produced, and the four wired-in recipes work fine on their own. It does not block merging, hence nit.
Co-authored-by: OpenAI Codex <codex@openai.com> Signed-off-by: Wei Zhao <51183510+wzhao18@users.noreply.github.com>
Co-authored-by: OpenAI Codex <codex@openai.com> Signed-off-by: Wei Zhao <51183510+wzhao18@users.noreply.github.com>
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=32087393709 |
1 participant
No description provided.