ENH: Improve parachute geometric parametrization #835

ArthurJWH · 2025-06-26T01:04:18Z

Pull request type

Code changes (bugfix, features)

Checklist

Tests for the changes have been added (if needed)
Docs have been reviewed and added / updated
Lint (black rocketpy/ tests/) has passed locally
All tests (pytest tests -m slow --runslow) have passed locally
CHANGELOG.md has 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

Yes
No

Copilot

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)

MateusStano · 2025-07-04T18:26:20Z

rocketpy/rocket/rocket.py

+        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).


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)

…zation

codecov · 2025-07-15T11:58:29Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 80.03%. Comparing base (f17893b) to head (65530fd).
⚠️ Report is 18 commits behind head on develop.

Additional details and impacted files

@@             Coverage Diff             @@
##           develop     #835      +/-   ##
===========================================
+ Coverage    80.02%   80.03%   +0.01%     
===========================================
  Files           98      103       +5     
  Lines        12004    12531     +527     
===========================================
+ Hits          9606    10029     +423     
- Misses        2398     2502     +104

☔ View full report in Codecov by Sentry.
📢 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.

Copilot

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, and porosity to Parachute and add_parachute API
Updated u_dot_parachute to 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

Copilot · 2025-07-16T01:09:55Z

rocketpy/simulation/flight.py

        # Calculate added mass
-        ma = ka * rho * (4 / 3) * np.pi * R**3
+        ma = ka * rho * (4 / 3) * np.pi * parachute_radius**2 * parachute_height


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.

Suggested change

ma = ka * rho * (4 / 3) * np.pi * parachute_radius**2 * parachute_height

ma = ka * rho * (2 / 3) * np.pi * parachute_radius**2 * parachute_height

Gui-FernandesBR

Overall code looks cleaner and better, congratulations on this implementation, @ArthurJWH .

I have a few comments that I believe would help you to improve clarity of your code. Please let me know if you have any questions.

rocketpy/rocket/parachute.py

rocketpy/simulation/flight.py

Gui-FernandesBR · 2025-06-26T01:39:27Z

rocketpy/simulation/flight.py

        # Get Parachute data
        cd_s = self.parachute_cd_s
+        parachute_radius = self.parachute_radius
+        parachute_height = self.parachute_height
+        porosity = self.parachute_porosity
+


We don't need these definitions...

We can save a few nanoseconds by not defining them here.

You can use the attributes directly: using self.cds instead of cds

Gui-FernandesBR · 2025-06-26T01:40:38Z

rocketpy/simulation/flight.py

        pseudo_drag = -0.5 * rho * cd_s * free_stream_speed
        # pseudo_drag = pseudo_drag - ka * rho * 4 * np.pi * (R**2) * Rdot


These comments??? Should we start using it?

Suggested change

pseudo_drag = -0.5 * rho * cd_s * free_stream_speed

# pseudo_drag = pseudo_drag - ka * rho * 4 * np.pi * (R**2) * Rdot

pseudo_drag = -0.5 * rho * cd_s * free_stream_speed

# pseudo_drag = pseudo_drag - ka * rho * 4 * np.pi * (R**2) * Rdot

Gui-FernandesBR · 2025-06-26T01:42:02Z

rocketpy/rocket/parachute.py

@@ -154,13 +157,27 @@ def __init__(
            The values are used to add noise to the pressure signal which is
            passed to the trigger function. Default value is ``(0, 0, 0)``.
            Units are in Pa.
+        parachute_radius : float, optional


can you be more spacific on the parachute_radius definition please? I believe it is still not intuitive what the radius is.

Gui-FernandesBR · 2025-07-15T17:41:19Z

README.md

@@ -261,6 +261,9 @@ main = calisto.add_parachute(
    sampling_rate=105,
    lag=1.5,
    noise=(0, 8.3, 0.5),
+    parachute_radius=1.5,
+    parachute_height=1.5,
+    porosity=0.0432,


having both cds and parachute_radius? do you think we could possible calculate the cd based on these other parameters?

Gui-FernandesBR · 2025-07-16T00:55:32Z

rocketpy/rocket/parachute.py

+    Parachute.parachute_radius : float
+        Length of the non-unique semi-axis (radius) of the inflated hemispheroid
+        parachute in meters.
+    Parachute.parachute_height : float


Suggested change

Parachute.parachute_height : float

Parachute.parachute_height : float, None

or

Suggested change

Parachute.parachute_height : float

Parachute.parachute_height : Optional[float]

Gui-FernandesBR · 2025-07-25T01:33:55Z

rocketpy/rocket/rocket.py

Question: how should the Monte Carlo simulations be affected by this PR?
Should we open another PR allowing the user to vary these new arguments within the StochasticParachute (https://docs.rocketpy.org/en/latest/reference/classes/monte_carlo/stochastic_models/stochastic_parachute.html) class?

Gui-FernandesBR · 2025-07-25T01:35:45Z

rocketpy/rocket/parachute.py

+        if parachute_height is None:
+            parachute_height = parachute_radius
+        self.parachute_height = parachute_height


self.parachute_height = parachute_height or parachute_radius

Gui-FernandesBR · 2025-08-15T00:17:16Z

rocketpy/rocket/rocket.py

+        parachute_radius=1.5,
+        parachute_height=None,


I just noticed it now...

Since it is the Parachute class, there's no need to specify this is the parachute radius or the parachute height.

Suggested change

parachute_radius=1.5,

parachute_height=None,

radius=1.5,

height=None,

I was looking in other Rocket class methods such as add_trapezoidal_fins and set_rail_buttons, and both use radius already to refer to the fuselage radius. So changing it in the add_parachute method wouldn't break the consistency?

Also, since we already use radius in some inputs as the fuselage radius, maybe it will be more clear as an input to explicitly mention parachute

I would like to hear a third opinion here, maybe @MateusStano or @phmbressan .

Based on my experience, I don't think parachute_radius is a good idea, it sounds verbose to me.
However, I don't know iff we would have any conflict with other classes.

I think the simpler naming is better. If the documentation is there we can rely on that

…tion

ArthurJWH added 2 commits June 19, 2025 12:01

ENH: added radius and porosity to parachute

0873d21

ENH: added parachute radius

02ddd1b

ArthurJWH requested a review from a team as a code owner June 26, 2025 01:04

github-project-automation bot added this to LibDev Roadmap Jun 26, 2025

github-project-automation bot moved this to Backlog in LibDev Roadmap Jun 26, 2025

ArthurJWH changed the base branch from master to develop June 26, 2025 01:05

ENH: fixing radius and height attribute in flight class

1119ef8

Gui-FernandesBR requested review from Copilot June 26, 2025 01:33

Copilot AI reviewed Jun 26, 2025

View reviewed changes

This comment was marked as outdated.

Sign in to view

ArthurJWH added 4 commits June 26, 2025 12:31

STY: ruff formatting

413503f

TST: updated test value due to gravity update in u_dot_parachute

f09a4b0

ENH: added new parameters into add_parachute method in Rocket class

78ef92e

DOC: added new parameters to the documentation

7defc38

MateusStano approved these changes Jul 4, 2025

View reviewed changes

github-project-automation bot moved this from Backlog to Next Version in LibDev Roadmap Jul 4, 2025

ArthurJWH and others added 3 commits July 4, 2025 16:57

Merge branch 'develop' into enh/improve-parachute-geometric-parametri…

465f526

…zation

TST: updated test values to match the right gravity model

c5ce166

DOC: improved descriptions of the new parameters

f84459d

Gui-FernandesBR requested a review from Copilot July 16, 2025 01:09

Copilot AI reviewed Jul 16, 2025

View reviewed changes

ArthurJWH self-assigned this Jul 19, 2025

Gui-FernandesBR approved these changes Jul 25, 2025

View reviewed changes

Gui-FernandesBR linked an issue Jul 26, 2025 that may be closed by this pull request

ENH: Improve Parachute Input Values #31

Open

ArthurJWH added 4 commits August 12, 2025 09:39

ENH: added new parameters to the stochastic parachute

4fcb406

ENH: implementing previous comments

159cce8

ENH: fixing formatting and adding docs

0c434f8

ENH: added ka property to Parachute class

f136ffb

Gui-FernandesBR reviewed Aug 15, 2025

View reviewed changes

ArthurJWH added 4 commits August 18, 2025 16:39

ENH: fixing variable name

112b75a

ENH: fixed attribute name

40d10c5

ENH: simplifying variable names

1e2a2fd

DOC: adding parameters documentation into StochasticParachute descrip…

65530fd

…tion

	ma = ka * rho * (4 / 3) * np.pi * parachute_radius*2 parachute_height
	ma = ka * rho * (2 / 3) * np.pi * parachute_radius*2 parachute_height

		pseudo_drag = -0.5 * rho * cd_s * free_stream_speed
		# pseudo_drag = pseudo_drag - ka * rho * 4 * np.pi * (R*2) Rdot

	Parachute.parachute_height : float
	Parachute.parachute_height : float, None

	Parachute.parachute_height : float
	Parachute.parachute_height : Optional[float]

Uh oh!

ENH: Improve parachute geometric parametrization #835

Are you sure you want to change the base?

ENH: Improve parachute geometric parametrization #835

Conversation

ArthurJWH commented Jun 26, 2025

Pull request type

Checklist

Current behavior

New behavior

Breaking change

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

This comment was marked as outdated.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Jul 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Jul 16, 2025

Choose a reason for hiding this comment

Uh oh!

Gui-FernandesBR left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

codecov bot commented Jul 15, 2025 •

edited

Loading