Skip to content

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

Merged
Gui-FernandesBR merged 6 commits into
RocketPy-Team:developfrom
thatrandomasiandev:enh/654-cache-netcdf
Sep 14, 2026
Merged

ENH: cache downloaded atmosphere netCDF datasets (#654)#1137
Gui-FernandesBR merged 6 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 76.52812% with 96 lines in your changes missing coverage. Please review.
✅ Project coverage is 91.41%. Comparing base (0923d8a) to head (81406da).

Files with missing lines Patch % Lines
rocketpy/environment/atmosphere_cache.py 77.15% 69 Missing ⚠️
rocketpy/environment/environment.py 74.28% 27 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #1137      +/-   ##
===========================================
- Coverage    91.51%   91.41%   -0.10%     
===========================================
  Files          131      132       +1     
  Lines        17733    18133     +400     
===========================================
+ Hits         16229    16577     +348     
- Misses        1504     1556      +52     

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

…ples

The five `except Exception` handlers swallowed anything, so a NameError
or any other real defect in this module would have been reported to the
user as an innocuous cache miss and a re-download.

Probed netCDF4 1.7.4 for what it actually raises in these paths: a
damaged, empty, truncated, missing or unwritable file gives OSError (or
one of its subclasses); a missing variable KeyError; a missing or
non-numeric attribute AttributeError; a wrong-length array or undeclared
dimension ValueError; an invalid dtype or attribute type TypeError; and
touching a closed dataset RuntimeError. RuntimeError is also kept on the
open path because netCDF4 surfaces some HDF5-level failures that way and
the installed version is not pinned.

Those become two documented module constants: CACHE_OPEN_ERRORS for
_open_valid_cache, whose only risky call is the open itself, and
CACHE_FILE_ERRORS for the read and write bodies, which also touch the
file's contents.

Tests pin both halves of the contract: each of the six error types still
degrades to a cache miss, while an unexpected exception now propagates
from the open path and from the read path instead of being silenced.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@Gui-FernandesBR
Gui-FernandesBR merged commit 88aed5a into RocketPy-Team:develop Sep 14, 2026
9 checks passed
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