Skip to content

fix: integrate GPU power when energy counter is absent - #1319

Open
davidberenstein1957 wants to merge 1 commit into
masterfrom
fix/gpu-without-energy-counter
Open

fix: integrate GPU power when energy counter is absent#1319
davidberenstein1957 wants to merge 1 commit into
masterfrom
fix/gpu-without-energy-counter

Conversation

@davidberenstein1957

Copy link
Copy Markdown
Collaborator

What

GPUDevice.delta now branches on the backend returning None for the cumulative energy counter. When it does, it computes Power.from_watts(self._get_power_usage()) and Energy.from_power_and_time(...) instead of comparing two identical counter readings, and logs a single warning per device so users know which method produced their numbers.

The per-sample logger.warning in NvidiaGPUDevice._get_total_energy_consumption is demoted to debug, since the once-per-device warning now carries the signal.

Why

On GPUs where NVML does not implement nvmlDeviceGetTotalEnergyConsumption (pre-Volta cards, many virtualised GPUs) the previous code returned last_energy unchanged, yielding energy_delta = 0 and power = 0 for the entire run — a GPU-bound job silently reported only CPU + RAM. The fallback lives in the shared GPUDevice, so the AMD backend (which returns None when no energy accumulator is present) is fixed by the same change.

Power integration is less accurate than the counter, but it is the same approximation the CPU path already uses and is dramatically better than zero.

How verified

tests/test_gpu_nvidia.py::TestGpu::test_gpu_without_energy_counter_falls_back_to_power patches the NVML energy query to raise and asserts 26 W / 29 W and 0.026 / 0.029 kWh over an hour. It fails on master (assert 0.0 == 26) and passes here. tests/test_gpu.py, tests/test_gpu_nvidia.py and tests/test_gpu_amd.py all pass (51 tests).

Closes #1316

🤖 Generated with Claude Code

@codecov

codecov Bot commented Aug 12, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.48%. Comparing base (3ec31a0) to head (1889fda).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1319      +/-   ##
==========================================
+ Coverage   91.43%   91.48%   +0.04%     
==========================================
  Files          49       49              
  Lines        5057     5071      +14     
==========================================
+ Hits         4624     4639      +15     
+ Misses        433      432       -1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@davidberenstein1957
davidberenstein1957 marked this pull request as ready for review August 12, 2026 19:14
@davidberenstein1957
davidberenstein1957 requested a review from a team as a code owner August 12, 2026 19:14
GPUs without nvmlDeviceGetTotalEnergyConsumption now integrate instantaneous power instead of reporting zero, re-baselining cleanly when the counter returns.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@davidberenstein1957
davidberenstein1957 force-pushed the fix/gpu-without-energy-counter branch from 4e36cc8 to 1889fda Compare August 19, 2026 15:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

GPUs without an NVML energy counter report 0 kWh

1 participant