[PowerX] enable Kimi-K3 AgentX power backfills / 启用功耗补测 - #3030
[PowerX] enable Kimi-K3 AgentX power backfills / 启用功耗补测#3030edwingao28 wants to merge 2 commits into
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 关于重新运行失败任务的文档 |
为 Kimi-K3 AgentX 补测启用必需功耗采集，保留失败诊断并修正 AMD 收尾与硬件元数据。 Signed-off-by: Wenyao Gao <wenyao.gao28@gmail.com>
53e55b6 to
09f2cf9
Compare
|
Claude finished @edwingao28's task in 6m 26s —— View job Review of PR #3030
Summary: No blocking issues found. Two 🟡 warnings posted inline: the new One question for the author, not a defect I can confirm: the GB200/GB300 recipes set |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| 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.
🟡 WARNING: Every Kimi-K3 power run now hard-fails when sacct cannot produce a terminal row for the job, and this is the first and only sacct dependency in runners/.
Why it matters: sacct needs slurmdbd accounting storage. If it is not installed or accounting is disabled on any of the four clusters (Nscale B200 is the least certain), the status file stays empty after three attempts, !found trips, rc=1, and a fully successful job with valid power exits non-zero. The unit tests only exercise a stubbed sacct, and none of the seven qualification runs have reached this step yet, so the branch is unverified on real hardware. The failure message would also be indistinguishable from a genuine Slurm failure.
Fix: Fall back to scontrol show job (or squeue's last-seen state) when sacct returns nothing, and make the failure reason explicit:
| 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 | |
| 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 | |
| if [[ ! -s "$logs_dir/power/native-job-status.txt" ]]; then | |
| # Accounting unavailable: fall back to the controller's view so a | |
| # successful job is not failed for a missing sacct row. | |
| scontrol show job "$job_id" > "$logs_dir/power/native-job-scontrol.txt" 2>&1 || true | |
| if ! grep -Eq 'JobState=COMPLETED .*ExitCode=0:0' "$logs_dir/power/native-job-scontrol.txt"; then | |
| echo "ERROR: could not confirm job $job_id COMPLETED via sacct or scontrol" >&2 | |
| rc=1 | |
| fi | |
| else | |
| echo "ERROR: job $job_id did not finish COMPLETED with ExitCode 0:0" >&2 | |
| rc=1 | |
| fi | |
| fi |
| @@ -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: .github/workflows/test-process-result.yml runs pytest from utils/ with an explicit file list, and test-changelog-gate.yml only picks up runners/test_slurm_utils.py. The 9 B200/H200 launcher tests described in the PR checklist therefore only ran locally, and future edits to the two launchers' routing or failure-staging blocks will not be caught.
Fix: Either move this file to utils/ and add it to the pytest list in test-process-result.yml (that workflow already triggers on runners/launch_b200-nscale-slurm.sh and runners/launch_h200-dgxc-slurm.sh), or add runners/test_kimik3_bh_power.py to the test-changelog-gate.yml invocation alongside runners/test_slurm_utils.py. I cannot edit workflow files from this bot.
将已下架的 5894fdf 镜像替换为可获取的 3696c77 构建，并追加性能变更记录。此提交仅修复镜像引用；DSXE RDMA 兼容性及端到端功耗验证仍待完成。
There was a problem hiding this comment.
Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.
Beyond the inline finding, I also checked two other candidates that were ruled out: _wait_for_amd_stop_coverage in benchmarks/benchmark_lib.sh falls back to the old fixed sleep when _amd_monitor_min_covered_tick returns empty, which is a safe degrade for non-epoch/unusable streams rather than a bug; and in runners/launch_b200-nscale-slurm.sh, a Slurm job that fails before creating LOGS_DIR still exits non-zero via the pre-existing [ ! -d "$LOGS_DIR" ] check even though the AgentX diagnostics-staging path is skipped, so failures are still correctly propagated.
Extended reasoning...
Beyond the already-posted inline finding (missing REQUIRE_POWER/ENABLE_AGENTX_POWER in the new B200/H200 recipe benchmark.env blocks), I traced two other candidates from the ruled-out list to confirm they are not additional bugs: (1) benchmarks/benchmark_lib.sh's _wait_for_amd_stop_coverage/_amd_monitor_min_covered_tick pair — when the tick helper returns empty (non-epoch timestamps or unusable CSV), the caller intentionally falls back to the pre-existing fixed sleep-and-return behavior rather than hanging or silently skipping cleanup, so this is a deliberate compatibility fallback, not a regression. (2) runners/launch_b200-nscale-slurm.sh's post-stream_slurm_job_log flow — for the new AgentX path, a Slurm job that fails before LOGS_DIR is ever created no longer exits immediately at the stream_slurm_job_log call, but it still hits the pre-existing if [ ! -d "$LOGS_DIR" ]; then ... exit 1; fi guard a few lines later and exits non-zero, so job failures are still correctly propagated even though the newly-added diagnostics-staging message is skipped in that specific corner case.
| type: custom | ||
| concurrencies: [1] | ||
| command: bash /infmax-workspace/benchmarks/multi_node/agentic_srt.sh | ||
| env: |
There was a problem hiding this comment.
🟡 (optional) The 7 new B200 and 3 new H200 Kimi-K3 AgentX recipes never set ENABLE_AGENTX_POWER/REQUIRE_POWER in benchmark.env, unlike the 13 GB200/GB300 recipes added in the same PR which set both to "1". Without REQUIRE_POWER, benchmark_lib.sh's early _write_agentx_multinode_window check always returns 0 even when the power window/telemetry setup is broken, so a bad setup is only caught by the login-node's post-hoc collect_agentic_power_results after the full ~1h benchmark runs, wasting the GPU job instead of failing in seconds like GB200/GB300 do. Fix: add REQUIRE_POWER: "1" (and ENABLE_AGENTX_POWER: "1") to all 10 B200/H200 recipe env blocks.
Extended reasoning...
run_agentic_replay_and_write_outputs() in benchmarks/benchmark_lib.sh reads REQUIRE_POWER via case "${REQUIRE_POWER:-0}" before calling _write_agentx_multinode_window; when unset it omits --require-power, so power_adapter.py's run_agentic_power/_fail_multinode_adapter always return 1 if require_power else 0 i.e. 0. The if [ "$power_rc" -ne 0 ]; then return "$power_rc"; fi guard right after the 'running' window write therefore never trips for B200/H200, letting the full replay run to completion even with broken telemetry. GB200/GB300 recipes set REQUIRE_POWER: "1" so they fail within seconds at the same check. Only the launcher's collect_agentic_power_results (hardcoded --require-power) catches it afterward on B200/H200, after the whole multi-node job already ran.
Verification: Severity: nit. The factual/mechanistic claim is real and verified. The 13 GB200/GB300 recipes set ENABLE_AGENTX_POWER="1" and REQUIRE_POWER="1" in benchmark.env (e.g. agg-gb200-tp8pp2-mooncake-c16-agentic.yaml:133-134), but the 7 B200 and 3 H200 recipes add only the telemetry block and concurrencies, no power env keys. On the compute node, agentic_srt.sh:149 calls… | nit. The 10 B200/H200…
|
close as this pr has been replaced by multiple smaller pr |
1 participant
Description
Reference only; do not merge or add sweep labels. Kimi-K3 power work is split below; source
2f3d637f6is preserved inchore/preserve-kimik3-3030-full.Testing: Existing CPU/matrix checks passed; 38 source files match the split against
main@900f1989d, with scoped changelog entries.Pending: Hardware validation and publication. Retarget stacked PRs to
mainafter dependencies merge; validate one scope at a time under the merge policy. Child PRs retain specific blockers.mainmainmainOne-hour profiles; planned counts, not completed measurements. / 标准一小时 Profile；计划任务数，非已完成测量。
中文
中文说明
仅作参考；请勿合并或添加扫描标签。 Kimi-K3 功耗工作已按下表拆分，原始提交
2f3d637f6保存在chore/preserve-kimik3-3030-full。测试： 已有 CPU/矩阵检查通过；相对
main@900f1989d，拆分结果逐字节还原 38 个源文件，仅按范围重组 changelog。待完成： 硬件验证和发布。堆叠依赖合并后，将子 PR 调整为以
main为基线，按合并规则逐个范围验证；具体阻塞见各子 PR。表中为标准一小时 Profile 的计划基准/评测任务数，并非已完成测量。Related Issue
Related implementation / 相关实现: #2767, #2926, #2927, #3026.
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 affect benchmark power validity, Slurm result collection, and many production recipe configs; mistakes could fail sweeps or mis-attribute energy, but scope is telemetry and launch plumbing rather than serving auth or data paths.
Overview
Turns on required DCGM / AgentX power for Kimi-K3 multi-node agentic recipes on B200, H200, GB200, and GB300: Slurm YAML gains
telemetryblocks, explicitconcurrencies, and (on GB lanes)ENABLE_AGENTX_POWER/REQUIRE_POWERwith head client placement. B200 recipes drop localhost metrics URLs where telemetry replaces that path.AMD monitor teardown in
benchmark_lib.shis reworked:amd-smistreams through a per-run FIFO so rows flush to disk before kill; stop waits on the CSV until every GPU has a usable integer-second tick at or after the stop request (configurableAMD_MONITOR_STOP_TIMEOUT_S), with fast abort teardown on signals. AgentX traps call stop in abort mode so cancelled runs do not block on coverage.Power integration skips N/A or non-positive samples outside the formal benchmark window, recording them as
boundary_degenerate_rowsinstead of invalidating energy or faking end bracketing.Cluster launchers route Kimi-K3 power jobs to a pinned
srt-slurmproducer, inject concurrency lists, and runcollect_agentic_power_results(Slurm accounting check, staged logs,--require-poweradapter) even when the native job fails. B300 master config moves to clusterb300-dsxeand vLLM image3696c77; B200 Kimi metadata is corrected to DCP8 withkv-offloading: none. CI docs andperf-changelog.yamldocument backfill rules.Reviewed by Cursor Bugbot for commit 2f3d637. Bugbot is set up for automated code reviews on this repo. Configure here.