Skip to content

ENH: cache downloaded atmosphere netCDF datasets (#654) - #1137

Open
thatrandomasiandev wants to merge 2 commits into
RocketPy-Team:developfrom
thatrandomasiandev:enh/654-cache-netcdf
Open

ENH: cache downloaded atmosphere netCDF datasets (#654)#1137
thatrandomasiandev wants to merge 2 commits into
RocketPy-Team:developfrom
thatrandomasiandev:enh/654-cache-netcdf

Conversation

@thatrandomasiandev

Copy link
Copy Markdown

Summary

  • Cache Forecast/Ensemble (and Reanalysis remote URLs) location-time profiles as compact .nc files under ~/.rocketpy_cache/atmosphere (override with ROCKETPY_CACHE).
  • Cache Windy JSON responses on the same path; second identical set_atmospheric_model calls reuse disk.
  • Add no_cache=False to Environment.set_atmospheric_model (and Windy processing) so callers can force a fresh download without breaking existing callers.

Notes / TODOs

OPeNDAP "Best" aggregations are virtual catalogs, not downloadable files, so this PR caches the extracted profiles RocketPy already materializes after the first fetch (same end-user outcome: reruns skip the network). Remaining follow-ups:

  • Open-Meteo / Meteomatics JSON response caching
  • Optional full-file download cache when a remote source is a concrete HTTP .nc (non-OPeNDAP)

Fixes #654

Test plan

  • PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 pytest tests/unit/environment/test_atmosphere_cache.py
  • Confirm Forecast shortcut reuses disk on rerun with same lat/lon/date
  • Confirm no_cache=True forces re-download

@thatrandomasiandev
thatrandomasiandev requested a review from a team as a code owner August 11, 2026 02:02
@Gui-FernandesBR Gui-FernandesBR linked an issue Aug 12, 2026 that may be closed by this pull request
Collapse the cache-key assignment the formatter wants on one line.
No behaviour change.

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

Copy link
Copy Markdown
Member

Not ready to merge yet. Three items, the third being the one I care most about.

1. Three of this PR's own new tests fail on all six platforms.

FAILED tests/unit/environment/test_environment.py::test_set_atmospheric_model_normalizes_shortcut_case_for_forecast
FAILED tests/unit/environment/test_environment.py::test_forecast_shortcut_and_dictionary_are_case_insensitive[AIGFS]
FAILED tests/unit/environment/test_environment.py::test_forecast_shortcut_and_dictionary_are_case_insensitive[HRRR]

IndexError: too many indices for array: array is 0-dimensional, but 2 were indexed
  rocketpy/environment/environment.py:1637

2. pylint fails (exit 8):

rocketpy/environment/atmosphere_cache.py:158:0: R0915: Too many statements (30/25)
rocketpy/environment/atmosphere_cache.py:270:0: R0915: Too many statements (35/25)
rocketpy/environment/environment.py:1237:4: R0912: Too many branches (28/25)

Please split those into smaller functions rather than adding # pylint: disable.

3. Cache invalidation — the design point I want resolved before this lands.

The cache is enabled by default, and the key contains neither a TTL nor the forecast model run/cycle. So a user who runs a GFS forecast today and again tomorrow silently gets yesterday's forecast, with no indication anything is stale. For forecast products that is a wrong-answer bug, not a stale-file annoyance, and it is the kind of thing that is very hard for a user to notice.

What I would want:

  • the model run / cycle timestamp as part of the cache key, or a TTL, so a new cycle can never be served from an old file;
  • caching off by default until we have confidence in the invalidation;
  • and it is worth distinguishing product classes: reanalysis/archive data (ERA5 and friends) is immutable and can be cached indefinitely, forecasts cannot.

Happy to revisit once those are addressed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ENH: Cache netcdf4 datasets to avoid multiple downloads

2 participants