ENH: Improve multi-variable drag compatibility, regular-grid handling, and related tests/docs#927
ENH: Improve multi-variable drag compatibility, regular-grid handling, and related tests/docs#927MateusStano wants to merge 15 commits intodevelopfrom
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #927 +/- ##
===========================================
+ Coverage 80.27% 81.09% +0.82%
===========================================
Files 104 107 +3
Lines 12769 13857 +1088
===========================================
+ Hits 10250 11238 +988
- Misses 2519 2619 +100 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
any reasons to delete these two test files?
There was a problem hiding this comment.
The new tests are covering what these were covering I believe
rocketpy/rocket/rocket.py
Outdated
| "Function, or callable." | ||
| ) | ||
|
|
||
| def __load_csv(self, file_path, coeff_name): |
There was a problem hiding this comment.
why do we need a load_csv function inside the Rocket class? is it extremelly necessary? It seems like a tools function
There was a problem hiding this comment.
Pull request overview
This pull request enhances multi-variable drag coefficient support and regular-grid interpolation handling in RocketPy. Following PR #875, it standardizes drag interfaces to support both 1D (Mach-only) and 7D (alpha, beta, mach, reynolds, pitch_rate, yaw_rate, roll_rate) drag functions, refines grid-based interpolation, and updates related tests and documentation.
Changes:
- Introduced a unified 7D drag coefficient interface (
power_*_drag_7d) that wraps all drag inputs (scalars, 1D Mach functions, or full 7D functions) - Refactored
Functionclass to handle regular-grid interpolation consistently with other interpolation methods (removing deprecatedfrom_gridclass method) - Updated flight dynamics to compute and pass all 7 aerodynamic parameters (alpha, beta, Mach, Reynolds, pitch/yaw/roll rates) to drag functions
- Added comprehensive test coverage for various drag input types and regular-grid interpolation
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
rocketpy/rocket/rocket.py |
Added __process_drag_input and __load_csv methods to normalize all drag inputs to 7D interface; created power_*_drag_by_mach convenience wrappers |
rocketpy/simulation/flight.py |
Replaced __get_drag_coefficient with __compute_drag_7d_inputs; updated all flight dynamics methods to use 7D drag interface |
rocketpy/mathutils/function.py |
Integrated regular-grid processing into standard __validate_source flow; removed from_grid class method and __get_value_opt_grid |
rocketpy/rocket/aero_surface/generic_surface.py |
Updated CSV loading to support column-order independence and prefer regular-grid interpolation when applicable |
rocketpy/plots/rocket_plots.py |
Updated drag curve plotting to use power_*_drag_by_mach attributes |
tests/unit/rocket/test_rocket.py |
Added comprehensive tests for drag input type handling and CSV column ordering |
tests/unit/mathutils/test_function.py |
Added tests for regular-grid constructor, interpolation, and extrapolation |
tests/integration/simulation/test_flight_3dof.py |
Added test verifying 7D drag interface usage in 3-DOF simulations |
docs/user/rocket/generic_surface.rst |
Updated documentation to reflect column-order independence for CSV files |
docs/user/function.rst |
Updated interpolation method documentation to include regular_grid |
Co-authored-by: Gui-FernandesBR <63590233+Gui-FernandesBR@users.noreply.github.com>
…etPy-Team/RocketPy into enh/multi-variable-drag-fix
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…etPy-Team/RocketPy into enh/multi-variable-drag-fix
| power_off_drag = self.power_off_drag_7d | ||
| power_on_drag = self.power_on_drag_7d |
There was a problem hiding this comment.
@phmbressan Should this be discretized? Can we discretize N-D Functions?
There was a problem hiding this comment.
If it has merely 10 point in each dimension, it will have 10^7 total points. So no, don't discretize unless your domain has very few relevant points. I am working on a PR to handle ND Discretization, but don't wait for me.
phmbressan
left a comment
phmbressan
left a comment
There was a problem hiding this comment.
For some reason, I had to publish a review for my answer to another comment from this PR not to be pending.
| nozzle_position=parameters.get("distance_rocket_nozzle")[0], | ||
| ) | ||
| # rocket information | ||
| BellaLui = Rocket( |
There was a problem hiding this comment.
PascalCase variable name? Probably should be snake_case.
| power_off_drag = self.power_off_drag_7d | ||
| power_on_drag = self.power_on_drag_7d |
There was a problem hiding this comment.
If it has merely 10 point in each dimension, it will have 10^7 total points. So no, don't discretize unless your domain has very few relevant points. I am working on a PR to handle ND Discretization, but don't wait for me.
3 participants
Pull request type
Checklist
black rocketpy/ tests/) has passed locallypytest tests -m slow --runslow) have passed locallyCHANGELOG.mdhas been updated (if relevant)Description
This PR follows up on #875, and improves how multi-variable drag data is handled, and preserves backward compatibility. It also aligns interpolation documentation and updates tests to reflect the current drag/function behavior. Changes:
Breaking change