Skip to content

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

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

ENH: cache downloaded atmosphere netCDF datasets (#654)#1137
thatrandomasiandev wants to merge 5 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.

Gui-FernandesBR and others added 3 commits September 12, 2026 03:37
The cache restored only the eight profile Functions, elevation and
_max_expected_height, so every attribute Environment derives from the
dataset was missing on the second run of an otherwise identical script.
atmospheric_model_type was still set to "Forecast", so info() and
all_info() walked into the Forecast branch of the prints and raised
AttributeError on atmospheric_model_init_date, and to_dict() serialized
those fields as None. Persist the full metadata (date range, grid
bounds, and the raw interpolation inputs) alongside the profiles and
reinstate it on a hit, bumping the cache format to v2.

Saving also indexed temperature and both wind profiles as 2-D arrays
while guarding only pressure, which raised IndexError for a constant
wind and broke three existing tests. Guarding pressure alone is not
sufficient either: np.asarray(None, dtype=float) yields nan, so a
missing column would be written out as an entry full of NaN winds.
Check every column against the pressure grid before writing.

Also:

- Expire forecast entries after ROCKETPY_CACHE_TTL seconds (default 6h,
  the GFS cycle). A launch date days out would otherwise pin the first
  forecast ever downloaded for it and silently reuse it forever.
  Reanalysis is immutable and never expires; a TTL of 0 disables expiry.
- Let ROCKETPY_CACHE=0/off/false/no/none/disabled turn the cache off.
  It previously only relocated the directory, so there was no way to opt
  out globally, and an empty value cached into the working directory.
- Include the variable dictionary and pressure conversion factor in the
  cache key, so the same source decoded two ways no longer collides.
- Add clear_atmosphere_cache() to remove the entries.
- Treat a damaged cache file as a miss rather than letting netCDF4's
  RuntimeError escape and take down the simulation with it.
- Pass usedforsecurity=False to hashlib.md5 so the key still builds
  under a FIPS-enabled Python.
- Isolate ROCKETPY_CACHE per test via an autouse fixture. The suite was
  writing to the real ~/.rocketpy_cache, which let one test read
  profiles cached by an earlier one.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The lint job installs pylint unpinned and has not run green on develop
since 2026-07-19. Pylint 4.x reports these four disables as
useless-suppression, which fails the job with exit code 8 on every open
PR regardless of what the PR changes -- two of them already appear in
this PR's first CI run from 2026-08-15.

These files are untouched by the caching work; this commit only removes
the stale comments so the lint job can go green. It is separate so it
can be dropped and landed on its own if maintainers prefer.

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

codecov Bot commented Sep 12, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 74.44717% with 104 lines in your changes missing coverage. Please review.
✅ Project coverage is 91.37%. Comparing base (0923d8a) to head (021ff13).

Files with missing lines Patch % Lines
rocketpy/environment/atmosphere_cache.py 74.33% 77 Missing ⚠️
rocketpy/environment/environment.py 74.28% 27 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #1137      +/-   ##
===========================================
- Coverage    91.51%   91.37%   -0.15%     
===========================================
  Files          131      132       +1     
  Lines        17733    18131     +398     
===========================================
+ Hits         16229    16567     +338     
- Misses        1504     1564      +60     

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

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