[1/4] [Power] feat: validate single-node GPU energy metrics / 校验单节点 GPU 能耗指标

[edwingao28]

Supersedes #2322. This PR was reopened from an upstream branch so repository CI, automated review, and hardware canary workflows can run with the required permissions.

Summary

• Validate single-node, non-disaggregated fixed-sequence GPU-board power over the formal benchmark window.
• Integrate each GPU independently with trapezoidal integration and boundary interpolation, then enforce expected GPU count, complete window coverage, bounded sample gaps, and valid telemetry values.
• Preserve the existing per-GPU avg_power_w meaning and add whole-deployment avg_total_gpu_power_w, total_gpu_energy_j, J/successful query, and J/input, output, and total token.
• Emit power_valid=0 plus an auditable validation sidecar when telemetry is invalid; REQUIRE_POWER=1 makes study/canary jobs fail after artifacts are written.
• Upload the raw benchmark result, raw gpu_metrics.csv, aggregate JSON, and validation JSON as an independent power audit bundle.
• Drop an interrupted, unterminated NVIDIA telemetry row before appending the final post-benchmark sample, preventing row concatenation from silently corrupting energy integration.
• Extend CPU CI coverage to the power aggregator, process-result integration, workflow contract, and NVIDIA monitor shutdown path.

This builds on the measured-power path introduced in #1558. It keeps the single-node aggregation scope separate from the multi-node and role-level work discussed in the closed #1635.

Scope

This PR covers single-node, non-disaggregated fixed-sequence serving, with H100/H200 8k/1k as the initial validation target.

Not included: GB200/GB300 multi-node or disaggregated telemetry, prefill/decode role attribution, srt-slurm changes, MI355X hardware validation, 4P1D/8P1D rack-scale experiments, repeated publication runs, plots, or article work.

Behavior change for AMD: the old best-effort path could publish avg_power_w from amd-smi telemetry. The new validation is strict, and the monitor lifecycle fixes cover NVIDIA only, so AMD runs will typically report power_valid=0 with no power fields until AMD validation lands in a follow-up.

Validation

• python -m pytest utils/test_aggregate_power.py utils/test_process_result.py -q — 81 passed
• Ruff check — passed
• mypy — passed
• Bash syntax, YAML parsing, and full diff checks — passed
• H200 strict 8k/1k canary — passed (run 30054883436): Qwen3.5 FP8, TP8/EP8, c4; power_valid=true, 8/8 GPUs observed, max sample gap 1.001 s, 0 malformed CSV rows.
• H100 strict 8k/1k canary — passed (run 30055976438): Qwen3.5 FP8, TP8/EP1, c4; power_valid=true, 8/8 GPUs observed, max sample gap 1.002 s, 0 malformed CSV rows.
• Independent recomputation of total energy, average power, J/query, and J/token matched both emitted validation sidecars.

中文说明

本 PR 替代 #2322。改为从 upstream 分支提交，以便仓库 CI、自动代码审查和硬件 canary workflow 获得所需权限并正常运行。

• 在正式基准测试窗口内校验单节点、非分离式固定序列推理的 GPU 板级功耗。
• 对每张 GPU 分别采用梯形积分和边界插值，并校验预期 GPU 数量、完整时间窗口覆盖、采样间隔上限和遥测数值有效性。
• 保留现有按单卡计算的 avg_power_w 语义，并新增整套部署的 avg_total_gpu_power_w、total_gpu_energy_j、每个成功查询的能耗，以及每个输入、输出和总 token 的能耗。
• 遥测无效时输出 power_valid=0 和可审计的校验 sidecar；设置 REQUIRE_POWER=1 后，研究或 canary 任务会在写出产物后失败。
• 通过独立的功耗审计产物上传原始基准测试结果、原始 gpu_metrics.csv、聚合 JSON 和校验 JSON。
• 在追加基准测试后的最终采样前删除因监控进程中断而未写完整的 NVIDIA 遥测末行，避免行拼接静默污染能耗积分。
• 扩展 CPU CI，覆盖功耗聚合器、结果处理集成、workflow contract 和 NVIDIA 监控进程停止路径。

本 PR 基于 #1558 引入的实测功耗路径，并将单节点聚合范围与已关闭的 #1635 中讨论的多节点和角色级功能保持分离。

范围

本 PR 仅覆盖单节点、非分离式固定序列推理，初始验证目标为 H100/H200 8k/1k。

不包括：GB200/GB300 多节点或分离式遥测、预填充/解码角色归因、srt-slurm 修改、MI355X 硬件验证、4P1D/8P1D 机架级实验、用于发布的重复实验、绘图或文章工作。

AMD 行为变化：旧的 best-effort 路径可以从 amd-smi 遥测发布 avg_power_w。新校验是严格的，且监控进程生命周期修复目前仅覆盖 NVIDIA，因此 AMD 运行通常会报告 power_valid=0 且不带任何功耗字段，待后续工作补上 AMD 校验。

验证

• python -m pytest utils/test_aggregate_power.py utils/test_process_result.py -q — 81 项通过
• Ruff check — 通过
• mypy — 通过
• Bash 语法、YAML 解析和完整 diff 检查 — 通过
• H200 严格模式 8k/1k canary — 已通过（run 30054883436）：Qwen3.5 FP8、TP8/EP8、c4；power_valid=true，观察到 8/8 张 GPU，最大采样间隔 1.001 秒，CSV 异常行 0。
• H100 严格模式 8k/1k canary — 已通过（run 30055976438）：Qwen3.5 FP8、TP8/EP1、c4；power_valid=true，观察到 8/8 张 GPU，最大采样间隔 1.002 秒，CSV 异常行 0。
• 对总能耗、平均功率、J/query 和 J/token 的独立复算结果与两个 validation sidecar 的输出一致。

[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 10m 22s —— View job

---

Review of PR #2323

• Read full diff vs origin/main
• Review utils/aggregate_power.py (energy integration + validation logic)
• Review utils/process_result.py integration
• Review benchmarks/benchmark_lib.sh and workflow changes
• Check tests (CI test-process-result.yml runs both suites; local sandbox blocked direct pytest execution)
• Post review findings

Summary

No blocking issues found. One 🟡 inline comment: _load_bench_window is now dead code — the new run() path uses _load_benchmark_data and nothing else references it, unlike the deliberately retained legacy helpers.

Things I verified in depth:

• Integration math: per-device trapezoidal integration with boundary interpolation is correct. _interpolate_power can only be reached when the window is bracketed (the samples[0][0] > start_unix or samples[-1][0] < end_unix guard), duplicate timestamps are deduped via the per-timestamp dict so zero-width intervals can't occur, and the gap check's bisect slice (bisect_right(start)-1 … bisect_left(end)+1) covers the correct boundary-neighbor range. The success-path print in run() can't hit an empty metrics dict — power_valid=True implies a non-None benchmark and complete per-GPU energies.
• Topology check consistency: expected_num_gpus = TP × PP_SIZE × PCP_SIZE matches how benchmark-tmpl.yml exports GPU_COUNT (line 272) and how the NVIDIA/AMD launchers allocate GPUs (--gpus/--gres=gpu:$GPU_COUNT), so containers observe exactly the expected device count.
• Bash repair logic (benchmark_lib.sh): the tail -c 1 | grep -q '^$' newline probe and sed '$d' last-line drop are correct for the truncated-row case, and the fresh post-benchmark sample is appended after the repair with matching --format=csv units. Note the repair/append only runs when the monitor is still alive at stop_gpu_monitor; if the monitor died mid-run, validation still catches it via bracketing/gap reasons.
• Fail-closed ordering: the always() power-audit bundle uploads before the strict failure surfaces (the Process result step exits non-zero), while the success-gated bmk_ artifact is correctly withheld — matching the "fail after artifacts are written" contract. REQUIRE_POWER: ${{ inputs.require-power && '1' || '0' }} evaluates correctly for boolean inputs.
• Bench JSON contract: benchmark_serving.py emits completed / total_input_tokens / total_output_tokens as ints, so the strict isinstance(..., int) checks in _load_benchmark_data won't spuriously invalidate real runs.

One behavioral note (by design, but worth stating): runs that previously published best-effort avg_power_w despite malformed in-window rows or sampling gaps (most likely on AMD, where the truncated-row repair is deferred) will now publish no power metrics at all (power_valid=0). That's the intended validated-only publication model, and the sidecar records the reason codes, but expect some existing configs to lose power data until their telemetry paths are hardened.

The pending H100/H200 strict canary is the right gate before relying on REQUIRE_POWER=1 broadly.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c2f27fe453

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[claude]

Beyond the two nits already flagged inline, I checked three other candidate issues and ruled them out. Multinode REQUIRE_POWER failing without writing a validation sidecar is expected, since multi-node telemetry is explicitly out of scope for this PR. REQUIRE_POWER causing the canonical 'Upload result' step to be skipped (it lacks always()) looks intentional — the separate power-audit-bundle step already carries always() and preserves the raw artifacts, while the canonical result is meant to stay unpublished once REQUIRE_POWER flags the run invalid. And a malformed timestamp row invalidating power_valid even when it's far from the benchmark window is unavoidable by construction: an unparseable timestamp can't be classified as far-from-window before it's parsed.

Extended reasoning...

This run re-examined three finder-raised candidates beyond the two nits already posted as inline comments (missing docstrings on new helpers, dead _load_bench_window). For the multinode/REQUIRE_POWER case, process_result.py's multinode branch never calls the aggregator at all, so no sidecar is written when it fails closed — read against the PR description, which explicitly scopes multi-node telemetry out of this PR, that's a deliberate gap rather than a bug. For the 'Upload result' step, benchmark-tmpl.yml shows the canonical bmk artifact upload has no always() guard while the new power-audit-bundle upload does, so a REQUIRE_POWER failure preserves the audit trail but withholds the canonical result — consistent with the PR's stated intent that REQUIRE_POWER runs 'fail after artifacts are written.' For the malformed-timestamp case, integrate_power in utils/aggregate_power.py parses the timestamp before it can apply the far-from-window exemption, so a row with an unparseable timestamp cannot be assessed against the window at all and is conservatively treated as invalid — a reasonable design tradeoff rather than a defect.

[edwingao28]

Hardware canary validation is complete on commit c2f27fe453ef613f4eb0f8d356a6dbd80ffed7dd.

• H200 8k/1k c4 strict-power canary: run 30054883436 — passed. power_valid=true, no reason codes, 8/8 GPUs observed, per-GPU max gap ≤1.0011 s, and 0 malformed CSV rows. Independent recomputation matched the sidecar: 194,539.406 J total, 2,381.250 W average total GPU power, 4,863.485 J/successful query, and 0.587990 J/total token.
• H100 8k/1k c4 strict-power canary: run 30055976438 — passed. power_valid=true, no reason codes, 8/8 GPUs observed, per-GPU max gap ≤1.0021 s, and 0 malformed CSV rows. Independent recomputation matched the sidecar: 204,019.925 J total, 2,533.624 W average total GPU power, 5,100.498 J/successful query, and 0.616645 J/total token.
• Both runs uploaded the raw benchmark result, raw gpu_metrics.csv, aggregate JSON, and validation JSON in their power-audit bundles.

These are implementation canaries, not a formal H100-vs-H200 performance comparison: the canonical c4 recipes use TP8/EP1 on H100 and TP8/EP8 on H200.

硬件 canary 已在提交 c2f27fe453ef613f4eb0f8d356a6dbd80ffed7dd 上完成验证。

• H200 8k/1k c4 严格功耗 canary：run 30054883436 — 通过。power_valid=true，无原因码，观察到 8/8 张 GPU，每卡最大采样间隔 ≤1.0011 秒，CSV 异常行 0。独立复算与 sidecar 一致：总能耗 194,539.406 J、GPU 总平均功率 2,381.250 W、每个成功查询 4,863.485 J、每个总 token 0.587990 J。
• H100 8k/1k c4 严格功耗 canary：run 30055976438 — 通过。power_valid=true，无原因码，观察到 8/8 张 GPU，每卡最大采样间隔 ≤1.0021 秒，CSV 异常行 0。独立复算与 sidecar 一致：总能耗 204,019.925 J、GPU 总平均功率 2,533.624 W、每个成功查询 5,100.498 J、每个总 token 0.616645 J。
• 两次运行均在 power-audit bundle 中上传了原始 benchmark 结果、原始 gpu_metrics.csv、聚合 JSON 和校验 JSON。

这两次运行用于验证实现，不是正式的 H100 与 H200 性能对比：canonical c4 recipe 在 H100 上使用 TP8/EP1，在 H200 上使用 TP8/EP8。

[edwingao28]

@claude review

[Klaud-Cold]

Claude finished @edwingao28's task in 5m 0s —— View job

---

Review of PR #2323 (at 02fff9ee)

• Read full diff vs origin/main
• Review utils/aggregate_power.py (integration + validation)
• Review utils/process_result.py integration
• Review benchmarks/benchmark_lib.sh + workflow changes (incl. new commit 02fff9e)
• Run test suites and lint locally
• Post review findings

Summary

No blocking issues found. This re-review covers the full diff with focus on 02fff9ee (the review-feedback commit) and the merge from main. All previously flagged items are resolved, and I verified the fixes rather than taking them on faith:

• Dead _load_bench_window removed — grep confirms zero remaining references; run() uses only _load_benchmark_data.
• Docstrings added — all six previously bare helpers (_append_reason, _gpu_sort_key, _empty_integration, _write_json_atomic, _patch_power_result, _validation_payload) now carry one-liners, consistent with AGENTS.md conventions.
• Codex P2 (audit bundle telemetry path) correctly fixed — the monitor default is now relative gpu_metrics.csv (benchmarks/benchmark_lib.sh:112), and I confirmed this resolves to the uploaded path: every launcher I checked (launch_h100-cw.sh:36-38, launch_h200-cw.sh:46-48, launch_h200-nb.sh, launch_mi355x*.sh:320-323) mounts $GITHUB_WORKSPACE at /workspace with --container-workdir=/workspace/, so the in-container relative write lands exactly where the power_audit_* and gpu_metrics_* artifacts upload from. The ${GPU_METRICS_CSV:-...} form also now respects a pre-set env var. The new test_gpu_monitor_default_matches_uploaded_artifact_path contract test locks this in.

Verified locally

• python -m pytest test_aggregate_power.py test_process_result.py -q — 82 passed (81 + the new workflow-contract test from 02fff9e).
• ruff check (repo default rules) — passed; bash -n benchmarks/benchmark_lib.sh — OK; all three touched workflow YAMLs parse.

Spot-checks on the core logic (unchanged conclusions from the earlier deep review, re-confirmed on the merged head)

• Trapezoidal integration with boundary interpolation is correct; _interpolate_power is only reachable when the window is bracketed, and duplicate timestamps are averaged so zero-width intervals can't occur.
• expected_num_gpus = TP × PP_SIZE × PCP_SIZE in process_result.py:250 matches GPU_COUNT in benchmark-tmpl.yml:272.
• Fail-closed ordering holds: the always() power-audit bundle uploads before the strict Process result failure, while the success-gated bmk_ artifact stays withheld.
• The bash truncated-row repair (tail -c 1 newline probe + sed '$d') is correct, and the two new subprocess tests exercise both the append and the truncation path against a fake nvidia-smi.
• The internal-error fallback (record_power_internal_error) keeps the audit trail intact even when the aggregator itself throws, in both best-effort and strict modes.

Non-blocking observations

1. CLI --csv default inconsistency — aggregate_power.py:780 still defaults to /workspace/gpu_metrics.csv while the monitor default is now relative gpu_metrics.csv. Inside containers (workdir /workspace) these are the same file, and process_result.py passes an explicit path with a fallback chain, so this only affects someone running the CLI by hand from another cwd — where it fails closed with telemetry_file_missing. Fine to leave; a one-word default change would remove the last trace of the old path. Fix this →
2. Timezone coupling (pre-existing) — _parse_timestamp interprets naive nvidia-smi timestamps in the processing host's local time, while the bench window is epoch-based. If the telemetry container and the host running process_result ever disagree on TZ, runs will fail closed with benchmark_window_not_bracketed rather than corrupt data — the safe direction, and both canaries confirm the paths agree today. Worth remembering if a future runner surfaces that reason code unexpectedly.
3. Draft PRs now run the CPU suite — test-process-result.yml dropped the draft != true guard, and a contract test asserts its absence, so this is clearly deliberate. It's a cheap ubuntu job; just noting the behavior change.

The two green strict canaries (H200 30054883436, H100 30055976438) plus the independent recomputation match give good confidence in the end-to-end path. LGTM from my side — this is ready for CODEOWNER checklist review.

--- · Branch

[claude]

No new issues found. Commit 02fff9e addresses all three items from my prior review round: the audit-bundle path mismatch (GPU_METRICS_CSV default now matches the uploaded relative path), the dead _load_bench_window helper (removed), and the missing docstrings on the six new helpers (added). This remains a substantial feature spanning power-integration math, bash telemetry handling, and CI workflow wiring, so a final human pass is still worthwhile before merge.

What was reviewed this round:

• Confirmed the GPU_METRICS_CSV default fix aligns with the power-audit bundle's relative path and the new workflow-contract regression test covers it.
• Confirmed _load_bench_window is gone and the six previously-undocumented helpers now have docstrings.
• Checked malformed-timestamp handling far outside the benchmark window — only rows within [start - max_gap, end + max_gap] affect validity, so a bad row elsewhere in the CSV doesn't invalidate the result.
• Checked REQUIRE_POWER=1 on multinode — agg_<run>.json is already written unconditionally before that check runs, so the failure isn't silently artifact-less; the single-node validation sidecar is simply out of scope there by design.

Extended reasoning...

This PR adds single-node GPU power validation (trapezoidal energy integration, GPU-count/window/gap checks) across utils/aggregate_power.py, utils/process_result.py, benchmarks/benchmark_lib.sh, and three GitHub Actions workflow files, with accompanying pytest coverage and two passing hardware canaries (H100/H200).

No security-sensitive surface is introduced — all inputs are CSV/JSON telemetry files and CI-controlled environment variables, with no auth, crypto, or permission logic touched.

The correct scrutiny level here is moderate-to-high: this is not a mechanical change. It has non-trivial integration math (per-device trapezoidal integration with boundary interpolation), touches CI workflow files (artifact upload paths, new require-power input plumbing across three workflow files), and modifies bash process lifecycle handling (stop_gpu_monitor's truncated-row repair). All three findings from my prior review round were addressed in commit 02fff9e, and this run's bug-hunting pass found nothing new, but the combination of size, cross-language surface (Python/bash/YAML), and CI-affecting changes still merits a human reviewer's final sign-off before merge.

[edwingao28]

sweep passed https://github.com/SemiAnalysisAI/InferenceX/actions/runs/30299816374