Repository navigation
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #481 +/- ##
=======================================
Coverage ? 84.67%
=======================================
Files ? 156
Lines ? 22628
Branches ? 0
=======================================
Hits ? 19161
Misses ? 3467
Partials ? 0 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Review — verified the DCGM claim against source, ran the suitesVerdict: looks right; two things to tighten before un-drafting, one open question. Verified
Tighten
Open question
Nits
|
…verified; review follow-ups Address the first review on NVIDIA#481: - cpu_rails: collapse DCGM_FIELD_NAMES / DCGM_FIELD_HWMON_LABELS / DCGM_FIELD_RAIL_KINDS into one DcgmPowerField(field_id, name, hwmon_label, kind) record per field (DCGM_POWER_FIELDS, DCGM_FIELD_BY_ID) so the three facts cannot drift; the derived dicts consumers use are built from it. Document why 1131 (envelope *cap*) and 1133 (Module Power, scope on GB200/GB300 unverified on live hardware) are deliberately excluded; the docs table now lists 1133 as unverified/excluded and gives the dcgmi + hwmon commands that would settle it. - cpu_parser: state that the new parser is order-independent and that the exporter's "1130 first per socket" emission order exists only for older srtctl parsers, so it is not removed as dead ordering later. - test_power_energy_report: explain why the rail-only warning test loads through the CSV (CpuSamples.sources comes from the `source` column; direct fixtures leave it empty). - test_cpu_power: a DCGM value for an entity we never enumerated is dropped (covers the guard Codecov flagged). - test_cpu_rails: pin the record table, 1130-first order, and the 1131/1133 exclusions. Report JSON for 430661 / 3054237 still identical to the pre-change output except the warning; 132 power tests, 14 cargo tests, ruff clean, ty count unchanged.
|
Thanks — addressed in e283ec9. Done
Deferred
Verification unchanged: report JSON for 430661 and 3054237 identical to the pre-change output except the warning; 132 power tests + 14 cargo tests pass, ruff clean, |
aee52f9 to
00e92e2
Compare
…verified; review follow-ups Address the first review on NVIDIA#481: - cpu_rails: collapse DCGM_FIELD_NAMES / DCGM_FIELD_HWMON_LABELS / DCGM_FIELD_RAIL_KINDS into one DcgmPowerField(field_id, name, hwmon_label, kind) record per field (DCGM_POWER_FIELDS, DCGM_FIELD_BY_ID) so the three facts cannot drift; the derived dicts consumers use are built from it. Document why 1131 (envelope *cap*) and 1133 (Module Power, scope on GB200/GB300 unverified on live hardware) are deliberately excluded; the docs table now lists 1133 as unverified/excluded and gives the dcgmi + hwmon commands that would settle it. - cpu_parser: state that the new parser is order-independent and that the exporter's "1130 first per socket" emission order exists only for older srtctl parsers, so it is not removed as dead ordering later. - test_power_energy_report: explain why the rail-only warning test loads through the CSV (CpuSamples.sources comes from the `source` column; direct fixtures leave it empty). - test_cpu_power: a DCGM value for an entity we never enumerated is dropped (covers the guard Codecov flagged). - test_cpu_rails: pin the record table, 1130-first order, and the 1131/1133 exclusions. Report JSON for 430661 / 3054237 still identical to the pre-change output except the warning; 132 power tests, 14 cargo tests, ruff clean, ty count unchanged.
…verified; review follow-ups Address the first review on NVIDIA#481: - cpu_rails: collapse DCGM_FIELD_NAMES / DCGM_FIELD_HWMON_LABELS / DCGM_FIELD_RAIL_KINDS into one DcgmPowerField(field_id, name, hwmon_label, kind) record per field (DCGM_POWER_FIELDS, DCGM_FIELD_BY_ID) so the three facts cannot drift; the derived dicts consumers use are built from it. Document why 1131 (envelope *cap*) and 1133 (Module Power, scope on GB200/GB300 unverified on live hardware) are deliberately excluded; the docs table now lists 1133 as unverified/excluded and gives the dcgmi + hwmon commands that would settle it. - cpu_parser: state that the new parser is order-independent and that the exporter's "1130 first per socket" emission order exists only for older srtctl parsers, so it is not removed as dead ordering later. - test_power_energy_report: explain why the rail-only warning test loads through the CSV (CpuSamples.sources comes from the `source` column; direct fixtures leave it empty). - test_cpu_power: a DCGM value for an entity we never enumerated is dropped (covers the guard Codecov flagged). - test_cpu_rails: pin the record table, 1130-first order, and the 1131/1133 exclusions. Report JSON for 430661 / 3054237 still identical to the pre-change output except the warning; 132 power tests, 14 cargo tests, ruff clean, ty count unchanged.
00e92e2 to
28551f5
Compare
…verified; review follow-ups Address the first review on NVIDIA#481: - cpu_rails: collapse DCGM_FIELD_NAMES / DCGM_FIELD_HWMON_LABELS / DCGM_FIELD_RAIL_KINDS into one DcgmPowerField(field_id, name, hwmon_label, kind) record per field (DCGM_POWER_FIELDS, DCGM_FIELD_BY_ID) so the three facts cannot drift; the derived dicts consumers use are built from it. Document why 1131 (envelope *cap*) and 1133 (Module Power, scope on GB200/GB300 unverified on live hardware) are deliberately excluded; the docs table now lists 1133 as unverified/excluded and gives the dcgmi + hwmon commands that would settle it. - cpu_parser: state that the new parser is order-independent and that the exporter's "1130 first per socket" emission order exists only for older srtctl parsers, so it is not removed as dead ordering later. - test_power_energy_report: explain why the rail-only warning test loads through the CSV (CpuSamples.sources comes from the `source` column; direct fixtures leave it empty). - test_cpu_power: a DCGM value for an entity we never enumerated is dropped (covers the guard Codecov flagged). - test_cpu_rails: pin the record table, 1130-first order, and the 1131/1133 exclusions. Report JSON for 430661 / 3054237 still identical to the pre-change output except the warning; 132 power tests, 14 cargo tests, ruff clean, ty count unchanged.
…d ACPI or a partial DCGM field set Two regressions the review of NVIDIA#481 found in the new ACPI-first `auto`: 1. ACPI was trusted on discovery alone. A node that exposes power_meter channels which all read zero would be committed to ACPI and publish zeros for the whole run -- 0 J that looks like a measurement. Now every discovered sensor is read, and if none reports a finite positive value the probe is retried once after 1 s (hwmon averages can be 0 on the first poll after boot); only then does auto fall back to DCGM, at WARN with the sensor count. `--source acpi` bails on all-zero, matching its existing strictness. This cannot rescue a node via DCGM in practice -- DCGM's sysmon reads the same power1_average files, so it sees the same zero -- but it stops zero pollution and names the problem in the exporter log. 2. Watching [1130, 1132] in one field group made DCGM mode all-or-nothing. A libdcgm that rejects 1132 for CPU entities (older release, or a sysmon without SysIO) failed where 1130 alone had always worked, and under ACPI-first auto DCGM is the last resort, so that meant no CPU power at all. `DcgmReader::new` now tries [1130, 1132] then [1130]; the fallback is logged at WARN with DCGM's reason, `DcgmReader::fields()` reports what is actually watched, and read_watts sizes its buffer from that. Tests: probe dead when every sensor reads 0; live when any reads positive; retry rescues a sensor that goes 0 -> value between passes; field-set candidates all start with 1130 and end at [1130]. 18 cargo tests pass.
77416ee to
c34294b
Compare
…verified; review follow-ups Address the first review on NVIDIA#481: - cpu_rails: collapse DCGM_FIELD_NAMES / DCGM_FIELD_HWMON_LABELS / DCGM_FIELD_RAIL_KINDS into one DcgmPowerField(field_id, name, hwmon_label, kind) record per field (DCGM_POWER_FIELDS, DCGM_FIELD_BY_ID) so the three facts cannot drift; the derived dicts consumers use are built from it. Document why 1131 (envelope *cap*) and 1133 (Module Power, scope on GB200/GB300 unverified on live hardware) are deliberately excluded; the docs table now lists 1133 as unverified/excluded and gives the dcgmi + hwmon commands that would settle it. - cpu_parser: state that the new parser is order-independent and that the exporter's "1130 first per socket" emission order exists only for older srtctl parsers, so it is not removed as dead ordering later. - test_power_energy_report: explain why the rail-only warning test loads through the CSV (CpuSamples.sources comes from the `source` column; direct fixtures leave it empty). - test_cpu_power: a DCGM value for an entity we never enumerated is dropped (covers the guard Codecov flagged). - test_cpu_rails: pin the record table, 1130-first order, and the 1131/1133 exclusions. Report JSON for 430661 / 3054237 still identical to the pre-change output except the warning; 132 power tests, 14 cargo tests, ruff clean, ty count unchanged.
…d ACPI or a partial DCGM field set Two regressions the review of NVIDIA#481 found in the new ACPI-first `auto`: 1. ACPI was trusted on discovery alone. A node that exposes power_meter channels which all read zero would be committed to ACPI and publish zeros for the whole run -- 0 J that looks like a measurement. Now every discovered sensor is read, and if none reports a finite positive value the probe is retried once after 1 s (hwmon averages can be 0 on the first poll after boot); only then does auto fall back to DCGM, at WARN with the sensor count. `--source acpi` bails on all-zero, matching its existing strictness. This cannot rescue a node via DCGM in practice -- DCGM's sysmon reads the same power1_average files, so it sees the same zero -- but it stops zero pollution and names the problem in the exporter log. 2. Watching [1130, 1132] in one field group made DCGM mode all-or-nothing. A libdcgm that rejects 1132 for CPU entities (older release, or a sysmon without SysIO) failed where 1130 alone had always worked, and under ACPI-first auto DCGM is the last resort, so that meant no CPU power at all. `DcgmReader::new` now tries [1130, 1132] then [1130]; the fallback is logged at WARN with DCGM's reason, `DcgmReader::fields()` reports what is actually watched, and read_watts sizes its buffer from that. Tests: probe dead when every sensor reads 0; live when any reads positive; retry rescues a sensor that goes 0 -> value between passes; field-set candidates all start with 1130 and end at [1130]. 18 cargo tests pass.
8d8bc9d to
4066fd1
Compare
edwingao28
left a comment
There was a problem hiding this comment.
the pr looks great i left some comments^
| let want_dcgm = matches!(args.source, SourceMode::Dcgm | SourceMode::Auto); | ||
| let want_acpi = matches!(args.source, SourceMode::Acpi | SourceMode::Auto); | ||
|
|
||
| // ACPI first: it is the only source with the socket envelope. DCGM reads |
There was a problem hiding this comment.
Could we add a test around init_metrics_state? This changes the default from DCGM first to ACPI first, but the Rust tests only cover the ACPI probe finds a lisensor and still pass if auto tried DCGM first or --source acpi fell back instead of returning an error
There was a problem hiding this comment.
Ah I see -- I wasn't honoring the contract that was established. If a specific backend is specified, then we throw an error instead of trying a fallback. This should be fixed now.
| fn probe_acpi_live(sensors: &[Sensor], retries: u32, delay: Duration) -> Option<usize> { | ||
| for attempt in 0..=retries { |
There was a problem hiding this comment.
Here should we check the socket totals before choosing ACPI? If the Grace total is stuck at 0 W but the CPU rail is live, this probe could still accepts ACPI and we record 0 W for that socket. The Python probe_live seem has the same issue
There was a problem hiding this comment.
So I think the issue here is that even if this returns 0, we go with it because DCGM can't provide anything more if even ACPI (which is a primary source) doesn't report it. Some breakdowns from DCGM show that it doesn't reference the Grace total power and instead only reports the power cap but never the actual average power. Here's a breakdown from Claude:
From NVIDIA/DCGM modules/sysmon/DcgmSystemMonitor.cpp @ 64df9f8:
| hwmon label | DCGM opens | stored in | getter → field |
|---|---|---|---|
CPU Power Socket N |
power1_average (L49–53, L165–168) |
m_cpuSocketToPowerUsagePath |
GetCurrentCPUPowerUsage → 1130 DCGM_FI_DEV_CPU_POWER_WATTS |
SysIO Power Socket N |
power1_average (L61–65, L169–172) |
m_sysioSocketToPowerUsagePath |
GetCurrentSysIOPowerUsage → 1132 DCGM_FI_DEV_SYSIO_POWER_UTIL_CURRENT |
Module Power Socket N |
power1_average (L55–59, L173–176) |
m_moduleSocketToPowerUsagePath |
GetCurrentModulePowerUsage → 1133 DCGM_FI_DEV_MODULE_POWER_UTIL_CURRENT (scope on GB200/GB300 unverified; excluded from our table) |
Grace Power Socket N (the socket envelope) |
power1_cap only (L44 GRACE_CAP_SOCKET_BEGINNING, L67–71, L183–187) |
m_socketToPowerCapPath — no *UsagePath sibling (.h L124–127) |
GetCurrentPowerCap (L289–295) → 1131 DCGM_FI_DEV_CPU_POWER_LIMIT_WATTS |
Any other label is dropped at L73–77 (log_debug("Ignoring file contents: ...")). No DCGM field reads the Grace device's power1_average, so the socket envelope's draw is never collected — 1131 is its configured limit.
|
@edwingao28 -- thanks for the review, mind taking another look? |
…verified; review follow-ups Address the first review on NVIDIA#481: - cpu_rails: collapse DCGM_FIELD_NAMES / DCGM_FIELD_HWMON_LABELS / DCGM_FIELD_RAIL_KINDS into one DcgmPowerField(field_id, name, hwmon_label, kind) record per field (DCGM_POWER_FIELDS, DCGM_FIELD_BY_ID) so the three facts cannot drift; the derived dicts consumers use are built from it. Document why 1131 (envelope *cap*) and 1133 (Module Power, scope on GB200/GB300 unverified on live hardware) are deliberately excluded; the docs table now lists 1133 as unverified/excluded and gives the dcgmi + hwmon commands that would settle it. - cpu_parser: state that the new parser is order-independent and that the exporter's "1130 first per socket" emission order exists only for older srtctl parsers, so it is not removed as dead ordering later. - test_power_energy_report: explain why the rail-only warning test loads through the CSV (CpuSamples.sources comes from the `source` column; direct fixtures leave it empty). - test_cpu_power: a DCGM value for an entity we never enumerated is dropped (covers the guard Codecov flagged). - test_cpu_rails: pin the record table, 1130-first order, and the 1131/1133 exclusions. Report JSON for 430661 / 3054237 still identical to the pre-change output except the warning; 132 power tests, 14 cargo tests, ruff clean, ty count unchanged.
…d ACPI or a partial DCGM field set Two regressions the review of NVIDIA#481 found in the new ACPI-first `auto`: 1. ACPI was trusted on discovery alone. A node that exposes power_meter channels which all read zero would be committed to ACPI and publish zeros for the whole run -- 0 J that looks like a measurement. Now every discovered sensor is read, and if none reports a finite positive value the probe is retried once after 1 s (hwmon averages can be 0 on the first poll after boot); only then does auto fall back to DCGM, at WARN with the sensor count. `--source acpi` bails on all-zero, matching its existing strictness. This cannot rescue a node via DCGM in practice -- DCGM's sysmon reads the same power1_average files, so it sees the same zero -- but it stops zero pollution and names the problem in the exporter log. 2. Watching [1130, 1132] in one field group made DCGM mode all-or-nothing. A libdcgm that rejects 1132 for CPU entities (older release, or a sysmon without SysIO) failed where 1130 alone had always worked, and under ACPI-first auto DCGM is the last resort, so that meant no CPU power at all. `DcgmReader::new` now tries [1130, 1132] then [1130]; the fallback is logged at WARN with DCGM's reason, `DcgmReader::fields()` reports what is actually watched, and read_watts sizes its buffer from that. Tests: probe dead when every sensor reads 0; live when any reads positive; retry rescues a sensor that goes 0 -> value between passes; field-set candidates all start with 1130 and end at [1130]. 18 cargo tests pass.
4cdeb38 to
c599ad1
Compare
…verified; review follow-ups Address the first review on NVIDIA#481: - cpu_rails: collapse DCGM_FIELD_NAMES / DCGM_FIELD_HWMON_LABELS / DCGM_FIELD_RAIL_KINDS into one DcgmPowerField(field_id, name, hwmon_label, kind) record per field (DCGM_POWER_FIELDS, DCGM_FIELD_BY_ID) so the three facts cannot drift; the derived dicts consumers use are built from it. Document why 1131 (envelope *cap*) and 1133 (Module Power, scope on GB200/GB300 unverified on live hardware) are deliberately excluded; the docs table now lists 1133 as unverified/excluded and gives the dcgmi + hwmon commands that would settle it. - cpu_parser: state that the new parser is order-independent and that the exporter's "1130 first per socket" emission order exists only for older srtctl parsers, so it is not removed as dead ordering later. - test_power_energy_report: explain why the rail-only warning test loads through the CSV (CpuSamples.sources comes from the `source` column; direct fixtures leave it empty). - test_cpu_power: a DCGM value for an entity we never enumerated is dropped (covers the guard Codecov flagged). - test_cpu_rails: pin the record table, 1130-first order, and the 1131/1133 exclusions. Report JSON for 430661 / 3054237 still identical to the pre-change output except the warning; 132 power tests, 14 cargo tests, ruff clean, ty count unchanged.
…d ACPI or a partial DCGM field set Two regressions the review of NVIDIA#481 found in the new ACPI-first `auto`: 1. ACPI was trusted on discovery alone. A node that exposes power_meter channels which all read zero would be committed to ACPI and publish zeros for the whole run -- 0 J that looks like a measurement. Now every discovered sensor is read, and if none reports a finite positive value the probe is retried once after 1 s (hwmon averages can be 0 on the first poll after boot); only then does auto fall back to DCGM, at WARN with the sensor count. `--source acpi` bails on all-zero, matching its existing strictness. This cannot rescue a node via DCGM in practice -- DCGM's sysmon reads the same power1_average files, so it sees the same zero -- but it stops zero pollution and names the problem in the exporter log. 2. Watching [1130, 1132] in one field group made DCGM mode all-or-nothing. A libdcgm that rejects 1132 for CPU entities (older release, or a sysmon without SysIO) failed where 1130 alone had always worked, and under ACPI-first auto DCGM is the last resort, so that meant no CPU power at all. `DcgmReader::new` now tries [1130, 1132] then [1130]; the fallback is logged at WARN with DCGM's reason, `DcgmReader::fields()` reports what is actually watched, and read_watts sizes its buffer from that. Tests: probe dead when every sensor reads 0; live when any reads positive; retry rescues a sensor that goes 0 -> value between passes; field-set candidates all start with 1130 and end at [1130]. 18 cargo tests pass.
c599ad1 to
3695e45
Compare
|
|
…breakdown, prefer ACPI in auto
DCGM has no CPU power backend of its own. Its sysmon module reads the ACPI
power_meter hwmon channels by power1_oem_info label, so each CPU-entity
field is one ACPI rail (NVIDIA/DCGM modules/sysmon/DcgmSystemMonitor.cpp,
DcgmModuleSysmon.cpp):
1130 DCGM_FI_DEV_CPU_POWER_WATTS <- "CPU Power Socket N" = cpu_rail
1132 DCGM_FI_DEV_SYSIO_POWER_UTIL_CURRENT <- "SysIO Power Socket N" = soc
1131 DCGM_FI_DEV_CPU_POWER_LIMIT_WATTS <- "Grace Power Socket N" cap only
No field reports the Grace envelope's draw. DCGM-mode power_w (field 1130)
is therefore the CPU rail, ~53 W/socket against a ~100 W ACPI envelope on
the GB200 reference runs, and has been silently under-reporting CPU power
relative to ACPI-mode runs.
Rail vocabulary (cpu_rails):
- DCGM_FIELD_RAIL_KINDS {1130: cpu_rail, 1132: soc}, DCGM_PRIMARY_FIELD_ID,
field names and hwmon labels, in the one module that owns rail names.
- cpu_sample.dcgm_rail_readings(): the one place field ids become rails.
1130 yields the dcgm primary (power_w, unchanged) and the cpu_rail
component; 1132 yields soc. Host reader and scrape parser both call it.
Producers:
- Python host collector watches 1130+1132, fills cpu_rail_w/soc_w, and
records a power_fields table (field, rail, hwmon label, column) in the
node metadata.
- Rust cpu-power-exporter watches both fields and publishes one
cpu_power_dcgm_watts{socket,field_id} sample per field, 1130 first so a
legacy collector still meets it first. `--source auto` now tries ACPI
first and falls back to DCGM, matching the Python collector; DCGM-first
is how run 430661 lost the socket envelope its nodes could provide.
- cpu_parser reads the field_id label; an unlabelled sample (older
exporter) is 1130, so old binaries keep working and now also get
cpu_rail_w filled.
Report:
- power_energy_report: CpuSamples.sources; DCGM-sourced runs get a
"CPU power source is DCGM field 1130 = CPU rail only ... not comparable
with ACPI-mode runs" warning. Numbers are unchanged: report JSON for
samples 430661 (DCGM) and 3054237 (ACPI) is identical to the pre-change
output except for that warning.
Docs: cpu-power-telemetry.md (field -> rail table, auto order, column
semantics), config-reference.md, dcgm-4.7-runtime-support.md, schema
docstring; stale "blank for DCGM" comments corrected.
Tests: cpu_rails/cpu_sample/cpu_parser/cpu_power/collector/energy-report
fixtures updated to the new contract plus new cases (labelled body,
missing 1130, garbage field_id, rail-only warning on/off); Rust
render_dcgm_metrics unit tests.
…verified; review follow-ups Address the first review on NVIDIA#481: - cpu_rails: collapse DCGM_FIELD_NAMES / DCGM_FIELD_HWMON_LABELS / DCGM_FIELD_RAIL_KINDS into one DcgmPowerField(field_id, name, hwmon_label, kind) record per field (DCGM_POWER_FIELDS, DCGM_FIELD_BY_ID) so the three facts cannot drift; the derived dicts consumers use are built from it. Document why 1131 (envelope *cap*) and 1133 (Module Power, scope on GB200/GB300 unverified on live hardware) are deliberately excluded; the docs table now lists 1133 as unverified/excluded and gives the dcgmi + hwmon commands that would settle it. - cpu_parser: state that the new parser is order-independent and that the exporter's "1130 first per socket" emission order exists only for older srtctl parsers, so it is not removed as dead ordering later. - test_power_energy_report: explain why the rail-only warning test loads through the CSV (CpuSamples.sources comes from the `source` column; direct fixtures leave it empty). - test_cpu_power: a DCGM value for an entity we never enumerated is dropped (covers the guard Codecov flagged). - test_cpu_rails: pin the record table, 1130-first order, and the 1131/1133 exclusions. Report JSON for 430661 / 3054237 still identical to the pre-change output except the warning; 132 power tests, 14 cargo tests, ruff clean, ty count unchanged.
…wer samples exist
`_build_power_energy_report` logged every PowerReportError at DEBUG, which
is right when telemetry is off (no power CSVs, the routine case) but hides
the real findings: a run that collected CPU/GPU power and still has no
power_energy_report.json. Auditing the sample runs, two such gaps were only
diagnosable by re-running the report offline:
3090377 profile_export_aiperf.json: not found (benchmark died, exit=1,
before aiperf wrote its summary; no profiling window)
648928 cpu/nvl72d020-T08/socket0: window narrower than the sample spacing
Now, when any samples.csv exists below the log dir, the skip is logged at
INFO as "Power energy report skipped (power samples present): <reason>";
without power samples it stays at DEBUG. Post-processing still never fails
the benchmark.
Tests: the quiet path pins DEBUG; a new case builds the 3090377 shape
(profiling records, no aiperf summary, a CPU CSV) and asserts the INFO line
names the missing file.
…d ACPI or a partial DCGM field set Two regressions the review of NVIDIA#481 found in the new ACPI-first `auto`: 1. ACPI was trusted on discovery alone. A node that exposes power_meter channels which all read zero would be committed to ACPI and publish zeros for the whole run -- 0 J that looks like a measurement. Now every discovered sensor is read, and if none reports a finite positive value the probe is retried once after 1 s (hwmon averages can be 0 on the first poll after boot); only then does auto fall back to DCGM, at WARN with the sensor count. `--source acpi` bails on all-zero, matching its existing strictness. This cannot rescue a node via DCGM in practice -- DCGM's sysmon reads the same power1_average files, so it sees the same zero -- but it stops zero pollution and names the problem in the exporter log. 2. Watching [1130, 1132] in one field group made DCGM mode all-or-nothing. A libdcgm that rejects 1132 for CPU entities (older release, or a sysmon without SysIO) failed where 1130 alone had always worked, and under ACPI-first auto DCGM is the last resort, so that meant no CPU power at all. `DcgmReader::new` now tries [1130, 1132] then [1130]; the fallback is logged at WARN with DCGM's reason, `DcgmReader::fields()` reports what is actually watched, and read_watts sizes its buffer from that. Tests: probe dead when every sensor reads 0; live when any reads positive; retry rescues a sensor that goes 0 -> value between passes; field-set candidates all start with 1130 and end at [1130]. 18 cargo tests pass.
…rter; document it
The Python host collector (`srtctl.core.cpu_power`, the telemetry.cpu_power
leg) had the two gaps the exporter fixed in the previous commit:
- `create_reader("auto")` committed to ACPI on discovery alone. It now
requires at least one sensor to read a positive value, retrying once after
1 s (`AcpiPowerMeterReader.probe_live`), and steps down to DCGM at WARN with
the sensor count otherwise. `--source acpi` fails on dead sensors instead.
- `DcgmCpuPowerReader` watched [1130, 1132] in one field group and failed
outright if libdcgm refused the set. It now tries the candidates in
`DCGM_FIELD_SET_CANDIDATES` ([1130, 1132], then [1130]), cleaning up the
rejected field group, and logs at WARN when it lands on the rail alone.
`power_field_ids` reports what is watched; `read_watts` accepts only those
fields; `_sensor_kinds()` expects only those kinds; metadata gains
`power_fields[].watched` so a per-node file shows which rung it landed on.
The collector runs as a standalone srun process, so `main()` installs a
stderr logging handler; the ladder's INFO/WARN lines land in the node's
telemetry_cpu_power.<node>.out.
Docs: cpu-power-telemetry.md spells out the four-rung ladder and what a
missing reading looks like in the CSV -- a component rail that cannot be read
is blank, never 0, for every cause (fallback to 1130-only, non-OK status,
zero/non-finite); a socket missing its primary yields no row rather than a
blank power_w; rails never substitute for the primary or join
total_power_w. config-reference.md and the schema docstrings say the same in
one line; schema-reference.md regenerated.
Tests (8 new): DCGM fallback to 1130 when 1132 is refused (cleanup, WARN, no
soc kind expected, metadata watched flags); fail when every set is refused;
ACPI probe dead / live / retry rescues; auto steps dead ACPI -> DCGM with the
count in the log; live ACPI never constructs DCGM; explicit acpi fails on
dead sensors. 258 power/postprocess/docs tests pass.
docs/cpu-power-telemetry.md promises every step down the auto ladder is logged at WARN, but the 'no ACPI sensors found' and 'hwmon discovery error' fallbacks in init_metrics_state used info!, so a node landing on DCGM's half-size CPU rail number could do so without a warning in its .out file. The dead-sensor fallback was already WARN; the Python host collector already warns on every step. Logging change only. Signed-off-by: Frank Di Natale <3429989+FrankD412@users.noreply.github.com>
…a-reference.md The source field's description was an attribute docstring (a bare string after the field), which schema_docs never reads -- it takes an Attributes: block in the class docstring or a # comment above the field -- so docs/schema-reference.md showed an empty description for cpu_power_exporter.source. Moved into an Attributes: block (adding port, which had none either) and regenerated with srtctl schema-docs. No behaviour change. Signed-off-by: Frank Di Natale <3429989+FrankD412@users.noreply.github.com>
… a live socket total Every reader accepted 0 W as a sample (>= 0 in the Rust exporter's read_acpi_watts, the Python host collector's read_watts, and both scrape parser families), so a Grace envelope stuck at 0 produced a power_w=0 row and a total_power_w that summed it in -- integrating to 0 J and passing for a measurement -- while docs/cpu-power-telemetry.md promised zero is dropped. The startup probe compounded it: any positive sensor made ACPI "live", so a dead envelope beside a live CPU rail was accepted for the whole run. power1_average reports 0 only when the sensor has produced no reading (a live Grace socket draws tens of watts), so 0 is now filed as missing at every read site, and liveness means at least one socket-total channel reads positive -- that is the rail that becomes power_w. With 0 treated as missing, the existing pivot and aggregate rules do the rest: a socket with a stuck total gets no row, and the node total is blank rather than partial. A node with rails but no live total steps down to DCGM in auto (WARN) or exits under --source acpi; the Python ACPI reader already refused to construct without a total channel. Falling back to DCGM for a dead envelope was considered and rejected as the fix: DCGM's sysmon reads the same hwmon files, so it cannot recover the envelope -- it would only re-read the CPU rail via libdcgm and label it. Tests: zero total is missing / socket dropped / node total blank (host collector), probe ignores live rails with no live total and counts only totals (Python + Rust), zero total and zero 1130 in scrape bodies produce no row (parser), zero rail omitted from the exporter body (Rust). All fail on the previous source. Signed-off-by: Frank Di Natale <3429989+FrankD412@users.noreply.github.com>
…ontract init_metrics_state mixed the ACPI decision with DcgmReader::new(), which needs libdcgm, so nothing proved that auto is ACPI-first or that --source acpi errors instead of stepping down: the probe tests passed regardless of ladder order. The ACPI leg is now decide_acpi(mode, root, probe) -> Use | FallBack(reason) | Skip, with the liveness probe injected, and init_metrics_state only reaches DCGM on FallBack/Skip. While there, discovery now rejects a sensor set with no socket-total channel before probing, naming the domains it did find -- the Python collector already refused to construct in that case; the exporter used to run the 1 s probe retry and report "no total read positive" instead. Eight tests: auto uses live ACPI; auto falls back (with the reason) on no sensors / no total channel / dead total / unreadable root; acpi errors on each of those; acpi uses live sensors; dcgm never runs the probe. A mutation that makes acpi fall back like auto fails explicit_acpi_errors_instead_of_falling_back. 30 cargo tests pass. Signed-off-by: Frank Di Natale <3429989+FrankD412@users.noreply.github.com>
…w semantics The docs restructure on main (NVIDIA#565/NVIDIA#566) moved the telemetry.cpu_power_exporter prose from config-reference.md to observability.md. Carry this branch's two edits to it there: the auto ladder (live ACPI socket total, then DCGM 1130+1132, then 1130 alone) and the statement that DCGM-mode power_w is field 1130 = the CPU rail, about half the ACPI envelope, flagged by the energy report.
3695e45 to
ffbe74a
Compare
Summary
DCGM-mode CPU power has been reporting the CPU rail, not the socket envelope — about half of what ACPI-mode reports for the same hardware (~53 W vs ~100 W per socket on GB200 reference runs). This PR makes that explicit in the data, adds the SysIO rail DCGM also exposes, and stops
automode from picking DCGM over the fuller ACPI source.Follows #453 (one row per socket, rail names in one module). Independent of #475 (Pareto/HTML report), which already renders the
warningsthis PR adds.Why (verified against NVIDIA/DCGM source)
DCGM has no CPU power backend of its own. The sysmon module walks
/sys/class/hwmon/*/device/power1_oem_info, matches the label prefix, and readspower1_averagefrom that channel — i.e. it is a second consumer of the same ACPIpower_meterinterface our ACPI reader uses. Pinned tomaster@64df9f8:DCGM_FI_DEV_CPU_POWER_WATTS(def, getter)CPU Power Socket N→power1_average(L49-53, L165-168)cpu_railDCGM_FI_DEV_SYSIO_POWER_UTIL_CURRENT(def)SysIO Power Socket N→power1_averagesocDCGM_FI_DEV_CPU_POWER_LIMIT_WATTSGrace Power Socket N→power1_caponly (L67-71, L183-186)No DCGM field reports the
Grace Power Socket Nenvelope's average. Real samples (per-socket means over the whole run):CPU Power(cpu_rail)Grace Power(envelope,power_w)CPU Power/Grace Powerpower_w)What changes
Vocabulary —
cpu_rails.DCGM_FIELD_RAIL_KINDS = {1130: cpu_rail, 1132: soc}(+DCGM_PRIMARY_FIELD_ID, field names, hwmon labels) in the one module that owns rail names.cpu_sample.dcgm_rail_readings()is the one place field ids become rails: 1130 → thedcgmprimary (power_w, unchanged) and thecpu_railcomponent; 1132 →soc. Host reader and scrape parser both call it.Producers
srtctl.core.cpu_power): watches 1130 + 1132; wide CSV getscpu_rail_w= 1130 andsoc_w= 1132; node metadata gains apower_fieldstable (field id, name, rail, hwmon label, column).cpu-power-exporter: watches[1130, 1132], falling back to[1130]alone (WARN, with DCGM's reason) if this libdcgm refuses the pair for CPU entities — 1130 alone is the floor every earlier exporter had./metricsemits onecpu_power_dcgm_watts{socket,field_id,source}sample per field, 1130 first so a legacy collector still meets it first.--source autois now ACPI-first (was DCGM-first, unlike the Python collector), and the Python host collector (srtctl.core.cpu_power) walks the same ladder. ACPI is not trusted on discovery alone: every sensor is read, and if all read zero the probe is retried once after 1 s before falling back to DCGM at WARN — a node whosepower_meterchannels never report would otherwise publish zeros that integrate to 0 J and pass for a measurement.--source acpibails on all-zero instead of falling back.cpu_parser: reads thefield_idlabel; an unlabelled sample (older exporter binary) is 1130, so old binaries keep working and now also getcpu_rail_wfilled.Report —
CpuSamples.sources; a DCGM-sourced run gets the warningCPU power source is DCGM field 1130 = CPU rail only (not the socket envelope; roughly half of ACPI-mode CPU power): not comparable with ACPI-mode runs.Docs —
cpu-power-telemetry.md(field→rail table,autoorder, column semantics),config-reference.md,dcgm-4.7-runtime-support.md,CpuPowerExporterConfigdocstring; stale "rails blank for DCGM" comments corrected.Data correctness
power_w/total_power_wsemantics are unchanged for every mode: 1130 stays the DCGM-mode socket power (SysIO is never summed in), ACPI is untouched. Summing 1130+1132 intopower_wwas considered and rejected — the ACPI envelope is ~100 W vs cpu_rail+soc ≈ 59 W, so the sum would still be wrong while looking less wrong.power_energy_report --json-outfor 430661 (DCGM) and 3054237 (ACPI) is identical to the pre-change output except for the one new warning on 430661.socket 0: power_w=50.149, rails={cpu_rail: 50.149, soc: 6.2},total_power_w=96.914— the same total the run recorded.Compatibility:
power_w=1130,cpu_rail_w=1130,soc_w=1132power_w=1130,cpu_rail_w=1130,soc_wblankfield_id, dedups by socket)Testing
uv run pyteston the power suites: 482 passed (cpu_rails / cpu_sample / cpu_parser / cpu_power / collector / contract / samples / energy report / validator / artifacts / telemetry). New cases: field-labelled body, body without 1130 (no socket published), garbagefield_id, rail-only warning present for DCGM and absent for ACPI, host reader watching both fields with a socket missing 1132.cargo test -p cpu-power-exporter: 18 passed (render_dcgm_metricsordering/labels,POWER_FIELDSpin, field-set candidates all start with 1130 and end at[1130], ACPI probe dead / live / retry-rescues-a-late-first-sample).cargo fmt --checkclean.tests/test_cpu_power.py, 8 new): DCGM falls back to 1130 when 1132 is refused (rejected field group cleaned up, WARN,sockind not expected,power_fields[].watchedin metadata); fails when every set is refused; ACPI probe dead / live / retry-rescues;autosteps dead ACPI → DCGM with the count logged; live ACPI never constructs DCGM; explicitacpifails on dead sensors.make lint: ruff clean;tydiagnostic count unchanged (38, pre-existing).Also in this PR
postprocess_stage._build_power_energy_reportlogging — the skip reason was DEBUG-only, so a run with power CSVs and nopower_energy_report.jsonleft no trace in the sweep log. Auditing the sample runs, two such gaps (3090377: benchmark died before aiperf wroteprofile_export_aiperf.json; 648928: window narrower than the sample spacing) were only diagnosable by re-running the report offline. When anysamples.csvexists the skip is now INFO with the reason; telemetry-off stays DEBUG. Two tests pin both paths.What a missing reading looks like
A component rail that cannot be read is blank, never
0, for every cause: the exporter/collector fell back to 1130 alone (blank on every row, one WARN at startup), DCGM returned a non-OK status for that (socket, field) on that scrape, or the value was zero/non-finite (dropped —0from these files means "not measured"). A socket missing its primary (ACPItotalor field 1130) yields no row at all rather than a blankpower_w. Rails never substitute for the primary and never jointotal_power_w, so a missingsoc_wchanges nothing else. Documented indocs/cpu-power-telemetry.md→ Output Format.Not verified on hardware
Everything above is verified against DCGM source, real sample CSVs, and unit tests. Nothing in this PR has run on a live Grace node. Specifically unconfirmed:
soc_wis left empty; nothing else depends on it).Module Power Socket N) is not the Grace envelope — it has a usage getter in DCGM, so the "no DCGM field reports the envelope" claim is only strictly true with 1133 ruled out. It is deliberately excluded from the table until measured (cpu_rails.py).dcgmEntitiesGetLatestValuesbuffer, and the[1130,1132]→[1130]field-set fallback, exercise a real libdcgm. Onlyrender_dcgm_metricsand the candidate table are unit-testable.autoon a node whose hwmon actually carries the envelope.One visit to a GB200 node settles all four:
dcgmi dmon -e 1130,1131,1132,1133 -i cpu:0 -c 3alongsidecat /sys/class/hwmon/*/device/power1_oem_info. Output will be pasted here.The 430661 explanation (ACPI discovery found nothing, so the old DCGM-first
autolanded on DCGM) is inferred from the flow, not observed — its exporter log only shows the DCGM connection. #524 (legacyhwmon*/device/nameregistration) is the likely actual cause and is the right fix for discovery; this PR makes the fallback honest and preferred-against, #524 removes the reason for it.Notes for review
field_idlabel on the existing metric (rather than a new metric family) was chosen for wire compactness; an old collector sees two samples per socket and relies on 1130 being emitted first, which the exporter guarantees and a unit test pins. The new parser itself is order-independent (cpu_parser.pysays so), so the ordering exists only for old binaries.power1_averagefiles, so it sees the same zero. Its value is refusing to publish zero pollution and naming the problem in the exporter log.DCGM_CPU_RAIL_ONLY_WARNINGis appended per concurrency point; a rollup-level warning when a combined report mixes ACPI and DCGM runs belongs in feat(power-report): interactive Pareto / power-over-time HTML report (AIP-1388) #475'sbuild_combined_reportand is tracked on AIP-1397.Tracking: AIP-1397 (https://linear.app/nvidia/issue/AIP-1397)