RocketPy icon indicating copy to clipboard operation
RocketPy copied to clipboard

Fix hard-coded radius value for parachute added mass calculation

Open Copilot opened this issue 1 month ago • 5 comments

Pull request type

  • [x] Code changes (bugfix, features)

Checklist

  • [ ] Tests for the changes have been added (if needed)
  • [x] Docs have been reviewed and added / updated
  • [x] Lint (black rocketpy/ tests/) has passed locally
  • [x] All tests (pytest tests -m slow --runslow) have passed locally
  • [x] CHANGELOG.md has been updated (if relevant)

Current behavior

Hard-coded radius=1.5 in Parachute.__init__ and Rocket.add_parachute causes incorrect added mass calculations for smaller parachutes. For drogue chutes (cd_s ≈ 1.0), the added mass was overestimated by ~31x, resulting in excessive drift distances.

Ref: #860

New behavior

Radius is now computed from cd_s when not explicitly provided:

R = sqrt(cd_s / (Cd * π))

Using HEMISPHERICAL_CD = 1.4 (typical hemispherical parachute drag coefficient).

Changes:

  • Default radius parameter changed from 1.5 to None in both Parachute class and Rocket.add_parachute method
  • Added HEMISPHERICAL_CD class constant
  • Auto-calculate radius from cd_s when not provided
  • Updated docstrings with formula documentation
  • Updated from_dict to handle new default

Example results:

Parachute cd_s Old radius New radius
Main 10.0 1.5 m 1.51 m
Drogue 1.0 1.5 m 0.48 m

Users can still explicitly set radius to override auto-calculation.

Breaking change

  • [ ] Yes
  • [x] No

Existing code providing explicit radius values is unaffected. Code relying on the implicit default will now get physically appropriate values based on cd_s.

  • Fixes RocketPy-Team/RocketPy#860
Original prompt

This section details on the original issue you should resolve

<issue_title>Remove Hard coded radius value for lateral acceleration in descent under parachute</issue_title> <issue_description>Is your feature request related to a problem? Please describe.

Lateral acceleration for descent under parachute (in the function u_dot_parachute) depends on the air trapped beneath a parachute to determine the mass to divide drag force by. This added mass value depends on the radius R which is hard coded to 1.5 This causes issues with drift distance for smaller rockets and drogue parachutes leading to much higher drift distances than expected

Describe the solution you'd like

Either remove the hard coded value or determine radius based on a hard coded value for the coefficient of drag for a parachute, which is a much more reasonable thing to be hard coded.

Additional context

N/A </issue_description>

<agent_instructions>Remove Hard coded radius value for lateral acceleration in descent under parachute #860 Open Task @RyanSamF Description RyanSamF opened on Oct 24 Is your feature request related to a problem? Please describe.

Lateral acceleration for descent under parachute (in the function u_dot_parachute) depends on the air trapped beneath a parachute to determine the mass to divide drag force by. This added mass value depends on the radius R which is hard coded to 1.5 This causes issues with drift distance for smaller rockets and drogue parachutes leading to much higher drift distances than expected

Describe the solution you'd like

Either remove the hard coded value or determine radius based on a hard coded value for the coefficient of drag for a parachute, which is a much more reasonable thing to be hard coded.

Additional context

N/A

Activity

github-project-automation added this to LibDev Roadmapon Oct 24

github-project-automation moved this to Backlog in LibDev Roadmapon Oct 24 Gui-FernandesBR Gui-FernandesBR commented on Oct 25 Gui-FernandesBR on Oct 25 Member Could you help us providing references?

RyanSamF RyanSamF commented on Oct 25 RyanSamF on Oct 25 Author RocketPy/rocketpy/simulation/flight.py

Lines 1969 to 1995 in 2655a4f

ka = 1 # Added mass coefficient (depends on parachute's porosity) R = 1.5 # Parachute radius

to = 1.2

eta = 1

Rdot = (6 * R * (1 - eta) / (1.2**6)) * (

(1 - eta) * t5 + eta * (to3) * (t**2)

)

Rdot = 0

Calculate added mass

ma = ka * rho * (4 / 3) * np.pi * R**3

Calculate freestream speed

freestream_x = vx - wind_velocity_x freestream_y = vy - wind_velocity_y freestream_z = vz free_stream_speed = (freestream_x2 + freestream_y2 + freestream_z**2) ** 0.5

Determine drag force

pseudo_drag = -0.5 * rho * cd_s * free_stream_speed

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

Dx = pseudo_drag * freestream_x Dy = pseudo_drag * freestream_y Dz = pseudo_drag * freestream_z ax = Dx / (mp + ma) ay = Dy / (mp + ma) az = (Dz - 9.8 * mp) / (mp + ma)

Gui-FernandesBR added the Task issue type 8 hours ago

Gui-FernandesBR added Parachute Related to parachutes methods and usage 8 hours ago Gui-FernandesBR Gui-FernandesBR commented 8 hours ago Gui-FernandesBR 8 hours ago Member @RyanSamF when I say "reference", I was actually meaning "bibliometric references". I'm going to search about it.

I have to investigate it better, but I think @ArthurJWH has already solved this one with his recent modifications to the parachute class

Gui-FernandesBR Gui-FernandesBR commented now Gui-FernandesBR now Member The main reference for parachute modeling is: https://apps.dtic.mil/sti/tr/pdf/ADA247666.pdf

Also, don't forget to run make format, make lint e make pytest antes de marcar o PR como concluido.

</agent_instructions>

Comments on the Issue (you are @copilot in this section)

@Gui-FernandesBR Could you help us providing references? @Gui-FernandesBR @RyanSamF when I say "reference", I was actually meaning "bibliometric references". I'm going to search about it.

I have to investigate it better, but I think @ArthurJWH has already solved this one with his recent modifications to the parachute class</comment_new> <comment_new>@Gui-FernandesBR

The main reference for parachute modeling is: https://apps.dtic.mil/sti/tr/pdf/ADA247666.pdf</comment_new>
  • Fixes RocketPy-Team/RocketPy#860

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Copilot avatar Nov 28 '25 00:11 Copilot

@copilot remind to add the PR to the CHANGELOG file

Gui-FernandesBR avatar Nov 28 '25 00:11 Gui-FernandesBR

@copilot remind to add the PR to the CHANGELOG file

Added the CHANGELOG entry in commit 314ca47.

Copilot avatar Nov 28 '25 01:11 Copilot

Codecov Report

:white_check_mark: All modified and coverable lines are covered by tests. :white_check_mark: Project coverage is 81.03%. Comparing base (9cf3dd4) to head (a5b3da6). :warning: Report is 23 commits behind head on develop.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop     #889      +/-   ##
===========================================
+ Coverage    80.27%   81.03%   +0.76%     
===========================================
  Files          104      107       +3     
  Lines        12769    13648     +879     
===========================================
+ Hits         10250    11060     +810     
- Misses        2519     2588      +69     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

:rocket: New features to boost your workflow:
  • :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

codecov[bot] avatar Nov 28 '25 03:11 codecov[bot]

The PR seems reasonable and correct, although it will introduce breaking changes to current simulations that relies on the previously hard coded radius. If this PR proceeds, I would suggest first changing the add_parachute default radius value for consistency in rocket class.

@copilot please solve the backwards compatibility issue here.

Gui-FernandesBR avatar Dec 02 '25 01:12 Gui-FernandesBR

The PR seems reasonable and correct, although it will introduce breaking changes to current simulations that relies on the previously hard coded radius. If this PR proceeds, I would suggest first ch...

Updated rocket.add_parachute to use radius=None as the default, matching the Parachute class. Now both APIs consistently auto-calculate radius from cd_s when not explicitly provided. See commit 24eb9a8.

Copilot avatar Dec 02 '25 01:12 Copilot