[AgentX]: B200 Kimi K3 DSpark with corrected harness - #2475
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 关于重新运行失败任务的文档 |
1 similar comment
|
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 关于重新运行失败任务的文档 |
There was a problem hiding this comment.
LGTM — approving despite the two nit-level inline findings (missing bilingual translation, stale contradictory comment), neither of which affects the recipe's runtime behavior.
Extended reasoning...
Overview
This PR adds a new AgentX benchmark recipe: a DSpark speculative-decoding variant of the existing Kimi-K3 MXFP4 B200 aggregated TP8×PP2 topology. It touches three files: a new recipe YAML (mostly copied from the existing sibling agg-b200-tp8pp2-agentic.yaml with DSpark-specific speculative-config, environment vars, and comments), a new entry in configs/nvidia-master.yaml wiring the recipe into the benchmark matrix, and an appended perf-changelog.yaml entry. No application/inference-engine code is touched — this is purely benchmark configuration.
Security risks
None. No auth, crypto, secrets, or user-facing code paths are touched. The new environment variables and vLLM speculative-decoding config are benchmark tuning knobs, not security-sensitive.
Level of scrutiny
Low-to-moderate is appropriate here. This is a benchmark-infra config addition following an extremely well-established pattern in this repo (nearly identical sibling recipes for other model/precision/SKU combos already exist and merge routinely, e.g. the Qwen3.5 AgentX MTP additions in recent history). The new recipe is a variant of an existing, already-validated recipe, with changes isolated to the DSpark speculative-config block and a couple of environment/comment additions. The master-config and perf-changelog entries are mechanical and match the established schema exactly.
Other factors
The bug hunting system's two findings are both correctly flagged as nits: (1) the PR title/body is missing the repo's mandatory bilingual Chinese translation — a real, enforced convention (visible in other recently-merged PRs' commit messages) but purely a process/metadata gap with no code impact; (2) a stale, self-contradictory comment about VLLM_PREFIX_CACHE_RETENTION_INTERVAL was copied forward from the sibling file — comment-only, no functional effect, and pre-existing elsewhere in the repo. Neither issue affects benchmark correctness, parsing, or CI outcomes, and the PR's own validation steps (YAML parsing, generating all six search-space points, git diff --check) cover the mechanical correctness of the change.
| @@ -0,0 +1,143 @@ | |||
| name: "kimik3-vllm-agg-b200-tp8pp2-dspark-agentic" | |||
There was a problem hiding this comment.
🟡 This PR's title and description are English-only, violating the AGENTS.md bilingual PR requirement (title format <English title> / <中文标题> plus a ## 中文说明 section in the body). This is a pre-existing convention that should be fixed but doesn't affect the recipe's functionality — the author can amend the title/body to add the Chinese translation.
Extended reasoning...
AGENTS.md line 7 is an explicit, mandatory repo instruction: "PR and GitHub-issue titles & descriptions must be bilingual — include a Simplified Chinese version in addition to English." It specifies the exact title format <English title> / <中文标题> and requires the PR body to follow the English content with a Chinese translation (e.g. a ## 中文说明 section mirroring the summary). It explicitly states "This applies to every PR and every issue."
This PR's title is [AgentX]: B200 Kimi K3 DSpark with corrected harness — English only, with no / <中文标题> counterpart. The PR body contains only ## Summary and ## Validation sections, both entirely in English, with no ## 中文说明 section or any Chinese translation anywhere in the description.
The bilingual-docs exception called out later in AGENTS.md (around line 84) applies to agent-instruction files (e.g. skill/agent docs), not to PR titles and bodies — so no exemption applies here. This is a straightforward, objectively-checkable violation of a documented MUST requirement, not a matter of subjective style.
Step-by-step verification:
- Read AGENTS.md line 7: requires bilingual title format
<English> / <中文>and a## 中文说明body section. - Read the PR title:
[AgentX]: B200 Kimi K3 DSpark with corrected harness— no/separator, no Chinese text. - Read the PR body: contains
## Summary(English bullets) and## Validation(English bullets) — grep for中文or any CJK characters finds none. - Conclusion: the PR fails both the title-format requirement and the body-translation requirement specified in AGENTS.md.
Impact is process/documentation only — the recipe YAML, master-config entry, and perf-changelog are unaffected functionally, and nothing here changes benchmark behavior, parsing, or CI outcomes. The fix is trivial: the author (or a reviewer) can amend the PR title to append / <中文标题> and add a ## 中文说明 section to the body summarizing the English content in idiomatic technical Chinese, per the translation style guide also in AGENTS.md.
Given this is a metadata/process gap with a trivial fix and no runtime or correctness impact, this should not block merge — hence "nit" rather than "normal" severity, though it is a real, explicitly-mandated violation worth calling out.
| # ~1.4TB of MXFP4 weights off shared Lustre: keep the engine-ready window | ||
| # generous, and let one long AgentX request hold a PP stage beyond vLLM's | ||
| # 300-second model-execution default. | ||
| VLLM_ENGINE_READY_TIMEOUT_S: "3600" | ||
| VLLM_EXECUTE_MODEL_TIMEOUT_SECONDS: "1800" | ||
| # Prefix-cache retention (variant G): 0, on the otherwise-unchanged | ||
| # GPU-resident variant D config. Any positive value must be a multiple of | ||
| # Kimi-K3's KDA-hybrid scheduler_block_size (3145728; the GB recipes' | ||
| # 32768 is hard-rejected at engine init — verified on this PR family), so | ||
| # 0 is the only value below one 3.1M-token scheduler block. | ||
| VLLM_PREFIX_CACHE_RETENTION_INTERVAL: "0" | ||
| # No VLLM_PREFIX_CACHE_RETENTION_INTERVAL: the GB200/GB300 AgentX value | ||
| # (32768) hard-fails engine init on Kimi-K3 — the KDA hybrid gives it a | ||
| # scheduler_block_size of 3145728 and the interval must be a multiple of | ||
| # it ("VLLM_PREFIX_CACHE_RETENTION_INTERVAL (32768) must be non-negative | ||
| # and a multiple of scheduler_block_size (3145728)"). Default retention | ||
| # served fine in earlier runs. | ||
| NCCL_CUMEM_ENABLE: "1" |
There was a problem hiding this comment.
🟡 The new file's aggregated_environment block copies a self-contradictory pair of comments from the sibling agg-b200-tp8pp2-agentic.yaml: lines 67-71 justify setting VLLM_PREFIX_CACHE_RETENTION_INTERVAL: "0", but lines 73-78 immediately follow with a stale "No VLLM_PREFIX_CACHE_RETENTION_INTERVAL" comment describing the var as unset and relying on default retention. This is comment-only (the effective value is unambiguously "0"), but it is now duplicated into a second file and should be cleaned up so maintainers aren'''t misled about actual behavior.
Extended reasoning...
What the bug is: The backend.aggregated_environment block sets VLLM_PREFIX_CACHE_RETENTION_INTERVAL: "0" at line 72, preceded by a comment (lines 67-71) explaining the rationale: any positive value must be a multiple of Kimi-K3'''s KDA-hybrid scheduler_block_size (3145728), so 0 is the only value below one 3.1M-token scheduler block. Immediately after, lines 73-78 contain a second comment block that begins # No VLLM_PREFIX_CACHE_RETENTION_INTERVAL: and ends with Default retention served fine in earlier runs. — describing the variable as unset, relying on vLLM'''s default retention behavior.
These two comment blocks are mutually exclusive descriptions of the same key: one says '''we explicitly set it to 0 because non-zero values are rejected''', the other says '''we deliberately leave it unset and let the default apply'''. Only the first is true — the YAML key is present with value "0" — so the second block is stale documentation that was never removed.
Code path / provenance: I confirmed via grep that this exact contradictory pair (both comment blocks, verbatim) already exists in the pre-existing sibling recipe agg-b200-tp8pp2-agentic.yaml. This PR copies that sibling file to create the new DSpark variant and carries the stale block along with it, so the confusion — previously isolated to one file — is now duplicated into a second file. That doubles the maintenance burden of ever cleaning it up, since a future editor has to remember to fix both copies.
Why nothing catches this: It'''s comment-only text with no YAML key collision, so git diff --check, YAML parsing, and the benchmark sweep are all blind to it — nothing in CI validates that adjacent comments in a config file are logically consistent.
Impact: Zero runtime impact — the effective configuration is unambiguous (VLLM_PREFIX_CACHE_RETENTION_INTERVAL="0") regardless of which comment a reader trusts. The impact is purely on future maintainers: someone skimming this block to understand '''is prefix-cache retention on or off here''' will read two adjacent, contradictory explanations and have to dig into the actual YAML key (or a batch of scheduler-block-size arithmetic) to resolve it.
Step-by-step proof:
- Read lines 67-72: comment explains 0 was chosen because positive values must be a multiple of 3145728; line 72 sets
VLLM_PREFIX_CACHE_RETENTION_INTERVAL: "0". - Read lines 73-78 immediately following: comment begins
No VLLM_PREFIX_CACHE_RETENTION_INTERVAL:and explains that leaving it unset avoids the GB200/GB300 default of 32768 hard-failing on Kimi-K3'''s scheduler_block_size, concluding '''Default retention served fine in earlier runs.''' - Compare: block 1 says the key IS present and set to 0; block 2 says the key is NOT present (implicitly, since it describes '''default retention''' behavior). Both cannot be true simultaneously for the same key in the same file.
grepthe siblingagg-b200-tp8pp2-agentic.yaml— the identical two-block sequence is already there, confirming this is inherited/copied, not newly authored confusion.- Conclusion: the second block (73-78) is leftover from an earlier revision where the key was omitted, and should be deleted or rewritten now that the key is explicitly set to 0.
Suggested fix: Delete the stale # No VLLM_PREFIX_CACHE_RETENTION_INTERVAL: ... comment block (lines 73-78), keeping only the accurate rationale in lines 67-71. Since this also exists in the sibling file, consider fixing both in a small follow-up cleanup.
中文:添加 B200 Kimi K3 AgentX 延迟、均衡吞吐、GPU 常驻高并发和 CPU KV 卸载配置,并通过 srt-slurm 启动多节点聚合式推理。
中文:绕过 B200 上不受支持的自定义集合通信,避免 TP16 在权重加载前停滞。
中文:修正 B200 DEP 启动参数、KV 事件发布、GPU 计数与 CPU DRAM 预算,并禁用跨节点不受支持的 FlashInfer 集合通信。
中文:为跨节点 TP16 和 TEP16 强制使用 PyNCCL,跳过会在 B200 集群上停滞的 MNNVL 自定义集合通信初始化。
中文:为跨节点 TP16 和 TEP16 禁用 Kimi K3 LatentMoE tail fusion,避免 torch symmetric memory 无法跨节点传递文件描述符。
中文:为跨节点 TP16 和 TEP16 禁用 allreduce/RMS fusion,避免 profile_run 再次选择不受支持的 FlashInfer MNNVL 工作区。
中文:改用可运行的 B200 TP8 x PP2 配置,并移除无法启动的跨节点 TP16、TEP16 和 DEP16 配置。
中文:DSpark 草稿模型不支持流水线并行,因此改回无 PP 的 TP16 配置,并保留 GPU 与 CPU KV 容量档位。
Summary
main.Validation
git diff --check