Skip to content

[PowerX] report power sample gaps instead of voiding the window / 报告功耗采样间隙而非作废窗口 - #3353

Open
edwingao28 wants to merge 4 commits into
mainfrom
fix/power-gap-metadata-mirror
Open

edwingao28 wants to merge 4 commits into
mainfrom
fix/power-gap-metadata-mirror

Conversation

@edwingao28

@edwingao28 edwingao28 commented Sep 21, 2026

Copy link
Copy Markdown
Collaborator

Description

Mirror of SemiAnalysisAI/srt-slurm#23: the producer no longer emits sample_gap_exceeded, so multinode.py's recompute must not either, or every lane with a reported gap fails window_validations does not match; single_node.py drops sampling_gap_exceeded too. The largest in-window gap per GPU is still recorded; a stream ending before the window still fails bracketing.

Testing: 240 power tests pass, ruff clean; flipped cases fail on main.

Limit: stored packages carrying sample_gap_exceeded (run 35500671769 C2/C3) stay invalid — replayed locally; rescue needs a follow-up stripping retired reason codes. New runs need #23 first.

AI model disclosure

  • Model/version: claude-fable-5-1 (Claude Fable 5.1)
  • Role: root cause, validator/test changes, replay, PR text; no delegated agents.

Related Issue

Producer: SemiAnalysisAI/srt-slurm#23.

Type of Change

  • Bug fix
  • New feature
  • Configuration change
  • Documentation update
  • Other (please describe)

Checklist

  • I have completed the AI model disclosure and kept it current
  • I have tested my changes locally
  • I have updated documentation if necessary
  • For every change that can affect benchmark performance and every recipe addition or modification, I have appended a new entry to the physical end of perf-changelog.yaml and have not edited historical entries
  • Before merging via reuse, an authorized maintainer (OWNER/MEMBER/COLLABORATOR) has commented /use <run_id> (or the legacy /reuse-sweep-run) on 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.
中文

镜像 SemiAnalysisAI/srt-slurm#23:生产方不再发出 sample_gap_exceededmultinode.py 的重算也必须去掉,否则任何带有已报告间隙的 lane 都会以 window_validations does not match 失败;single_node.py 同样去掉 sampling_gap_exceeded。每 GPU 的窗口内最大间隙仍被记录;窗口结束前停止的数据流仍因未被包围而失败。

测试: 240 项功耗测试通过,ruff 无问题;改判用例在 main 上失败。

限制: 已存储且带 sample_gap_exceeded 的包(run 35500671769 C2/C3)仍然无效——本地已回放;救回需后续在对账前剔除已退役的原因码。新 run 需先合入 #23

AI 模型使用说明: claude-fable-5-1(Claude Fable 5.1)——根因分析、校验器与测试改动、回放、PR 文本;无委派 agent。

关联: 生产方 SemiAnalysisAI/srt-slurm#23改动类型: Bug fix。

Mirror of srt-slurm fix(power) "report over-long sample gaps instead of
voiding the window": the producer no longer emits sample_gap_exceeded, and the
consumer recompute in infx/results/power/multinode.py must agree with the
stored per-window audit rows or every lane with a reported gap fails
"window_validations does not match". The single-node path drops its own
sampling_gap_exceeded verdict for the same reason.

One dcgm-exporter reply past the 2 s request timeout is a 3 s+ gap at a 1 s
cadence, so the 3.0 s threshold turned one slow HTTP reply into a discarded
lane; H200 Kimi-K3 run 35532102407 lost TP16 that way. The largest gap per
device is still recorded (per_gpu_max_sample_gap_s, unchanged field, already
ingested by the dashboard as max_sample_gap_s), and a stream that stops before
the window ends still fails bracketing.

Tests: the multinode fixture can now drop scrapes for one device so the stored
audit carries a real 6 s gap, and the package publishes every metric; the
single-node 8 s-gap case flips from rejected to valid with the gap reported and
energy integrated. Both fail on the previous validators.

中文:镜像 srt-slurm 的 fix(power)「报告超长采样间隙而非作废窗口」:生产方不再发出
sample_gap_exceeded,infx/results/power/multinode.py 的消费侧重算必须与存储的逐窗口
审计行一致,否则任何带有已报告间隙的 lane 都会以 "window_validations does not match"
失败。单节点路径出于同样原因去掉自己的 sampling_gap_exceeded 判定。

1 秒节拍下 dcgm-exporter 一次超过 2 秒请求超时的回复就是 3 秒以上的间隙,3.0 秒阈值
把一次慢 HTTP 回复变成一条被丢弃的 lane;H200 Kimi-K3 run 35532102407 的 TP16 就是
这样丢的。每设备最大间隙仍被记录(per_gpu_max_sample_gap_s,字段不变,仪表板已作为
max_sample_gap_s 摄入),窗口结束前停止的数据流仍因未被包围而失败。

测试:multinode fixture 现在可以让单个设备漏掉若干次 scrape,使存储的审计带上真实的
6 秒间隙,且该包仍发布全部指标;单节点 8 秒间隙用例由拒绝改为有效、报告间隙并完成
能量积分。两者在旧校验器上均失败。
Keyed to the H200 Kimi-K3 latency recipe whose TP16 sweep the 3.0 s verdict
discarded, following the #3206 convention for a processing change that applies
to every telemetry-enabled recipe.

中文:以 H200 Kimi-K3 latency 配方为键记录本次功耗采样间隙处理变更(其 TP16 sweep
曾被 3.0 秒判定丢弃),沿用 #3206 对适用于所有启用遥测配方的处理类变更的记录方式。
Keeps the new perf-changelog entry at the physical end after concurrent entries on main.

中文:合入 main,使新增的 perf-changelog 条目在并发新增条目之后仍位于文件末尾。
Keeps the new perf-changelog entry at the physical end after concurrent entries on main.

中文:合入 main,使新增的 perf-changelog 条目在并发新增条目之后仍位于文件末尾。

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I reviewed this PR and didn't find any bugs. Because it changes the validity semantics for published power/energy results (removing the in-window gap check as an invalidation reason, mirroring an external producer contract change that isn't independently verifiable from this repo), a human look would still be worthwhile.

What was reviewed: the gap-check removal in single_node.py's integrate_power and multinode.py's _check_coverage, and whether bracketing alone still bounds pathological cases — a 2-sample or 0-sample-inside-window stream can pass bracketing and now stays power_valid with the gap only reported, not rejected; the rewritten test_integrate_power_reports_sampling_gap_without_rejecting and its hand-computed 5000.0 J value — note it holds power constant across the gap, so it can't distinguish correct trapezoidal gap interpolation from a broken one.

Extended reasoning...

Diff removes gap-based invalidation (sample_gap_exceeded / sampling_gap_exceeded) from two GPU power validators and updates matching tests; touches only result-processing/validation logic, no auth/crypto/injection surface. No inline findings were produced by the bug hunt, but the change alters what counts as power_valid for published benchmark results and rests on an unverifiable cross-repo contract claim, plus bracketing still permits very sparse (2-sample) streams to pass now that the gap safety net is gone — a real design-policy decision worth a human's sign-off rather than automatic approval.

This review covers commit 003c42d, which is no longer the latest commit on this pull request; later commits are not covered by it.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

1 participant