Skip to content

feat: cache and back off Electricity Maps carbon intensity - #1358

Open
davidberenstein1957 wants to merge 2 commits into
masterfrom
feat/intensity-providers
Open

davidberenstein1957 wants to merge 2 commits into
masterfrom
feat/intensity-providers

Conversation

@davidberenstein1957

@davidberenstein1957 davidberenstein1957 commented Aug 12, 2026 •

Copy link
Copy Markdown
Collaborator

Description

Slice 1 of 5 of the pluggable carbon-intensity provider work described in #1354. It fixes two live defects in the current Electricity Maps path without introducing any new abstraction, config key, or output field. get_carbon_intensity(geo, token) -> float is extracted out of get_emissions(), the intensity is cached for 60 seconds keyed by location (lat/lon or countryCode) and token, and after a failure the location+token goes into an exponential cooldown doubling from 30s to a 1-hour ceiling during which no request is issued. Skipped requests raise ElectricityMapsAPICooldownError, logged at debug level, so a bad token produces one error line instead of one per API call. get_emissions() keeps its exact signature and return value.

Related Issue

Refs #1354 (no issue closed by this PR directly)

Motivation and Context

codecarbon/core/electricitymaps_api.py fetched grid carbon intensity on every emissions computation. On a long run that is a lot of HTTP requests for a value the grid publishes hourly at most, and when the token is wrong or the network is down, every one of them is doomed and logs an error — one per measurement tick for the whole run.

How Has This Been Tested?

uv run pytest tests/test_electricitymaps_cache.py tests/test_electricitymaps_api.py \
  tests/test_emissions.py -q
37 passed, 1 skipped

tests/test_electricitymaps_cache.py, all network mocked with responses: cache hit within TTL, a long run bounded to one request, refetch after expiry, per-location keying, no request while in cooldown, cooldown doubling to the ceiling, cooldown reset after success, and cooldown isolation between tokens.

Screenshots (if appropriate):

N/A

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

AI Usage Disclosure

  • 🟥 AI-vibecoded: You cannot explain the logic. Car analogy : the car drive by itself, you are outside it and just tell it where to go.
  • 🟠 AI-generated: Car analogy : the car drive by itself, you are inside and give instructions.
  • ⭐ AI-assisted. Car analogy : you drive the car, AI help you find your way.
  • ♻️ No AI used. Car analogy : you drive the car.

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the docs/how-to/contributing.md document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

Behaviour change, not a pure optimisation

The 60s TTL means measurements inside that window convert energy with the same intensity value rather than a freshly fetched one. The TTL is deliberately shorter than the default api_call_interval × measure_power_secs (~2 minutes), so the cache only collapses the extra calls that tasks and stop() add on top. Documented in docs/how-to/configuration.md.

What is deferred (future slices of #1354)

Slice 2 lifts today's bundled-data branches into StaticProvider unchanged. Slice 3 puts Electricity Maps behind the protocol. Slice 4 adds resolve_intensity() with a fallback chain and the carbon_intensity_providers config key. Slice 5 exposes carbon_intensity_g_co2e_kwh and carbon_intensity_source on EmissionsData. Further out: ENTSO-E and WattTime providers, time-weighted intensity. Also deliberately not in this slice: stale-serve (returning an expired cached value on API error), which should land with the slice-5 is_live reporting.

This PR was opened as a draft pending review of the overall direction in #1354.

@davidberenstein1957

Copy link
Copy Markdown
Collaborator Author

CI fix pushed.

The failing test (test_cumulative_emissions_with_varying_intensity) was written to protect a different contract than the one it appeared to break. Its job is to prove that emissions are accumulated as per-tick deltas (delta_energy x intensity at that tick) rather than recomputed as total_energy x latest_intensity; varying the intensity across three measurements was only the mechanism for telling those two behaviours apart. The 5-minute TTL cache is correct behaviour on its own terms - grid intensity is published hourly at best, and refetching on every measurement tick was the request storm this PR set out to remove - so I kept the caching design exactly as it is and did not change any implementation code.

What changed is the test only: it now patches ELECTRICITYMAPS_CACHE_TTL to a negative value for the duration of the test, so each measurement expires the entry and sees a fresh mocked intensity, and it resets the module-level cache before and after so the global state cannot leak in from or out to other tests. The cumulation assertions (0.1 / 0.3 / 0.6, and the < 0.8 guard against the non-cumulative 0.9) are untouched, and no network calls are made - the responses are mocked as before.

The caching behaviour itself remains covered by tests/test_electricitymaps_cache.py, including TTL expiry, per-location keying and the failure cooldown.

Verified locally: tests/test_emissions_tracker.py, tests/test_electricitymaps_cache.py, tests/test_electricitymaps_api.py and tests/test_emissions.py all pass, and pre-commit is clean on the changed file.

@codecov

codecov Bot commented Aug 12, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 98.24561% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 91.84%. Comparing base (3ec31a0) to head (d9b956f).
⚠️ Report is 41 commits behind head on master.

Files with missing lines Patch % Lines
codecarbon/core/electricitymaps_api.py 98.18% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1358      +/-   ##
==========================================
+ Coverage   91.43%   91.84%   +0.41%     
==========================================
  Files          49       49              
  Lines        5057     5201     +144     
==========================================
+ Hits         4624     4777     +153     
+ Misses        433      424       -9     

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

Comment thread codecarbon/core/electricitymaps_api.py Fixed
Comment thread codecarbon/core/electricitymaps_api.py Fixed
@davidberenstein1957
davidberenstein1957 marked this pull request as ready for review August 13, 2026 05:23
@davidberenstein1957
davidberenstein1957 requested a review from a team as a code owner August 13, 2026 05:23
davidberenstein1957 added a commit that referenced this pull request Aug 16, 2026
`find_green_window` fetched the forecast and then asked /latest for the
current intensity, a second HTTP call whose value only fed a "saves ~X%" line
and the --threshold short-circuit. The forecast's first point is that same
period, so use it and drop the call, the fallback and the try/except with it.

Add --finish-by as the complement to --deadline: --deadline bounds the start,
--finish-by bounds the end and is what most people mean. It is a subtraction,
not a second search path.

The Electricity Maps request extraction this branch used to carry now lives in
its base branch (#1358) where it belongs, so `clear_cooldown` is gone: request()
clears its own location's cooldown on a usable response.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
davidberenstein1957 added a commit that referenced this pull request Aug 16, 2026
`find_green_window` fetched the forecast and then asked /latest for the
current intensity, a second HTTP call whose value only fed a "saves ~X%" line
and the --threshold short-circuit. The forecast's first point is that same
period, so use it and drop the call, the fallback and the try/except with it.

Add --finish-by as the complement to --deadline: --deadline bounds the start,
--finish-by bounds the end and is what most people mean. It is a subtraction,
not a second search path.

The Electricity Maps request extraction this branch used to carry now lives in
its base branch (#1358) where it belongs, so `clear_cooldown` is gone: request()
clears its own location's cooldown on a usable response.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
davidberenstein1957 added a commit that referenced this pull request Aug 19, 2026
`find_green_window` fetched the forecast and then asked /latest for the
current intensity, a second HTTP call whose value only fed a "saves ~X%" line
and the --threshold short-circuit. The forecast's first point is that same
period, so use it and drop the call, the fallback and the try/except with it.

Add --finish-by as the complement to --deadline: --deadline bounds the start,
--finish-by bounds the end and is what most people mean. It is a subtraction,
not a second search path.

The Electricity Maps request extraction this branch used to carry now lives in
its base branch (#1358) where it belongs, so `clear_cooldown` is gone: request()
clears its own location's cooldown on a usable response.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Carbon intensity was fetched from the Electricity Maps API on every emissions
computation, so a long run with a short `measure_power_secs` issued thousands
of requests for a value the grid publishes hourly. A failing token produced
one doomed request per measurement tick for the whole run.

`get_carbon_intensity()` is extracted from `get_emissions()`, its result is
cached for 60 s per location and token, and the API goes into a flat 60 s
cooldown after a failure. `get_emissions()` is unchanged for callers.

A 60 s TTL is deliberate: 5 minutes silently overrode the ~2 minute
`api_call_interval` cadence and halved the intensity resolution. Both the
cache and the cooldown are keyed by location and token, so trackers with
different tokens do not share a value and one tracker's bad token or
unreachable network cannot block another tracker's good one. The token is used
directly as part of the in-process dict key and is never rendered into a log
line. Cooldown raises a dedicated error logged at debug, so a bad token no
longer produces one error line per tick.

Cache and cooldown state are read-modify-written from the background
measurement thread, so they are guarded by one module-level lock, never held
across the HTTP request.

Behaviour change worth calling out: a non-200 whose body is not the expected
JSON error object now surfaces `resp.text` instead of raising a
`JSONDecodeError` (or `ElectricityMapsAPIError(None)` when the body is JSON
without `error`/`message`). Covered by a test on a 502 HTML body.

Refs #1354

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@benoit-cty

Copy link
Copy Markdown
Contributor

🤖 This review comment was written and posted by Claude Opus 5.5 (AI assistant), at the request of @benoit-cty. Findings were checked by reading the code and running tests locally (merged with current master where relevant), but please double-check before acting on them.

Verdict: 🔧 Request changes (small)

Extracting get_carbon_intensity(), caching per (location, token), a cooldown after a failure, and debug-level skip logs are all good ideas. Thread safety under the lock is fine: parallel cache misses only cause a harmless duplicate fetch. 66 tests pass, and the PR merges cleanly with master.

Must fix:

  1. The documented backoff doesn't exist.
    • docs/how-to/configuration.md and the description promise an "exponentially growing cooldown (30 s up to 1 hour)".
    • The code uses a fixed ELECTRICITYMAPS_COOLDOWN = 60 (electricitymaps_api.py:22, :40), and no test covers doubling.
    • Please implement it: start at 30 s, double on each consecutive failure, cap at 3600 s, reset on success, with tests. Or fix the docs. Implementing it is preferable, see point 2.
  2. With default settings, the cache and cooldown almost never trigger.
    • During a run, intensity is fetched about every api_call_interval × measure_power_secs = 8 × 15 = 120 s (emissions_tracker.py:1278-1284). That is longer than both the 60 s TTL and the 60 s cooldown.
    • So the docs' claim that the cache "removes roughly three requests out of four" is wrong. Please correct it.
    • A bad token still logs one error per API call on a normal run. The exponential cooldown from point 1 fixes that.

Low:
3. A 30 s request timeout is still hit once per cooldown window, on the scheduler thread. This interacts with #1339's 5 s join cap in stop(). Consider a shorter timeout for intensity lookups.

Nit:

  • Pre-existing: if geo.latitude: treats latitude 0.0 as missing. Use is not None.

… cache docs

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@davidberenstein1957

Copy link
Copy Markdown
Collaborator Author

Made the changes in d9b956f: cooldown now 30 s doubling to 1 h (reset on success, tested), docs no longer claim 3-in-4 savings, intensity timeout cut to 10 s, and latitude is not None. Kept the 60 s cache TTL as is.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants