[4/4] [Power] feat: strict AMD gpu monitor lifecycle for MI355X / MI355X 单节点功率监控严格生命周期

[edwingao28]

Hardens the AMD branch of the single-node GPU power monitor so MI355X runs produce strictly valid energy data through the same pipeline as NVIDIA. Stacked on #2323 (PR1); part of the cross-platform power series (#2323 → #2437 → #2456).

What changes

• stop_gpu_monitor waits one extra tick on AMD before killing the watch stream, so the last benchmark window's right edge is always bracketed for boundary interpolation. The amd-smi one-shot CSV has no timestamp column, so the NVIDIA-style "append one post-exit sample" approach cannot work on AMD.
• The awk stage of the AMD watch pipeline now flushes per row; killing the pipe can no longer discard buffered samples.
• Start and stop write hardware energy-accumulator snapshots (gpu_metrics_energy_{start,end}.csv) plus a gpu_metrics_identity.json static dump. The accumulator endpoint delta gives auditors an independent cross-check of the trapezoid integral.
• The truncated-tail repair is factored into a shared helper used by both vendors; NVIDIA behavior is unchanged (existing tests still pass).
• benchmark-tmpl.yml uploads the three sidecars in the gpu_metrics and power_audit artifacts (if-no-files-found: ignore, so non-AMD lanes are unaffected).

Ground truth (measured on an MI355X compute node inside the production sglang-rocm container, AMDSMI 26.2.0 / host ROCm 7.1.1):

• one-shot p95 invocation latency 80.9 ms at 1 Hz; 60/60 sample presence on all visible GPUs
• energy accumulator monotonic, unit joules, per-second delta ≈ socket_power reading
• watch-mode CSV: preamble line + timestamp,... header, epoch-second timestamps, bare-number socket_power; the header also contains power_management, so a test now pins column detection to socket_power
• one-shot metric -E --csv carries gpu,total_energy_consumption

Tests: 3 new bash lifecycle tests (mirroring the existing NVIDIA harness) + 1 real-header column-detection pin + 11 accumulator cross-check tests; test_process_result.py + test_aggregate_power.py = 97 passed. The cross-check ships in aggregate_power.py as an advisory accumulator_check block in the validation payload — it never affects power_valid.

中文说明

把单节点 GPU 功率监控的 AMD 分支补齐到严格生命周期,使 MI355X 与 NVIDIA 走同一条能量管线:

• stop 前多等一拍,让 watch 流覆盖窗口右沿(amd-smi 一次性 CSV 无时间戳列,无法照搬 NVIDIA 的补样做法);
• awk 逐行 fflush,kill 不再丢缓冲样本;
• 起止两端写硬件能量累加器与 identity 快照,累加器差值可独立交叉验证梯形积分;
• 截断行修复抽为两 vendor 共用函数,NVIDIA 行为不变;
• workflow 上传三个 sidecar,非 AMD lane 不受影响。

依据为 MI355X 计算节点上生产容器内的实测(AMDSMI 26.2.0):调用延迟 p95 80.9ms、累加器单调且单位为焦耳、每秒增量与瞬时功率一致、watch 表头含 power_management 陷阱列(已用测试钉死列探测)。

---

Anchor results, N=3 per platform (mean over 3 independent dispatches; CV% in parentheses; Qwen3.5-397B-A17B-FP8, 8k1k, sglang). MI355X: 30960241075, 30962586410, 30964321893. B200: 30958610420, 30962588049, 30964371690. Every window power_valid=1.

hw	tp	conc	N	J/out-tok (CV%)	W/GPU (CV%)	J/query	out tok/s/GPU	TTFT s	TPOT s
b200	4	4	3	3.846 (2.37)	495.5 (3.22)	3,539.0	128.8	0.361	0.0072
b200	4	32	3	1.680 (1.39)	731.3 (1.38)	1,554.6	435.4	0.946	0.0169
b200	8	4	3	5.665 (0.80)	425.4 (0.61)	5,212.8	75.1	0.368	0.0061
mi355x	4	4	3	4.882 (1.16)	502.8 (0.90)	4,492.1	103.0	0.387	0.0091
mi355x	4	32	3	1.967 (0.34)	764.1 (0.42)	1,820.6	388.4	0.870	0.0191

On MI355X the hardware energy-accumulator cross-check ran in production on all six windows: relative error vs the trapezoid integral 0.07%–1.30%, all within the 5% advisory tolerance. The b200 TP8 c4 row rides along because the sweep matrix has no TP filter; it is reported for completeness, not compared against TP4 rows.

中文:两平台各 3 次独立 dispatch 的锚点复现表。J/out-tok 的 CV 最差 2.37%(b200 TP4 c4),MI355X 两档均 ≤1.16%;MI355X 六个窗口的硬件累加器交叉验证误差 0.07%–1.30%,全部在 5% 容差内。TP8 c4 行是 sweep 矩阵无 TP 过滤的顺带产物,仅作记录。

[github-actions]

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 As a PR reviewer and CODEOWNER, I have reviewed this and have.

For PR verification, add the full-sweep-fail-fast label (strongly recommended) to this PR — the benchmark sweep only runs on labeled PRs. Use full-sweep-enabled only if you need matrix jobs to keep running past a failure.

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 模板，包括保留英文语句 As a PR reviewer and CODEOWNER, I have reviewed this and have。

如需进行 PR 验证，请为此 PR 添加 full-sweep-fail-fast 标签（强烈推荐）— 基准测试 sweep 仅在带有标签的 PR 上运行。仅当需要矩阵任务在失败后继续运行时才使用 full-sweep-enabled。

PR 作者有责任确保合并后所有 GitHub Action 任务完全通过。 很多时候失败只是偶发抖动（flake），重新运行失败的任务即可解决。参见 GitHub 关于重新运行失败任务的文档

[claude]

Claude finished @edwingao28's task in 3m 45s —— View job

---

Review

• Gather context (diff vs wenyao/validated-single-node-power, read changed files)
• Review benchmarks/benchmark_lib.sh lifecycle changes
• Review utils/aggregate_power.py accumulator cross-check
• Review tests and workflow changes
• Post review feedback

LGTM - no blocking issues found.

Checked specifically: the refactored _repair_truncated_gpu_metrics_tail preserves the exact NVIDIA append-gating semantics of the old inline code; the AMD pre-kill sleep (interval + 2) is correctly reasoned for integer-second amd-smi timestamps; sidecar naming is consistent between bash (${output%.csv}_energy_start.csv) and Python (csv_path.stem); the advisory accumulator_check is computed after power_valid is finalized and exceptions are contained, so it cannot affect validity; and the multi-path artifact upload keeps gpu_metrics.csv at the artifact root with if-no-files-found: ignore leaving NVIDIA lanes unaffected. No master-config changes, so no perf-changelog.yaml entry is required.

Note: I could not run pytest in this environment (command approval denied), so I relied on static review; the suites run in CI.

[claude]

I didn't find any bugs, but this PR is worth a human look given it makes several hardware-timing assumptions about AMD's amd-smi tool (a magic interval+2s sleep to bracket the window boundary, buffering behavior under kill) that are hard to verify outside the actual MI355X hardware they were tuned against, and it touches the power-measurement pipeline behind the published benchmark anchor numbers.

What was reviewed: the AMD stop-lifecycle sleep/kill/repair sequence in benchmark_lib.sh, the awk fflush buffering fix, and the new cross_check_accumulator GPU-set matching/tolerance logic in aggregate_power.py (advisory only, does not affect power_valid). Checked that the accumulator cross-check intersects GPU IDs between start/end snapshots and the stream rather than assuming matching populations, and that a hung amd-smi call in the monitor sidecars is bounded by the surrounding job timeout rather than blocking indefinitely — both raised as candidates and not reproducible in the diff.

Extended reasoning...

This PR hardens the AMD branch of the single-node GPU power-monitor lifecycle (benchmark_lib.sh), adds an advisory hardware energy-accumulator cross-check (utils/aggregate_power.py), and wires three new sidecar artifacts through the benchmark CI template. It is part of a stacked cross-platform power-measurement series (#2323 → #2437 → #2456 → this PR).

Security risk is minimal — no auth, secrets, or user-facing surface is touched; the changes are internal CI/benchmarking tooling that shells out to amd-smi/nvidia-smi and writes local CSV/JSON artifacts.

The level of scrutiny warranted is higher than a typical CI tweak because this pipeline computes the power/energy metrics used in externally-published benchmark numbers (the PR body includes an anchor-results table with J/token and W/GPU figures). The bash changes rely on empirically-tuned timing (a before killing the AMD watch stream, PYTHONUNBUFFERED + per-row fflush to avoid losing buffered samples on kill) that the author validated against real MI355X hardware runs, but which cannot be independently re-verified without that hardware. The Python cross-check is well isolated and explicitly advisory (never affects power_valid), which limits its blast radius, but the bash lifecycle changes directly gate whether a run's telemetry is considered valid.

Other factors: test coverage is extensive (new bash lifecycle tests mirroring the NVIDIA harness, a real-header column-detection regression test, and 11 accumulator cross-check unit tests), and the bug-hunting pass found no defects, including on the specific hardware-timing and GPU-set-matching concerns a finder raised. Given the combination of non-trivial hardware-specific timing logic and downstream use in published metrics, I'd still like a human familiar with the AMD tooling to confirm the timing assumptions before merge.