[PowerX] separate native multinode collector contract / 拆分原生多节点采集契约 - #3051
Conversation
保留完整请求结果状态与诊断批次,拆分 Slurm 生命周期和原生功耗采集。
|
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 关于重新运行失败任务的文档 |
拆分原生多节点功耗采集契约、UTC 上下文和生命周期屏障,不启用新路径。
d5550f1 to
ba0275c
Compare
保留每个测量窗口的 SMI 上下文工件,覆盖原 #3027 的共享上传规则。
独立生成 prefill 与 decode 角色指标,并在 AMD 采集器被回收前完成取消状态记录。
在现有 CPU 工作流中执行原生采集契约回归,并跟踪对应文件变更。
将已验证的信号就绪屏障移至共享原生采集前置,避免重放 exec 前的竞态。
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 findings, I also checked whether native_multinode.py's unguarded division by expected_prefill_gpus (unlike sibling multinode.py, which guards prefill_avg_power_w/decode_avg_power_w behind if prefill_gpus > 0 / if decode_gpus > 0 checks) is reachable for a decode-only disaggregated topology — fixed_sequence.py raises before either multinode path is called if prefill_gpus <= 0, so a zero-prefill-GPU call is not reachable through the current caller.
Extended reasoning...
Findings were already reported for this PR (AMD teardown identity capture not propagating failure into collector_rc, and native_multinode's audit dict omitting boundary_degenerate_rows despite single_node.py's integrate_power now populating it). I independently re-verified both by reading benchmarks/native_power_collect.sh:24-50 and infx/results/power/native_multinode.py:213-253, and both hold up. Beyond those, I traced the candidate ZeroDivisionError concern (native_multinode.py dividing prefill_gpu_energy_j by expected_prefill_gpus unconditionally when expected_decode_gpus>0) through the only current caller, infx/results/fixed_sequence.py, and confirmed that function raises ValueError("Multinode results require at least one prefill GPU.") whenever prefill_gpus<=0, before it ever reaches native_multinode.run — including in the aggregate-role branch, which reassigns prefill_gpus=aggregate_gpus (>0) rather than leaving it zero. So the decode-only-disagg division-by-zero path is not reachable today, even though the function itself lacks the sibling module's defensive guard. Since the inline findings already signal that a human should look at this PR, I'm not approving; this note is purely to record that one specific extra path was checked and ruled out.
| if [[ "$vendor" == amd ]]; then | ||
| _write_amd_smi_sidecar "$power_dir/gpu_metrics_devices_end.json" list --json |
There was a problem hiding this comment.
🟡 (optional) The AMD branch of the teardown identity capture (_write_amd_smi_sidecar ... gpu_metrics_devices_end.json) has no || collector_rc=1, unlike the NVIDIA branch two lines below; _write_amd_smi_sidecar itself always returns 0 even when amd-smi fails (it only removes the output file and prints a warning). So an AMD node whose end-of-run identity capture fails still writes collector_exit_code=0/lifecycle=complete and reports done-rank=0 to the barrier, while NVIDIA correctly flags the same failure. Fix: check _write_amd_smi_sidecar's underlying amd-smi exit status (have it return the real status, or test the output file) at both call sites (here and the begin-side write at line 66) and set collector_rc=1 on failure, matching the NVIDIA path.
Extended reasoning...
amd-smi fails/errors during teardown (e.g. GPU busy/reset while the process exits); _write_amd_smi_sidecar deletes the empty output and prints a stderr warning but returns 0, so finish() never sets collector_rc=1 and writes done-$rank=0. powerx_wait_collectors done sees rc=0 and reports success for that rank, even though the manifest lifecycle says complete/exit 0. The real gap is only caught later when native_multinode.run() opens the missing gpu_metrics_devices_end.json and raises FileNotFoundError, which the per-node try/except then reports as native_node_invalid — but the barrier/orchestration layer, which is the PR's stated per-node failure-audit mechanism, never sees the failure at collection time, unlike the symmetric NVIDIA path which does via its explicit || collector_rc=1.
Verification: nit. Real vendor asymmetry, but the failure is caught downstream so no bad power data ships. native_power_collect.sh:38 (AMD teardown) writes _write_amd_smi_sidecar ".../gpu_metrics_devices_end.json" list --json with no || collector_rc=1, whereas the NVIDIA branch at lines 40-41 has ... || collector_rc=1. Worse, _write_amd_smi_sidecar (benchmark_lib.sh:277-284) always returns 0: on…
| "observed_gpu_count": integration.observed_num_gpus if integration else 0, | ||
| "per_gpu_role": {uuid: role for role, uuids in roles.items() for uuid in uuids}, | ||
| "per_gpu_sample_counts": integration.per_gpu_sample_counts if integration else {}, | ||
| "per_gpu_max_sample_gap_s": integration.per_gpu_max_sample_gap_s if integration else {}, |
There was a problem hiding this comment.
🟡 (optional) native_multinode's audit dict omits boundary_degenerate_rows, even though it calls integrate_power() from single_node.py, which now populates that field on every PowerIntegration result (single_node.py:718 writes it into its own audit). Docs (docs/results-and-ingestion.md's new "Native multinode telemetry" section) advertise this field as part of the shared teardown-noise accounting, but native multinode runs silently drop it, so any per-GPU teardown-row count is lost for native power_validation_*.json audits. Fix: include integration.boundary_degenerate_rows if integration else {} alongside per_gpu_sample_counts/per_gpu_max_sample_gap_s in the audit dict.
Extended reasoning...
native_multinode.run() calls integrate_power(combined_path, ...) at line ~213, whose PowerIntegration now carries boundary_degenerate_rows (single_node.py PowerIntegration dataclass, populated at lines 399/409/487). The audit dict built at lines 240-253 copies per_gpu_sample_counts and per_gpu_max_sample_gap_s from that same integration object but never reads .boundary_degenerate_rows, so the key is absent from the written validation_result JSON for every native multinode run, unlike single_node's _validation_payload (single_node.py:718) which always includes it (even as {}). A consumer or dashboard that, per this same PR's docs, expects boundary_degenerate_rows on power_validation artifacts will get it for single-node/legacy-multinode runs but not for native multinode runs, silently losing teardown-noise visibility precisely for the new code path this PR introduces.
Verification: nit. The finding is factually correct. native_multinode.run() calls integrate_power() (native_multinode.py:218-219), whose returned PowerIntegration carries boundary_degenerate_rows (populated in single_node.py at lines 399/409/487, dataclass field at 75). The native audit dict (native_multinode.py:238-253) copies integration.per_gpu_sample_counts (line 247) and… | nit. The native_multinode audit…
保留主分支已发布的 Kimi-K3 B200 改动,将当前契约声明追加至 changelog 末尾。
保留原生采集测试并纳入主分支 Kimi-K3 测试,复用已验证的业务改动。
只忽略已定义的诊断 sidecar,保留损坏与缺失点门禁;旧处理器逐点处理并累计失败,新版导入错误不回退。
继承已验证的 sidecar 分类与旧版本逐点结果处理,并保留 UTC 产物测试。
恢复合并时意外改动的测试注释分隔线与空行。
独立核对每种角色的 GPU 数量,并记录窗口外异常样本,窗口内异常仍阻止功耗有效性。
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 6ed8f8b. Configure here.
发布就绪收据前检查采集进程仍存活,并将 AMD 结束身份采集失败写入失败收据。补充真实进程与清单回归测试。
同步主分支的请求诊断与测试,保留原生功耗采集及历史变更记录。
将 UTC 上下文限制在原生采集器,保留常规监控行为,并为取消后的尾部采样和失败回执预留有界清理时间。

Description
Add shared native collection interfaces and offline power adapters. Ordinary monitoring remains unchanged; UTC setup is confined to the native collector. No recipe enables it.
Testing: CPU regressions and real NVIDIA/AMD artifact replays passed. Native cancellation preserves failure receipts.
Scope: Hardware validation belongs to the enabling PRs #3055–#3057.
中文
添加公共原生采集接口和离线功耗适配器。常规监控保持不变;UTC 设置限定在原生采集器内,尚未由任何配方启用。
测试: CPU 回归及真实 NVIDIA/AMD 产物回放通过;原生采集取消流程保留失败回执。
范围: 硬件验证由后续启用采集的 #3055–#3057 完成。
Related Issue
Parent #3026 is merged; activation remains separate.
前置 #3026 已合并;采集启用单独交付。
Type of Change
Checklist
perf-changelog.yamland have not edited historical entries — Not applicable: no executed recipe or monitor change.OWNER/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 power validity semantics and multinode routing for benchmark aggregates; impact is mitigated by opt-in native artifacts, explicit mutual exclusion with SRT packages, and broad regression tests, but mis-staged telemetry could still mark runs invalid.
Overview
Introduces an InferenceX-owned native multinode GPU power path alongside the existing SRT/Slurm package: per-node collectors (
native_power_collect.sh,native_power_lifecycle.sh) run SMI monitoring with ready/stop control receipts, UTC timestamps, andgpu_metrics_context.json, while launchers stage artifacts underLOGS/native_power.Result processing now auto-selects this package when
LOGS/native_power(orPOWERX_NATIVE_DIR) exists and rejects mixed native + SRT trees. The newnative_multinodeadapter validates manifests (topology, clock sync, collector completion, stable GPU UUIDs), merges traces into the benchmark window, and writes a dedicated power audit with per-node receipts and role energy when valid.Single-node integration is tightened: rows outside the formal window with missing/N/A/≤0 W power are counted in
boundary_degenerate_rowsinstead of poisoning validity or faking window bracketing; optional*_context.jsonwithtimestamp_timezone: UTCenables replay across host timezones. CI uploads context sidecars andLOGS/native_power/**, and extends process-result tests for the new scripts and adapter.Reviewed by Cursor Bugbot for commit 583c388. Bugbot is set up for automated code reviews on this repo. Configure here.