Skip to content

[PowerX] replace client GPU sampling with native telemetry / [PowerX] 用原生遥测替代客户端 GPU 采样 - #3684

Closed
edwingao28 wants to merge 17 commits into
mainfrom
fix/retire-bench-gpu-monitor
Closed

edwingao28 wants to merge 17 commits into
mainfrom
fix/retire-bench-gpu-monitor

Conversation

@edwingao28

@edwingao28 edwingao28 commented Oct 2, 2026 •

Copy link
Copy Markdown
Collaborator

Description

Replace client GPU sampling with native NVIDIA/AMD telemetry for single-node throughput and AgentX. Retain formal results and provenance, finalize AgentX power, and preserve legacy CSV reads.

Testing: CPU regressions, 21 native configuration renders, Ruff and workflow security pass; hardware qualification pending.

Dependency: #3610; AMD producer patch upstreaming remains pending.

中文

以原生 NVIDIA/AMD 遥测替代单节点吞吐量和 AgentX 的客户端 GPU 采样。保留正式结果和来源信息,完成 AgentX 功耗处理,并保留历史 CSV 读取。

测试: CPU 回归测试、21 项原生配置生成检查、Ruff 和 workflow 安全检查通过;硬件验收待完成。

依赖: #3610;AMD producer 补丁尚待提交上游。

GPT-6 用于实现及委派审查,精确变体和早期贡献者模型版本无法核实。

AI model disclosure

  • Model/version: GPT-6; exact variant and earlier contributors’ versions unavailable.
  • Role: Implementation and delegated review.

Related Issue

Stacked on #3610.

Type of Change

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

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 inferencex-e2e/perf-changelog.yaml and have not edited historical entries
  • If this PR can affect benchmark performance or adds or modifies a recipe, 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.

adibarra and others added 7 commits October 2, 2026 09:28
…collector

Split out of the benchmark_lib.sh port (#3610) so the port diff is only
the port. None of these paths is reachable from a checked-in config:

- llm-d: Dockerfile, binaries, recipes and the multi-node job/submit/server
  scripts.
- SWE-bench Lite and Modal: run_swebench_eval and its agentic generation,
  Modal credential handling, scorer, task YAML, runtime patches, workflow
  inputs, secrets, uploads, runtime settings and the threshold.
- native_power_collect.sh and native_power_lifecycle.sh, used only by the
  retired AMD lanes. The adapter that reads LOGS/native_power stays.
…rics patches

- SPEED-Bench: vLLM ships --chat-template-kwargs for client-rendered
  datasets since v0.23.0 (vllm-project/vllm#44244). Default collection to
  vllm/vllm-openai:v0.30.0, note the minimum on the workflow input, and
  remove apply_chat_template_kwargs_shim and the DSpark preflight.
- MiniMax-M3 TRT-LLM AgentX: stop patching PerfMetricsManager off.
  return_perf_metrics, which these recipes need for trtllm-serve's
  Prometheus metrics, also enables per-step timing until TensorRT-LLM
  decouples the two upstream. Both recipes now share
  minimaxm3-trtllm-agentx.sh, which only applies the store=false patch.
…/SWE-bench leftovers

Remove 18 benchmark_lib.sh functions with no callers (_background_process_descendants, _background_process_groups_alive, _background_process_is_running, agentic_kv_offload_enabled, agentic_pip_install, append_command, exit_after_background_process_cleanup, require_agentic_kv_offload_backend, require_agentic_kv_offload_none, rewrite_lm_eval_meta_env, run_amd_multinode_after_preflight, select_available_server_port, setup_eval_context, stop_background_process_groups, stop_background_process_tree, validate_required_agentic_server_metrics, write_agentic_result_json, write_command) along with the one test that exercised select_available_server_port, the llmd-vllm case in multi_node/runtime_settings.sh, and SWE-bench's "resolved" metric family in eval result collection.
Nothing reads metrics_plots.png or workload_distribution_plots.png. Delete
generate_aiperf_plots.py, drop the histogram from
analyze_benchmark_distributions (the ISL/OSL summary stays), and drop
matplotlib from the AgentX client venv.
Delete benchmarks/benchmark_lib.sh and move its behavior into the
container-side Python package infx/bench (stdlib only, Python 3.10):
wait, fixed-seq, agentic and eval commands behind one
`python3 -m infx.bench` entrypoint, with shared process, readiness and
input helpers. srt entry scripts become thin shims; recipe paths are
unchanged. benchmarks/check_env.sh keeps check_env_vars for workflows
and recipe setup scripts. AgentX single-concurrency validation moves
from the workflow into LaunchRequest.

Also drops code only the retired AMD/TileRT lanes used (server_watch,
agentic --server-pid, the monitor command, fixed-seq point mode), the
PROFILE trace relay, and dead topology aliases from the meta_env.json
mapping.
Follows the plot removal in the base PR: no generate_aiperf_plots call and no
matplotlib in the AIPerf client venv.
移除客户端 GPU 功耗采样,改用 SRT 原生测量窗口。
@github-actions

github-actions Bot commented Oct 2, 2026

Copy link
Copy Markdown
Contributor

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 任务通过。临时性失败通常可以通过重新运行恢复;参见重新运行失败任务的说明。

@edwingao28
edwingao28 marked this pull request as ready for review October 2, 2026 20:36
@edwingao28
edwingao28 requested a review from a team as a code owner October 2, 2026 20:36
@claude

claude Bot commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

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


Review summary

  • Gather context
  • Review diff (base: refactor/remove-benchmark-lib)
  • Post findings

🔴 1 blocking issue (inline): srt_single now requires SRT_MEASUREMENT_WINDOW_DIR before every throughput point. No launcher or workflow provides it to single-node jobs yet: infx/srt_slurm/ has no telemetry or power wiring, and nothing sets POWER_ARTIFACT_DIR for single-node lanes. So every single-node SRT fixed-sequence point would exit 1. It also turns broken power telemetry in best-effort mode into a lost benchmark point, where AgentX records invalid power instead. The PR body already lists this as a merge blocker. The inline comment describes how to keep the best-effort behaviour until the launcher work lands.

What I checked and found OK: the _write_json_atomic import moved to common.py, and infx.results.power.window / proc.RelaySignals.run both exist. The single-node POWER_ARTIFACT_DIR branch in fixed_sequence.py uses GPU_COUNT, which benchmark-tmpl.yml exports. The changelog entry is appended at the end of the file with an allowed XXX placeholder. The workflows still set GPU_MONITOR_INTERVAL and upload gpu_metrics* artifacts. That does no harm, but they should be cleaned up in a follow-up, since this app can't edit .github/workflows.

Comment thread inferencex-e2e/infx/bench/fixed_seq.py
将功耗采样清理的变更日志链接更新为 PR 3684。

@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.

Beyond the inline finding, I also checked two related single-node power paths: fixed_seq.py's new unconditional SRT_MEASUREMENT_WINDOW_DIR requirement looks intentional (its own comment states removing the local sampler must not silently downgrade measured points to unmeasured), and the GPU_COUNT lookup in fixed_sequence.py's aggregate_power_result can't actually raise in practice, since srt_slurm/single_node.py already requires GPU_COUNT for any single-node launch before results processing ever runs.

Extended reasoning...

This PR removes client-side GPU power sampling from single- and multi-node benchmark/launch paths (run.py, fixed_seq.py, fixed_sequence.py, single_node.py, plus tests) in favor of SRT measurement windows, with no auth/crypto surface touched. A confirmed inline finding already flags that single-node AgentX jobs now hard-fail when SRT_MEASUREMENT_WINDOW_DIR is unset, which matches the author's own stated "blocker" about single-node lanes not yet being covered. I additionally traced two other candidate issues from the ruled-out list (fixed_seq.py's unconditional env.require, and a potential GPU_COUNT KeyError in aggregate_power_result) and confirmed both are either intentional or unreachable given the launcher's existing GPU_COUNT requirement.

Comment on lines 115 to 118

def _power_mode(enabled: str, multinode: bool, env: Mapping[str, str]) -> PowerMode:
def _power_mode(enabled: str, env: Mapping[str, str]) -> PowerMode:
if enabled not in TRUE_VALUES:
return "off"

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.

🔴 Single-node AgentX jobs with ENABLE_AGENTX_POWER=1 and REQUIRE_POWER=1 now fail the whole run instead of measuring power locally, once a launcher hasn't set SRT_MEASUREMENT_WINDOW_DIR. _power_mode (run.py:118) returns "missing" for any job lacking that var regardless of node count; run.py:204's --multinode-contract-missing path then reaches power_adapter.py's _fail_multinode_adapter, which returns 1 when --require-power is set. On the base branch single-node always got "monitor" mode (local GpuMonitor sampling), which never depended on that var. Fix: don't let REQUIRE_POWER enforcement for single-node reach the hard-fail path until native telemetry/SRT_MEASUREMENT_WINDOW_DIR is actually provisioned for that lane (the PR's own stated blocker).

Why this was flagged

Trigger: a single-node AgentX point with ENABLE_AGENTX_POWER=1 and REQUIRE_POWER=1, launched before the operator's infra sets SRT_MEASUREMENT_WINDOW_DIR, entering via Plan.from_env -> _power_mode (run.py:115-118). That function now returns "missing" there, so _power_audit (run.py:199-204) sends --multinode-contract-missing --require-power to power_adapter.py's main(), whose _fail_multinode_adapter (power_adapter.py:227-229) returns 1, failing the benchmark run entirely. On the base branch this same job got power="monitor" and ran infx.bench.gpu_monitor.GpuMonitor locally, succeeding independent of SRT_MEASUREMENT_WINDOW_DIR. No check in run.py gates REQUIRE_POWER on whether the launcher has actually rolled out native telemetry for the single-node lane; the PR description calls this exact gap a merge blocker but the code enforces it unconditionally.

Verification: Base run.py _power_mode returned "monitor" for any non-multinode job, so single-node AgentX jobs sampled GPUs locally via GpuMonitor regardless of SRT_MEASUREMENT_WINDOW_DIR. The new _power_mode (run.py:116-119) drops the node-count branch and returns "missing" whenever SRT_MEASUREMENT_WINDOW_DIR is unset, which then reaches _fail_multinode_adapter, returning 1 when require_power (power_adapter.py:227-229), propagated as a full run failure.

为单节点 NVIDIA 与 AMD 作业接入原生功耗采集、产物保留和 AgentX 结果处理。
@edwingao28 edwingao28 changed the title [PowerX] retire client-side GPU sampling / [PowerX] 移除客户端 GPU 功耗采样 [PowerX] replace client GPU sampling with native telemetry / [PowerX] 用原生遥测替代客户端 GPU 采样 Oct 4, 2026
edwingao28 and others added 4 commits October 4, 2026 01:26
中文:明确单节点结果收尾函数的名称。
- uv everywhere: shared infx/bench/uv.py; lm-eval, vendor evals, BFCL and the
  AIPerf venv install with uv (BFCL passes --excludes to reuse the image stack).
- fixed-seq: drop the pip3 runtime install and SWEEP_FLAGS so single- and
  multi-node clients send the same policy.
- AgentX: drop inputs the agentx scenario (#3646) owns or nobody varies; keep
  only the two 062126 trace loaders; move venv deps to requirements.txt.
- Tests audited for regression value; perf-changelog entry for the loader pins.
中文:使原生功率遥测适配基准客户端重构。
精简原生功耗遥测的配置说明。
Base automatically changed from refactor/remove-benchmark-lib to main October 6, 2026 16:18
中文:将原生遥测接入当前主分支并解决合并冲突。
中文:同步主分支的 Klaud 更新。
中文:同步主分支后保留 llm-d 的脚本、配方、文档和客户端测试。
cquil11 pushed a commit that referenced this pull request Oct 7, 2026
main removed it in #3675 together with the client-side GPU monitor that fed it; the #3684 merge brought it back with no caller.

中文:删除旧版单节点 gpu_metrics.csv 功耗消费代码;主干已在 #3675 连同喂给它的客户端 GPU 监控一起移除,#3684 合并时被带回且无任何调用方。
edwingao28 added a commit that referenced this pull request Oct 7, 2026
中文:perf-changelog 只保留本 PR 的一条记录,移除沿用自 #3610/#3684/#3685 的旧条目。
@edwingao28

Copy link
Copy Markdown
Collaborator Author

close as superseded by #3781

@edwingao28 edwingao28 closed this Oct 8, 2026
@edwingao28
edwingao28 deleted the fix/retire-bench-gpu-monitor branch October 8, 2026 13:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants