[edwingao28]

Summary

Multinode points now measure GPU power by default wherever the cluster has a GPU exporter: DCGM on GB200, GB300, B200, B300 and H200, the native exporter on MI355X. CPU power comes from Grace (GB200/GB300) and RAPL (MI355X). Failed telemetry keeps the result unless the row sets power: true. Recipes with dedicated etcd/NATS nodes skip collection. No config key changes, so no changelog entry.

Testing: Unit tests pass. Hardware proofs, each a point without power:

Cluster	Point	GPU power	CPU power
GB200	MiniMax AgentX	Valid, 4 GPUs, 324 W each	Valid Grace, 187 W
B300	Qwen3.5 FP8 AgentX disagg c4	Valid, 8 GPUs, 308 W each	No exporter
MI355X	DeepSeek-R1 FP4 8k1k c1, 1P1D TP8	Valid, 16 GPUs, 362 W each	Valid RAPL, 1483 W

MI355X TP4-prefill variants idle some GPUs, so their power is invalid; results stay.

PR checklist and change type

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
• If python -m infx.matrix.changed --base origin/main lists affected config keys, I have appended one new entry covering them to the physical end of inferencex-e2e/perf-changelog.yaml; if it lists none, I have added none. I have not edited historical entries
• If this PR appends an inferencex-e2e/perf-changelog.yaml entry, it carries exactly one primary sweep label (a maintainer applies it on fork PRs): full-sweep-fail-fast (recommended), full-sweep-enabled, or non-canary-full-sweep-enabled. Optional modifiers all-evals, evals-only, and agentx-fast require a primary label; the last two block reuse while applied.
• 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 primary sweep label will no longer automatically kick off new sweeps. Remove and re-add the primary sweep label to force a new sweep.

[github-actions]

Thanks for the contribution!

• Review: If this PR changes files owned by someone other than a repository admin or @SemiAnalysisAI/core, ask one eligible CODEOWNER to complete the latest PR_REVIEW_CHECKLIST.md before contacting a core maintainer on Slack. Follow the template exactly, including As a PR reviewer and CODEOWNER, I have reviewed this and have, so sign-off verification triggers.
• PR verification: Sweeps run only when the PR appends an inferencex-e2e/perf-changelog.yaml entry and carries exactly one primary label: full-sweep-fail-fast (strongly recommended; canary plus per-matrix fail-fast), full-sweep-enabled (canary; matrix jobs continue after a failure), or non-canary-full-sweep-enabled (no canary or fail-fast). The modifiers all-evals, evals-only, and agentx-fast require a primary label. On fork PRs, a maintainer applies the label. See sweep labels and reuse.
• After merging: PR authors must ensure all GitHub Actions jobs pass. Transient failures often pass on rerun; see how to rerun failed jobs.

中文

感谢你的贡献！

• **审阅：**如果 PR 修改的文件归属于仓库管理员及 @SemiAnalysisAI/core 之外的 CODEOWNER，请先联系一位有资格的 CODEOWNER 填写最新的 PR_REVIEW_CHECKLIST.md，再通过 Slack 联系核心维护者。必须严格遵循模板，并保留 As a PR reviewer and CODEOWNER, I have reviewed this and have，才能触发签核验证。
• **PR 验证：**只有当 PR 在 inferencex-e2e/perf-changelog.yaml 末尾追加了条目，并且恰好带有一个主标签时，才会运行扫描：full-sweep-fail-fast（强烈推荐；canary 加逐矩阵 fail-fast）、full-sweep-enabled（有 canary；矩阵任务在失败后继续运行）或 non-canary-full-sweep-enabled（无 canary，也无 fail-fast）。修饰标签 all-evals、evals-only 和 agentx-fast 必须与主标签一起使用。fork PR 的标签由维护者添加。参见扫描标签与复用。
• **合并后：**PR 作者必须确保所有 GitHub Actions 任务通过。临时性失败通常可以通过重新运行恢复；参见重新运行失败任务的说明。

[claude]

Claude finished @edwingao28's task in 1m 53s —— View job

---

LGTM - no blocking issues found

_{The PR's pytest suites were not run here because this job's permissions blocked uv run pytest.}

[claude]

I reviewed this PR and didn't find any clear-cut bugs. Because it's a large change (88 files) introducing default NVL72 power-telemetry collection across the SLURM launch path, driver init, power-decision logic, and collection enforcement — with several intertwined code paths for cluster-default vs. legacy per-recipe power opt-in and require_power/adapter propagation — a human look would still be worthwhile.

What was reviewed: the decide_power/resolve_power rewrite in power.py (cluster-default settings path vs. legacy per-recipe DCGM opt-in, require_power propagation); adapter flag handling in the settings branch of resolve_power — not carried into the final PowerDecision, checked whether any caller relies on it (none found in this diff, so not flagged as a confirmed bug); single-node AgentX --require-power enforcement wiring in collect.py; and multinode REQUIRE_POWER env propagation in drivers/srt/__init__.py for the eval-only/cluster-default branch.

Extended reasoning...

The change adds default GPU/CPU power telemetry collection for GB200/GB300 SRT recipes: it touches infx/launch/drivers/srt/{init.py,power.py,collect.py}, infx/clusters/slurm.py, infx/results code, ~70 recipe YAMLs, docs, and tests. No auth/crypto/injection surface is touched; the sensitive logic is benchmark-config correctness (whether power enforcement/defaults apply on the right lanes). Four candidate issues around require_power/adapter propagation were investigated and ruled out as confirmed bugs, but the branching is dense enough (cluster-default settings vs legacy opt-in, eval-only short circuits, multinode vs single-node enforcement) that I deferred rather than approved given the size and the non-trivial design decisions embedded in the propagation logic.

[edwingao28]

@claude. review plz

[Klaud-Cold]

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

---

Review of PR #3670: NVL72 power collection on by default

I found no blocking bugs. Below are the checks I ran myself and a few non-blocking notes.

• Gather context (diff vs feat/powerx-nvl72-cpu-power, 2 commits)
• Review launch/power code (power.py, collect.py, srt/__init__.py, slurm.py, request.py)
• Review results code (power_adapter.py, fixed_sequence.py, artifacts.py)
• Review recipes, scripts, workflow, changelog
• Run tests and validate recipes against the pinned srtctl
• Post review

Checks I ran

• Tests: the 7 affected suites pass (251 passed), including test_nvl72_power_defaults.py. That file imports srtctl, so the utils/srt-slurm submodule has to be checked out first. That is the same pattern other tests already use.
• Recipe validation: I took every recipe that the GB200/GB300 keys in nvidia-master.yaml reference (110 refs, with variants and zip overrides expanded). For each one I applied telemetry_arguments(...) and loaded it through the SrtConfig schema from the pinned srt-slurm (098e15a). There were 0 failures.
  • To check that the validator really runs, I tried the base-branch dsv4/vllm/.../disagg-1p4d-dep4-tp8-c4-mtp.yaml. It is rejected with telemetry requires benchmark.client_placement: head, which is the error the placement moves in this PR fix.
• Changelog: every GB200/GB300 key in the master configs is listed in the new entry, and the entry lists no unknown keys.
• "Optional" power stays non-fatal on the cluster side: telemetry_arguments never sets telemetry.required, and srtctl defaults it to False. So when exporters fail to start, srtctl keeps serving and the benchmark still runs. Strictness comes only from host-side REQUIRE_POWER validation. This matches the PR's intent.
• Single-node routing: SRT_SINGLE has no legacy POWER_LANES entry. On clusters without power-telemetry, resolve_power returns NO_POWER, so telemetry_arguments(None, …) can't be reached. SingleNodeRequest.conc is a required int, so the [request.conc] path is safe.
• Slurm fetch_outputs returns the shared directory without copying. The agg result and power_validation.json that check_single_node writes therefore survive the later finish_single_node callback.

Non-blocking notes

1. Different require_power sources on the single-node path (collect.py:113 vs collect.py:72). check_single_node passes --require-power from request.require_power. finish_single_node, and the multinode collect(), use power.require_power. The two values are identical today because SRT_SINGLE has no legacy rules. Using power.require_power in both places would stop them drifting apart if a single-node lane is added later.
2. Copying the whole log tree (collect.py:70). shutil.copytree(logs, run.workspace / "LOGS", ...) copies the entire job log tree, server logs included, into the runner workspace. The workflow only uploads LOGS/power. On long AgentX runs this can be large. It also uses symlinks=False, unlike _copy_tree_into (symlinks=True), so symlinks get followed.
   • If POWER_RESULT_ROOT=LOGS only needs power/ plus the result JSON, copying just those would be cheaper.
   • This failure is already non-fatal unless require_power is set, so it isn't a correctness problem.
3. Strictness differs by model under cluster defaults. In resolve_power, the settings branch ORs in the legacy decision.require_power. As a result, gb200 kimik3/glm5.2 AgentX recipes that already opt in to dcgm (the legacy agentx=True rules) stay strict whatever the require-power input is. Newly covered AgentX models (dsv4, qwen3.5, minimaxm3, …) are best-effort. test_existing_strict_agentx_recipe_remains_strict shows this is intended. Saying so in the docs or changelog would help, because power failures will look different across models in the same sweep.
4. Lane policy errors are ignored when settings are present (power.py, except PowerPolicyError). If the cluster has power-telemetry, a lane's error/agentic_error no longer blocks a dcgm-enabled recipe that matches no rule; it falls back to cluster defaults. This matches "without recipe or model allowlists". It does mean those lane error messages for gb200-nv/gb300-nv can no longer be reached and could be removed later.
5. Small cleanups in check_single_node:
   • require(request, "INFERENCEX_RESULTS_PYTHON", "GPU_COUNT") and request.env["GPU_COUNT"]: SingleNodeRequest already requires gpu_count: int, so str(request.gpu_count) is enough.
   • The new --audit-source results/power_validation.json only holds if workspace/results doesn't exist yet when _copy_tree_into(logs/agentic, workspace/results) runs. That copy behaviour existed before this PR; I'm noting it because the audit path is now hard-coded.

Topology and performance

The placement changes alter host contention. Dedicated discovery/client nodes become dedicated frontend heads with etcd/nats and the client on head. The DSV4 trtllm and vllm frontend/client move from decode nodes to head. glm5.2/trtllm changes the other way: frontend goes to dedicated, which srtctl implements as dedicated_node=True plus head, and the client goes to head. All of these end with client, frontend and collector on the head clock, and none change node counts. The changelog says they need fresh results.

Before merge: the PR has no primary sweep label yet, and the hardware sweep (373 matrix + 242 eval entries) depends on #3296. Every perf number for these keys should come from a fresh run on this branch, not from reuse.

[github-actions]

inferencex-e2e/infx diff: +412 -240

Generally most InferenceX submissions don't need to add changes to the infx package. On the off chance they do, generally the diff is small. Please prompt your agent to clean your PR to keep your diff to only necessary & minimal changes too. We have an increased code quality bar for changes to the infx package.

中文

大多数 InferenceX 提交通常不需要修改 infx 包。即使确实需要，改动通常也很小。请让你的 agent 清理 PR，使改动只包含必要且最小的内容。infx 包的改动有更高的代码质量要求。

[edwingao28]

Closing: GLM-5.2 GB200 power moved to #4553; NVL72 default-on collection can be redone once #3296 lands.

[claude]

Findings marked 🟡 are optional suggestions and need no follow-up push.

Additional findings (outside the current diff — GitHub can't attach inline comments there):

• 🟡 inferencex-e2e/infx/srt_slurm/generate.py — Recipes written by infx generate for GB200/GB300 NVL72 fixed-sequence disagg points without power: true no longer match what the launcher submits. The generator skips the power binding at generate.py:110 whenever a non-agentic point lacks power. The launcher's decide_power now turns on DCGM and Grace CPU telemetry for every gb200-nv/gb300-nv SRT_MULTI point. Fix: decide power binding the same way the launcher does, by always calling decide_power for placed multi-node points, so NVL72 points get telemetry, the CPU source and REQUIRE_POWER. [also at: inferencex-e2e/infx/launch/drivers/srt/power.py:170 - On GB200/GB300 multi-node fixed-sequence points without power: true, infx generate output no longer matches what is launched.]

  Why this was flagged

  The trigger is a multi-node fixed-seq-len disagg point on runner gb200/gb300 (cluster gb200-nv or gb300-nv) whose search-space entry has no power: true, such as the dsr1 trtllm 8k1k disagg points. At launch, decide_power (power.py:153-170) sees default true and returns dcgm=True, cpu=True. run_multinode (init.py:193) then binds the recipe with the exporter port, the cpu_power_exporter source and REQUIRE_POWER. In infx generate, _binder_inputs (generate.py:110) returns None, None, {} because point.get("power") is falsy, so decide_power never runs. The written recipe has no telemetry block, no CPU exporter and no REQUIRE_POWER. The module docstring says it writes "the recipes the launcher submits". On the base branch these points got no power at launch either, so both paths agreed. Now the generated recipe silently differs from the submitted one, so recipe review and composed checks built on it no longer cover the telemetry that actually runs.

  Verification: The generator and the launcher now disagree. _binder_inputs in generate.py:110 never calls power.decide_power for these points. In power.py, the if default: branch returns PowerDecision(dcgm=True, ..., cpu=True) whether or not request.power is set. On the base branch, decide_power returned NO_POWER when power was not set, so the generator matched the launcher for these points.