Conversation
…#889) * Fix hard-coded radius value for parachute added mass calculation Calculate radius from cd_s using a typical hemispherical parachute drag coefficient (1.4) when radius is not explicitly provided. This fixes drift distance calculations for smaller parachutes like drogues. Formula: R = sqrt(cd_s / (Cd * π)) Closes #860 Co-authored-by: Gui-FernandesBR <63590233+Gui-FernandesBR@users.noreply.github.com> Address code review: improve docstrings and add explicit None defaults Co-authored-by: Gui-FernandesBR <63590233+Gui-FernandesBR@users.noreply.github.com> Add CHANGELOG entry for PR #889 Co-authored-by: Gui-FernandesBR <63590233+Gui-FernandesBR@users.noreply.github.com> Update rocket.add_parachute to use radius=None for consistency Changed the default radius from 1.5 to None in the add_parachute method to match the Parachute class behavior. This ensures consistent automatic radius calculation from cd_s across both APIs. Co-authored-by: Gui-FernandesBR <63590233+Gui-FernandesBR@users.noreply.github.com> Refactor Parachute class to remove hard-coded radius value and introduce drag_coefficient parameter for radius estimation Fix hard-coded radius value for parachute added mass calculation Calculate radius from cd_s using a typical hemispherical parachute drag coefficient (1.4) when radius is not explicitly provided. This fixes drift distance calculations for smaller parachutes like drogues. Formula: R = sqrt(cd_s / (Cd * π)) Closes #860 Co-authored-by: Gui-FernandesBR <63590233+Gui-FernandesBR@users.noreply.github.com> Add CHANGELOG entry for PR #889 Co-authored-by: Gui-FernandesBR <63590233+Gui-FernandesBR@users.noreply.github.com> Refactor Parachute class to remove hard-coded radius value and introduce drag_coefficient parameter for radius estimation MNT: Extract noise initialization to fix pylint too-many-statements in Parachute.__init__ Co-authored-by: Gui-FernandesBR <63590233+Gui-FernandesBR@users.noreply.github.com> * Refactor environment method access in controller test for clarity * fix pylint * fix comments * avoid breaking change with drag_coefficient * reafactors Parachute.__init__ method * fix tests --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: Gui-FernandesBR <63590233+Gui-FernandesBR@users.noreply.github.com> Co-authored-by: Gui-FernandesBR <guilherme_fernandes@usp.br>
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #943 +/- ##
===========================================
+ Coverage 80.27% 81.20% +0.92%
===========================================
Files 104 107 +3
Lines 12769 14039 +1270
===========================================
+ Hits 10250 11400 +1150
- Misses 2519 2639 +120 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…pytest standards (GitHub Copilot) (#937)
…ngelog (#940) * Initial plan * ENH: Add explicit timeouts to ThrustCurve API requests Co-authored-by: MateusStano <69485049+MateusStano@users.noreply.github.com> * DOC: Add timeout fix PR to changelog Co-authored-by: MateusStano <69485049+MateusStano@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: MateusStano <69485049+MateusStano@users.noreply.github.com>
…utes for raw user input and update changelog (#941) * Initial plan * ENH: Restore power_off/on_drag as Function, add _input attributes for raw user input Co-authored-by: MateusStano <69485049+MateusStano@users.noreply.github.com> * DOC: Add PR #941 compatibility fix to changelog Co-authored-by: MateusStano <69485049+MateusStano@users.noreply.github.com> * Update rocketpy/rocket/rocket.py Co-authored-by: Gui-FernandesBR <63590233+Gui-FernandesBR@users.noreply.github.com> * MNT: ruff pylint --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: MateusStano <69485049+MateusStano@users.noreply.github.com> Co-authored-by: Gui-FernandesBR <63590233+Gui-FernandesBR@users.noreply.github.com> Co-authored-by: MateusStano <mateusstano@usp.br>
…or.evaluate_geometry` (#944) * Initial plan * BUG: Fix incorrect Jacobian in only_radial_burn branch of evaluate_geometry Co-authored-by: MateusStano <69485049+MateusStano@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: MateusStano <69485049+MateusStano@users.noreply.github.com>
* chore: added personal toolkit files * update branch name in workflow * chore: update toolkit files * Fix: add wraparound logic for wind direction and related tests * style: fix ruff formatting * Remove unused import Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * refactor: move repetitive logic into helper method * fix: update test logic in test_environment * add changelog entry --------- Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Co-authored-by: Gui-FernandesBR <63590233+Gui-FernandesBR@users.noreply.github.com>
|
I will take a look at this one over the weekend |
There was a problem hiding this comment.
Pull request overview
This PR updates RocketPy’s forecast data ingestion to use UCAR THREDDS OPeNDAP endpoints (fixing NOMADS breakage) and introduces mapping support for both THREDDS-style and legacy NOMADS-style variable naming, while explicitly disabling unavailable latest-model shortcuts (GEFS/HIRESW).
Changes:
- Switch GFS/NAM/RAP latest-model fetchers to UCAR THREDDS “Best” datasets and add projected-grid handling (Lambert Conformal) in the Environment ingestion path.
- Expand
WeatherModelMappingwith explicit*_LEGACYdictionaries and add runtime mapping resolution/fallback when dataset variables don’t match the selected mapping. - Update docs/tests/changelog to reflect THREDDS migration and the temporary unavailability of GEFS/HIRESW shortcuts.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
rocketpy/environment/fetchers.py |
Migrates GFS/NAM/RAP URLs to UCAR THREDDS; adds HRRR/AIGFS fetcher helpers. |
rocketpy/environment/tools.py |
Adds Lambert Conformal projection conversion and rewrites index-finding helpers to work directly with netCDF variables (incl. descending grids). |
rocketpy/environment/weather_model_mapping.py |
Adds THREDDS-first mappings plus explicit *_LEGACY aliases and case-insensitive lookup. |
rocketpy/environment/environment.py |
Adds mapping compatibility checks/fallback, projected-grid support for x/y datasets, and disables GEFS/HIRESW shortcuts with explicit errors. |
tests/unit/environment/test_environment.py |
Adds unit tests for dictionary resolution behavior and legacy alias lookup. |
tests/integration/environment/test_environment.py |
Updates integration tests to expect errors for disabled GEFS/HIRESW shortcuts. |
docs/user/environment/3-further/other_apis.rst |
Documents built-in/legacy mapping names and projected-grid projection usage. |
docs/user/environment/1-atm-models/standard_atmosphere.rst |
Adds an explicit Sphinx label and minor formatting tweak. |
docs/user/environment/1-atm-models/soundings.rst |
Removes RUC-specific workflow guidance and redirects users to supported alternatives. |
docs/user/environment/1-atm-models/forecast.rst |
Updates guidance around remote OPeNDAP usage and marks HIRESW shortcut as unavailable. |
docs/user/environment/1-atm-models/ensemble.rst |
Adds Sphinx label and marks GEFS shortcut as unavailable with guidance for explicit datasets. |
CHANGELOG.md |
Adds an Unreleased “Fixed” entry for the THREDDS migration. |
Comments suppressed due to low confidence (1)
rocketpy/environment/environment.py:2109
- Ensemble datasets still require a valid
dictionary["ensemble"]mapping when the file actually has an ensemble dimension: if the mapping omits it (or__resolve_dictionary_for_datasetfalls back to a non-ensemble mapping), the code later fails ininverse_dictionary[dim]when buildingparams. Instead of treating missingensembleas “single member”, detect whether the dataset dimensions include an ensemble axis and raise a clear ValueError (or infer the ensemble dimension name) when the mapping doesn’t define it.
# Get ensemble data from file
has_ensemble_dimension = True
try:
num_members = len(data.variables[dictionary["ensemble"]][:])
except KeyError:
has_ensemble_dimension = False
num_members = 1
# Get pressure level data from file
levels = get_pressure_levels_from_file(data, dictionary)
inverse_dictionary = {v: k for k, v in dictionary.items()}
param_dictionary = {
"time": time_index,
"ensemble": range(num_members),
"level": range(len(levels)),
"latitude": (lat_index - 1, lat_index),
"longitude": (lon_index - 1, lon_index),
}
# Get dimensions
try:
dimensions = data.variables[dictionary["geopotential_height"]].dimensions[:]
except KeyError:
dimensions = data.variables[dictionary["geopotential"]].dimensions[:]
# Get params
params = tuple(param_dictionary[inverse_dictionary[dim]] for dim in dimensions)
| Mapping families | ||
| ---------------- | ||
| - Base names (for example ``GFS``, ``NAM``, ``RAP``) represent the current | ||
| default mappings used by the latest-model shortcuts and THREDDS-style | ||
| datasets. | ||
| - ``*_LEGACY`` names represent older NOMADS-style variable naming | ||
| conventions (for example ``lev``, ``tmpprs``, ``ugrdprs`` and | ||
| ``vgrdprs``) and are intended for archived or previously downloaded files. | ||
|
|
||
| Notes | ||
| ----- | ||
| - Mappings can also include optional keys such as ``projection`` for | ||
| projected grids and ``ensemble`` for member dimensions. | ||
| - The :meth:`get` method is case-insensitive, so ``"gfs_legacy"`` and | ||
| ``"GFS_LEGACY"`` are equivalent. |
There was a problem hiding this comment.
The class docstring indentation under “Mapping families” / “Notes” is inconsistent with the surrounding lines, which will likely render as a literal block when Sphinx/inspect dedents the docstring. Align those section headings/bullets to the same indentation level as the rest of the docstring text.
| Mapping families | |
| ---------------- | |
| - Base names (for example ``GFS``, ``NAM``, ``RAP``) represent the current | |
| default mappings used by the latest-model shortcuts and THREDDS-style | |
| datasets. | |
| - ``*_LEGACY`` names represent older NOMADS-style variable naming | |
| conventions (for example ``lev``, ``tmpprs``, ``ugrdprs`` and | |
| ``vgrdprs``) and are intended for archived or previously downloaded files. | |
| Notes | |
| ----- | |
| - Mappings can also include optional keys such as ``projection`` for | |
| projected grids and ``ensemble`` for member dimensions. | |
| - The :meth:`get` method is case-insensitive, so ``"gfs_legacy"`` and | |
| ``"GFS_LEGACY"`` are equivalent. | |
| Mapping families | |
| ---------------- | |
| - Base names (for example ``GFS``, ``NAM``, ``RAP``) represent the current | |
| default mappings used by the latest-model shortcuts and THREDDS-style | |
| datasets. | |
| - ``*_LEGACY`` names represent older NOMADS-style variable naming | |
| conventions (for example ``lev``, ``tmpprs``, ``ugrdprs`` and | |
| ``vgrdprs``) and are intended for archived or previously downloaded files. | |
| Notes | |
| ----- | |
| - Mappings can also include optional keys such as ``projection`` for | |
| projected grids and ``ensemble`` for member dimensions. | |
| - The :meth:`get` method is case-insensitive, so ``"gfs_legacy"`` and | |
| ``"GFS_LEGACY"`` are equivalent. |
* ENH: added a new function (simulate_convergence) * DOC: added a cell to show simulate_convergence function usage * TST: integration test for simulate_convergence * DOC: updated changelog for this PR * ENH: ran black to lint intg test file * new fixes thx to copilot comments * linted rocketpy/simulation/monte_carlo.py --------- Co-authored-by: Malmahrouqi3 <mohdsaid497566@gmail.com>
Co-authored-by: Gui-FernandesBR <63590233+Gui-FernandesBR@users.noreply.github.com>
Co-authored-by: Gui-FernandesBR <63590233+Gui-FernandesBR@users.noreply.github.com>
Co-authored-by: Gui-FernandesBR <63590233+Gui-FernandesBR@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…RocketPy-Team/RocketPy into bug/deactivation-opendap-nomads
The base branch was changed.
Summary
Solving #933
We are now using OpenDAP with UCAR THREDDS:
The following models are not available on THREDDS and were deactivated
Particularly GEFS is very important for Monte Carlo simulations and we should look for alternatives as fast as possible
Observations
The Thredds OpenDap approach took me a long time to find information about. So in between I tried out other methods which will still be important for us to understand for the future (probably for re-implementing GEFS):
THREDDS actually has an archival version of GEFS that goes up to 2020. I think we can find other alternatives, but this could be a last resort option.
Useful Links