Skip to content

ENH: Auto-Detection of Pressure Conversion Factor#966

Merged
Gui-FernandesBR merged 5 commits into
developfrom
enh/auto-pressure-conversion-factor
Jun 19, 2026
Merged

ENH: Auto-Detection of Pressure Conversion Factor#966
Gui-FernandesBR merged 5 commits into
developfrom
enh/auto-pressure-conversion-factor

Conversation

@MateusStano

Copy link
Copy Markdown
Member

Description

In #955 a pressure unit conversion was added to the set_atmospheric_model. This fixed the pressure units for the new "Forecast" models added in #943. However, currently the default pressure unit is "Pa" when it used to be "hPa", this broke the definition of atmospheric models defined by files (such as reanalysis usage). One example of this breaking can be found in camoes_flight_sim.ipynb

This PR adds the functionality to automatically set the pressure unit based on the File/Model type

Breaking change

  • Yes
  • No

@MateusStano MateusStano requested a review from a team as a code owner June 8, 2026 14:15
@MateusStano MateusStano added Enhancement New feature or request, including adjustments in current codes Environment Enviroment Class related features labels Jun 8, 2026
@MateusStano MateusStano changed the title ENH: auto-detection of pressure conversion factor ENH: Auto-Detection of Pressure Conversion Factor Jun 8, 2026
@codecov

codecov Bot commented Jun 8, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 82.85714% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.11%. Comparing base (9cf3dd4) to head (5951126).
⚠️ Report is 76 commits behind head on develop.

Files with missing lines Patch % Lines
rocketpy/environment/tools.py 42.85% 4 Missing ⚠️
rocketpy/environment/environment.py 92.85% 2 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop     #966      +/-   ##
===========================================
+ Coverage    80.27%   81.11%   +0.84%     
===========================================
  Files          104      113       +9     
  Lines        12769    14582    +1813     
===========================================
+ Hits         10250    11828    +1578     
- Misses        2519     2754     +235     

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

Comment thread rocketpy/environment/environment.py Outdated
Comment thread rocketpy/environment/environment.py Outdated

@Gui-FernandesBR Gui-FernandesBR left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the CHANGELOG.md file must be updated...

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR restores expected pressure-unit handling for file-based atmospheric models by introducing auto-detection of the pressure conversion factor (to Pa) when Environment.set_atmospheric_model() is used with forecast/reanalysis/ensemble datasets.

Changes:

  • Allow pressure_conversion_factor=None and auto-detect the factor from model/dictionary shortcuts and/or NetCDF variable units.
  • Add unit auto-detection in get_pressure_levels_from_file() when the conversion factor is None.
  • Emit a runtime warning if the resulting ground-level pressure is outside a sanity range, guiding users to override the conversion factor.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
rocketpy/environment/tools.py Auto-detect pressure unit conversion from the NetCDF level variable units attribute when no factor is provided.
rocketpy/environment/environment.py Default pressure_conversion_factor to None, implement model/dictionary-based detection with metadata fallback, and warn on implausible ground pressure.

Comment thread rocketpy/environment/environment.py Outdated
Comment thread rocketpy/environment/environment.py
Comment thread rocketpy/environment/environment.py Outdated
@Gui-FernandesBR Gui-FernandesBR force-pushed the enh/auto-pressure-conversion-factor branch from 7d27c90 to b463870 Compare June 19, 2026 01:33

@Gui-FernandesBR Gui-FernandesBR left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Gui-FernandesBR Gui-FernandesBR merged commit de3cf1b into develop Jun 19, 2026
10 checks passed
@Gui-FernandesBR Gui-FernandesBR deleted the enh/auto-pressure-conversion-factor branch June 19, 2026 02:05
Gui-FernandesBR pushed a commit to khushalkottaru/RocketPy that referenced this pull request Jun 19, 2026
Gui-FernandesBR added a commit that referenced this pull request Jul 4, 2026
* MNT: final fixes before next release

Documentation, CHANGELOG and test polish ahead of the next release
(v1.13.0), covering PRs merged since v1.12.0.

CHANGELOG
- Add missing entries: Individual Fins (#818), AIGFS/HRRR forecast
  models (#951), duplicate-controller fix (#949), Valkyrie flight
  example (#967).
- Hygiene: de-duplicate #958/#966, move #974 to Fixed and #1041 to
  Removed only, drop the already-released #914 duplicate, and point the
  logging (#973) and ND-interp (#969) entries at their PRs.
- Backfill #940/#941/#944 (shipped in v1.12.0 code but never logged)
  under the [v1.12.0] section.

Docs
- New exceptions reference page (rocketpy.exceptions) wired into the
  reference index; note UnstableRocketWarning in the rocket stability
  docs (#970).
- tanks.rst: switch examples to radius_function= and add a deprecation
  note (#957).
- forecast.rst: fix the HRRR example (missing code directive + stray
  sentence) (#951).
- airbrakes.rst: document discrete vs continuous controllers
  (sampling_rate=None) (#946).
- rocket_usage.rst: note that Parachute is now an abstract base;
  instantiate HemisphericalParachute (#958).

Tests
- New regression tests: 3D ND-interp NaN outside convex hull (#969),
  abstract Parachute cannot be instantiated (#958), Monte Carlo
  convergence stopping (#922), EnvironmentAnalysis surviving wind API
  (#1041), radial-burn grain geometry over time (#944), RingClusterMotor
  full flight (#924), discrete controller invoked once per node (#949),
  acceleration-based parachute trigger deploys (#911).
- Backfill regression tests for ThrustCurve API timeouts (#940) and
  power_off/on_drag Function objects + _input attributes (#941).


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

Labels

Enhancement New feature or request, including adjustments in current codes Environment Enviroment Class related features

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants