[AgentX B200] enable Kimi-K3 power and correct DCP metadata / 启用功耗并修正元数据 - #3043
Conversation
收集并校验 AgentX Slurm 功耗结果，保留原生任务失败证据。
启用 Kimi-K3 B200 实测功耗并修正 DCP 与 Offload 元数据。
267c374 to
6fc9524
Compare
|
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 关于重新运行失败任务的文档 |
|
Claude finished @edwingao28's task in 5m 45s —— View job Review of PR #3043
SummaryNo blocking issues found. Two 🟡 warnings posted inline:
Checks that passed: Note: the |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| @@ -0,0 +1,141 @@ | |||
| import os | |||
There was a problem hiding this comment.
🟡 WARNING: This new test file is not executed by any CI workflow.
Why it matters: test-changelog-gate.yml only runs runners/test_slurm_utils.py, and test-process-result.yml runs an explicit file list under utils/ that does not include this file. The "B200 (5) CPU tests" in the PR description therefore only pass locally; a regression in the launcher routing, the failed-job artifact staging, or the exporter import URI would go unnoticed by CI. (test-process-result.yml also does not list runners/slurm_utils.sh in its paths:, so future edits to collect_agentic_power_results would not trigger the collector test added in utils/test_gb300_power_official_contract.py.)
Fix: Add runners/test_kimik3_bh_power.py to the paths: filter and to the pytest invocation in .github/workflows/test-process-result.yml (or move the file under utils/ next to the other power contract tests), and add runners/slurm_utils.sh to that workflow's paths:. I cannot edit workflow files from this bot, so this needs to be done by the author.
| for attempt in 1 2 3; do | ||
| echo "$attempt" > "$logs_dir/power/native-job-status-attempts.txt" | ||
| sacct -X -n -P -j "$job_id" --format=JobIDRaw,State,ExitCode \ | ||
| > "$logs_dir/power/native-job-status.txt" \ | ||
| 2>> "$logs_dir/power/native-job-status.stderr" || true | ||
| if awk -F'|' -v job="$job_id" ' | ||
| $1 == job && $2 !~ /^(PENDING|RUNNING|COMPLETING)$/ { found = 1 } | ||
| END { exit !found } | ||
| ' "$logs_dir/power/native-job-status.txt"; then | ||
| break | ||
| fi | ||
| if [[ "$attempt" != "3" ]]; then sleep 5; fi | ||
| done | ||
| if ! awk -F'|' -v job="$job_id" ' | ||
| $1 == job { found = 1; if ($2 != "COMPLETED" || $3 != "0:0") failed = 1 } | ||
| END { exit (!found || failed) } | ||
| ' "$logs_dir/power/native-job-status.txt"; then | ||
| rc=1 |
There was a problem hiding this comment.
🟡 WARNING: The collector hard-fails every point when sacct returns no row for the job, and sacct is not used by any other launcher in this repo.
Why it matters: If the nscale login node does not have Slurm accounting storage enabled (or sacct is not on PATH for the runner user), the sacct call at line 177 writes nothing to native-job-status.txt, the found check at line 188 fails, rc=1, and every Kimi-K3 B200 power run exits non-zero after the benchmark and power capture succeeded. The only evidence would be native-job-status.stderr inside the LOGS bundle.
Fix: Please confirm sacct -X -n -P -j <id> returns a row on the nscale login node before enabling the lane, or add an explicit fallback so accounting unavailability is distinguishable from a failed job, e.g.:
| for attempt in 1 2 3; do | |
| echo "$attempt" > "$logs_dir/power/native-job-status-attempts.txt" | |
| sacct -X -n -P -j "$job_id" --format=JobIDRaw,State,ExitCode \ | |
| > "$logs_dir/power/native-job-status.txt" \ | |
| 2>> "$logs_dir/power/native-job-status.stderr" || true | |
| if awk -F'|' -v job="$job_id" ' | |
| $1 == job && $2 !~ /^(PENDING|RUNNING|COMPLETING)$/ { found = 1 } | |
| END { exit !found } | |
| ' "$logs_dir/power/native-job-status.txt"; then | |
| break | |
| fi | |
| if [[ "$attempt" != "3" ]]; then sleep 5; fi | |
| done | |
| if ! awk -F'|' -v job="$job_id" ' | |
| $1 == job { found = 1; if ($2 != "COMPLETED" || $3 != "0:0") failed = 1 } | |
| END { exit (!found || failed) } | |
| ' "$logs_dir/power/native-job-status.txt"; then | |
| rc=1 | |
| for attempt in 1 2 3; do | |
| echo "$attempt" > "$logs_dir/power/native-job-status-attempts.txt" | |
| sacct -X -n -P -j "$job_id" --format=JobIDRaw,State,ExitCode \ | |
| > "$logs_dir/power/native-job-status.txt" \ | |
| 2>> "$logs_dir/power/native-job-status.stderr" || true | |
| if awk -F'|' -v job="$job_id" ' | |
| $1 == job && $2 !~ /^(PENDING|RUNNING|COMPLETING)$/ { found = 1 } | |
| END { exit !found } | |
| ' "$logs_dir/power/native-job-status.txt"; then | |
| break | |
| fi | |
| if [[ "$attempt" != "3" ]]; then sleep 5; fi | |
| done | |
| if ! grep -q . "$logs_dir/power/native-job-status.txt"; then | |
| echo "ERROR: sacct returned no accounting row for job $job_id; see $logs_dir/power/native-job-status.stderr" >&2 | |
| fi | |
| if ! awk -F'|' -v job="$job_id" ' | |
| $1 == job { found = 1; if ($2 != "COMPLETED" || $3 != "0:0") failed = 1 } | |
| END { exit (!found || failed) } | |
| ' "$logs_dir/power/native-job-status.txt"; then | |
| rc=1 | |
| fi |
There was a problem hiding this comment.
I reviewed this PR and didn't find any bugs. Because it changes SLURM launcher control flow (continuing past a failed job to stage power evidence) and introduces a second pinned power producer (AGENTX_POWER_SRT_SLURM_PIN) alongside the existing POWER_SRT_SLURM_PIN, a human look would still be worthwhile.
What was reviewed: the USES_AGENTX_POWER routing and pin-verification path in runners/launch_b200-nscale-slurm.sh, the new collect_agentic_power_results helper in runners/slurm_utils.sh, the DCGM exporter enroot URI fix, and the recipe/master-config/perf-changelog trio for the seven B200 Kimi-K3 recipes. Checked and ruled out: dropping AIPERF_SERVER_METRICS_URLS while keeping AIPERF_REQUIRED_SERVER_METRIC_PREFIX — the dropped URL was identical to the frontend's own AIPERF_SERVER_URL (vllm-frontend, non-dynamo topology), which benchmark_lib.sh already auto-scrapes, so it was redundant; and the 3-retry/~15s sacct polling in collect_agentic_power_results treating a non-terminal row as failure — by the time this function runs, stream_slurm_job_log has already waited for the job to leave the live queue, so the retries only cover accounting-DB lag, not a job that's still actually running. The enroot URI fix was verified against the old code path, which did produce a double # separator for the DCGM exporter image; the new test confirms a single separator.
Extended reasoning...
Overview
This PR wires official DCGM power telemetry into the seven existing B200 Kimi-K3 agentic (vLLM, FP4, TP8/PP2, mooncake) recipes and adds a distinct "AgentX power" pipeline (separate srt-slurm pin, staged recipes, per-concurrency power injection) alongside the existing fixed-sequence dcgm-power lane. It touches recipe YAMLs, the matching master-config search-space entries (kv-offloading/dcp-size correction), the B200 Slurm launcher, a new shared collect_agentic_power_results helper in slurm_utils.sh, and adds/extends two test files.
Security risks
No injection, auth, or data-exposure concerns. The main integrity-relevant surface is the new pinned commit (AGENTX_POWER_SRT_SLURM_PIN) used for a git clone+checkout, which is verified against git rev-parse HEAD before use (same pattern as the existing POWER_SRT_SLURM_PIN), and a container image import path (DCGM exporter) that is checksummed and validated with unsquashfs -l. No secrets or credentials are introduced in the diff.
Level of scrutiny
This warrants a full/careful review rather than a rubber-stamp: it changes SLURM launcher control flow (no longer exiting immediately on a failed job, to allow post-job power-evidence staging), adds a second power-producer pin with its own trust/verification discipline, and modifies control-flow-sensitive bash (retry/failure classification for sacct). These are exactly the kind of infra changes where a subtle off-by-one or race could silently corrupt or mask benchmark results, even though I did not find such a bug in the specific paths I traced.
Other factors
Test coverage is solid and exercises the real implementation: test_kimik3_bh_power.py sources the actual launcher script with mocked git/cp/sacct/enroot to test power-path routing, pin-mismatch failure, evidence-preservation-before-exit, and the exporter URI fix; test_gb300_power_official_contract.py adds a parametrized test that calls collect_agentic_power_results directly (not a re-implementation) across completed/failed/incomplete-sweep scenarios. The perf-changelog.yaml entry is a clean append at the tail, and the recipe/master-config pairing (kv-offloading: none + dcp-size: 8) is consistent across all seven recipes. I traced two candidate concerns from the bug-hunting pass to their root cause and found both explained by design (frontend metrics auto-detection redundancy; job-already-left-queue precondition for the sacct retry loop), so I'm not raising them independently, but the launcher's altered exit-on-failure behavior and the new pin's provenance are the kind of design choices worth a maintainer's explicit sign-off.
从共享脚本所在的仓库解析功耗适配器，并在 CI 中执行 B200/H200 路由与结果收集测试。
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=34674595026 |
|
/reuse-sweep-run 34674595026 |
在最终提交触发复用校验，保留已验收的 B200 源码祖先与测量结果。
1 participant
Description
Enable required power for seven B200 Kimi-K3 recipes through the shared collector. Preserve failure artifacts, resolve collector imports, and align DCP8/offload metadata.
Testing: 360 CPU tests, current-head CI and reuse authorization checks passed. Full sweep at
6fc95245: seven power/accounting points and seven tool-schema evals validated; collector-only replay unchanged. Request errors retained.Pending: Core/CODEOWNER approval, checklist signoff and main publication.
中文
中文说明
启用七个 B200 Kimi-K3 配方的必需功耗采集，复用公共采集器。保留失败产物、修正采集器导入，并对齐 DCP8/offload 元数据。
测试： 360 项 CPU 测试、当前提交 CI 及复用授权检查通过。
6fc95245的完整 sweep 中，七个功耗及请求核算点、七个工具 schema eval 均已核验；采集器修复后的重放结果不变。保留请求错误。待完成： Core/CODEOWNER 审批、清单签核及主分支发布。
Related Issue
Scope index and shared policy / 范围索引与共同规则: #3030.
Type of Change
Checklist
perf-changelog.yamland have not edited historical entriesOWNER/MEMBER/COLLABORATOR) has commented/reuse-sweep-runon this PR. Do this only once there is a final full sweep that is all green with evals passing, since after this comment the sweep label will no longer automatically kick off new sweeps. Remove and re-add the label to force one.Note
Medium Risk
Changes official benchmark recipes, master search-space metadata, and Slurm launch/power validation paths that gate published energy numbers and job success semantics.
Overview
Turns on required DCGM power telemetry for the seven B200 Kimi-K3 agentic vLLM Slurm recipes: each YAML now declares
telemetry(dcgm-power, pinned exporter), explicitconcurrencies, and dropsAIPERF_SERVER_METRICS_URLS.Master config for
kimik3-fp4-b200-dynamo-vllm-agentic-dsparknow records DCP8 with KV offload disabled (kv-offloading: none,dcp-size: 8) instead of Mooncake DRAM offload, matching the serving setup.The B200 nscale launcher adds an AgentX power lane for Kimi-K3 agentic runs: checkout a separate pinned
srt-slurmproducer, copy agentic recipes into the clone, inject power concurrencies beforesrtctl apply, and after the job callcollect_agentic_power_results(Slurmsacctchecks + per-concurrencypower_adapter) while still staging logs before failing on power validation. Slurm stream failures no longer abort early on this lane. DCGM exporter cold import now passes the full image ref toimport_squashso Enroot gets a single#registry separator.Adds
collect_agentic_power_resultsinslurm_utils.sh,runners/test_kimik3_bh_power.py, an agentic collection contract test, CI path/trigger updates, and aperf-changelog.yamlentry.Reviewed by Cursor Bugbot for commit 18f1be8. Bugbot is set up for automated code reviews on this repo. Configure here.