ENH: Improve parachute geometric parametrization #835
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR improves the parachute geometric parametrization by introducing new parameters (radius, height, and porosity) to provide a more flexible and accurate simulation. Key changes include:
- Adding lambda callbacks to update parachute_radius, parachute_height, and parachute_porosity in the simulation.
- Updating the added mass computation in flight dynamics to use parachute_radius and parachute_height.
- Extending the Parachute class to support the new parameters, with appropriate defaults and serialization.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| rocketpy/simulation/flight.py | Updated simulation callbacks and mass computation with new parameters. |
| rocketpy/rocket/parachute.py | Extended the Parachute class to include and serialize new parameters. |
Comments suppressed due to low confidence (2)
rocketpy/simulation/flight.py:2002
- [nitpick] Consider adding a comment to explain the rationale behind the modified added mass formula using parachute_radius squared times parachute_height, clarifying the geometrical model or assumptions used.
ma = ka * rho * (4 / 3) * np.pi * parachute_radius**2 * parachute_height
rocketpy/simulation/flight.py:2018
- Ensure that the variable 'z' is defined in this scope (for example, by extracting altitude from the state vector) to avoid potential runtime errors.
az = (Dz - mp * self.env.gravity.get_value_opt(z)) / (mp + ma)
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
rocketpy/rocket/rocket.py
Outdated
| porosity : float, optional | ||
| Porosity of the parachute material, which is a measure of how much air can | ||
| pass through the parachute material. | ||
| Default value is 0.0432 (for consistency with previous versions). |
There was a problem hiding this comment.
Could you define more precisely what porosity is? Something like: "The ratio of the area of the porous structure to the total area of the canopy" (I don't know if this is the correct definition in our case, just an example)
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #835 +/- ##
===========================================
+ Coverage 80.02% 80.05% +0.03%
===========================================
Files 98 98
Lines 12004 12026 +22
===========================================
+ Hits 9606 9628 +22
Misses 2398 2398 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull Request Overview
This PR enhances the parachute model by introducing geometric parameters (radius, height) and porosity, updating simulation calculations accordingly, and aligning tests and documentation.
- Added
parachute_radius,parachute_height, andporositytoParachuteandadd_parachuteAPI - Updated
u_dot_parachuteto compute added mass and gravity using the new parameters - Updated tests and user guides to include the new parameters
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| rocketpy/rocket/parachute.py | Added radius, height, and porosity attributes & serialization |
| rocketpy/rocket/rocket.py | Extended add_parachute signature and docstrings |
| rocketpy/simulation/flight.py | Incorporated geometry and porosity into mass & force calculations |
| tests/unit/test_utilities.py | Updated expected safety_factor for infinite Mach |
| tests/unit/test_flight.py | Updated expected final time in aerodynamic test |
| docs/user/rocket/rocket_usage.rst | Added new parameters to usage examples |
| docs/user/first_simulation.rst | Added new parameters to example |
| README.md | Added new parameters to README example |
Comments suppressed due to low confidence (1)
rocketpy/simulation/flight.py:1999
- The new parachute_radius, parachute_height, and porosity parameters are not explicitly covered by existing tests. Consider adding unit tests that verify how changes in these parameters affect added mass and drag force calculations in
u_dot_parachute.
porosity = self.parachute_porosity
| # Calculate added mass | ||
| ma = ka * rho * (4 / 3) * np.pi * R**3 | ||
| ma = ka * rho * (4 / 3) * np.pi * parachute_radius**2 * parachute_height |
There was a problem hiding this comment.
The added mass calculation uses the full ellipsoid volume (4/3·π·a²·c) but the parachute is modeled as a hemispheroid; this should use half the volume (2/3·π·a²·c) to correctly compute added mass.
| ma = ka * rho * (4 / 3) * np.pi * parachute_radius**2 * parachute_height | |
| ma = ka * rho * (2 / 3) * np.pi * parachute_radius**2 * parachute_height |
Copilot uses AI. Check for mistakes.
Pull request type
Checklist
black rocketpy/ tests/) has passed locallypytest tests -m slow --runslow) have passed locallyCHANGELOG.mdhas been updated (if relevant)Current behavior
Parachute radius is assumed to be fixed (=1.5m)
New behavior
Other than adding a radius parameter, there is also additional height and porosity parameters.
The default is still set to the current behavior.
Breaking change