Skip to content

Conversation

@kevin-alcaniz
Copy link
Collaborator

@kevin-alcaniz kevin-alcaniz commented Mar 19, 2025

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

RocketPy currently does not take into account the thrust dependence on the ambient pressure.

It's a important behaviour considering two situations: correction for thrust curves obtained from a static hot fire test performed in a altitude much different from the launch or in-flight corrections for high target altitude flights.

This PR is related to an Issue: ENH: Add pressure thrust #769

New behavior

The theory applied to this method is explained in the image:

image

  • Motor classes:

A new optional attribute, reference_pressure, has been introduced. If not defined, RocketPy's behavior remains unchanged. Given the reference pressure and the thrust curve, the vacuum thrust curve can be computed using the new get_vacuum_thrust function.

  • Flight class:

If a reference pressure is provided, a new method is implemented to calculate the net thrust. Otherwise, the net thrust remains equal to the current thrust.

Additionally, a new funcify_method has been created for net thrust, and thrust power is now calculated based on net thrust as well.

Breaking change

  • Yes
  • No

Validation

The results obtained after implementing this new feature are shown below:

image

The Y-axis represents Thrust (N) and the X-axis represents Time (s). As you can see, at the very beginning, both curves (raw and net thrust) are the same because the reference pressure is set to sea level pressure. As the rocket ascends, the net thrust increases because the negative term in equation 5 decreases.

I have also attempted to validate this behavior with ASTOS, and the results obtained are shown below:

WhatsApp Image 2025-03-19 at 11 02 52_564937b2

In this plot, the vacuum thrust curve (blue) is input directly into ASTOS. The net thrust curve (black) is evaluated by ASTOS. Finally, the calculated thrust curve (red) was evaluated by me using the same method implemented in this PR.

the wind factor wasn't applied to the env.wind_velocity properties
It showed the nominal and the standard deviation values and it doesn't make sense in a uniform distribution. In a np.random.uniform the 'nominal value' is the lower bound of the distribution, and the 'standard deviation' value is the upper bound. Now, a new condition has been added for the uniform distributions where the mean and semi range are calculated and showed. This way the visualize_attribute function will show the whole range where the random values are uniformly taken in
The thrust generated by a SolidMotor is now adjusted for the atmospheric pressure. To achieve that, a new attribute, 'vacuum_thrust', has been created. The 'net_thrust' is the result of 'vacuum_thrust' minus the atmospheric pressure multiplied by the nozzle area.
@kevin-alcaniz
Copy link
Collaborator Author

There are other modifications related to another PR (BUG: fix the wind velocity factors usage and better visualization of uniform distributions in Stochastic Classes #783)

@codecov
Copy link

codecov bot commented Mar 20, 2025

Codecov Report

Attention: Patch coverage is 82.14286% with 5 lines in your changes missing coverage. Please review.

Project coverage is 79.10%. Comparing base (4df0b38) to head (e8215df).
Report is 1 commits behind head on develop.

Files with missing lines Patch % Lines
rocketpy/motors/motor.py 58.33% 5 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop     #789      +/-   ##
===========================================
- Coverage    79.11%   79.10%   -0.02%     
===========================================
  Files           96       96              
  Lines        11575    11589      +14     
===========================================
+ Hits          9158     9167       +9     
- Misses        2417     2422       +5     

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

@Gui-FernandesBR
Copy link
Member

Another great contribution made by @kevin-alcaniz , thank you!!

We are going to review it completely as soon as possible

@caioessouza caioessouza mentioned this pull request Mar 21, 2025
4 tasks
Copy link
Collaborator

@phmbressan phmbressan left a comment

Choose a reason for hiding this comment

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

Amazing work @kevin-alcaniz ! I made some comments regarding the split into two thrust attributes that I would like to hear some second opinions from the team before proceeding; of course, feel free to also participate on the discussions.

@Gui-FernandesBR Gui-FernandesBR linked an issue Mar 25, 2025 that may be closed by this pull request
4 tasks
@phmbressan
Copy link
Collaborator

phmbressan commented Mar 29, 2025

Good work addressing the review comments @kevin-alcaniz ! I plan to give this PR a final look tomorrow, sorry for the delay.

@kevin-alcaniz
Copy link
Collaborator Author

@phmbressan Thank you! Everything is ready on my end for the merge. Let me know if you find any issues.

Copy link
Member

@Gui-FernandesBR Gui-FernandesBR left a comment

Choose a reason for hiding this comment

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

LGTM.

Waiting for @phmbressan 's review

@github-project-automation github-project-automation bot moved this from Backlog to Next Version in LibDev Roadmap Mar 29, 2025
@kevin-alcaniz
Copy link
Collaborator Author

One minute, I'm going to update de CHANGELOG. I always forget 😂

@kevin-alcaniz
Copy link
Collaborator Author

Done!

Copy link
Collaborator

@phmbressan phmbressan left a comment

Choose a reason for hiding this comment

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

After reviewing a bit of literature on the topic and the excellent info that @kevin-alcaniz provided in the PR's description, I believe the root of my questions regarding the exhaust_velocity are related to these two concepts:

  • exhaust velocity: the actual exhaust velocity of the gases exiting the nozzle, denoted by $V_9$;
  • effective exhaust velocity: a measure of efficiency that can be computed by $\frac{T_{\text{net}}}{\dot{m}}$ (or $\frac{T_{\text{ref}}}{\dot{m}}$ in case of a static fire).

If I understood correctly, I believe the current exhaust_velocity method for liquid motors actually computes the effective exhaust velocity of the reference thrust curve (e.g. static fire). A similar reasoning apply to the exhaust_velocity methods of the other motors, but assuming the mass flow rate as constant.

In my view, this behavior is ok, but it should be documented (likely a Notes section in the docstring) that the value being computed is not the $V_9$ in the formula, but the effective exhaust velocity of the reference curve provided.

What do you think @kevin-alcaniz , did I misunderstand anything?

The rocket's thrust in a vaccum.
"""
if self.reference_pressure is None:
print("Reference pressure is not set, cannot calculate vacuum thrust")
Copy link
Collaborator

Choose a reason for hiding this comment

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

To be clearer on the outcome of the method, I would mention in the message that the vacuum thrust will be equivalent to thrust. Maybe a user warning would be better.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I’m not sure I fully understood what you meant by that.
Would you like me to clarify in the print that the vacuum thrust refers to the actual thrust the rocket would produce in a vacuum?
Or are you suggesting that I should add a warning indicating that this thrust value won’t be used in the actual simulation?
Could you let me know what you had in mind, please?

Copy link
Collaborator

Choose a reason for hiding this comment

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

My suggestion is that, since we cannot compute the vacuum thrust when the reference pressure is None, we should raise a warning in this case to inform that the vacuum_thrust method will simply return the thrust.

@Gui-FernandesBR
Copy link
Member

If I understood correctly, I believe the current exhaust_velocity method for liquid motors actually computes the effective exhaust velocity of the reference thrust curve (e.g. static fire). A similar reasoning apply to the exhaust_velocity methods of the other motors, but assuming the mass flow rate as constant.

In my view, this behavior is ok, but it should be documented (likely a Notes section in the docstring) that the value being computed is not the V 9 in the formula, but the effective exhaust velocity of the reference curve provided.

What do you think @kevin-alcaniz , did I misunderstand anything?

@kevin-alcaniz what do you think about it?

@kevin-alcaniz
Copy link
Collaborator Author

Sorry, @phmbressan , for the delay — I’ve been quite busy these past few days.

Yes, I think you're right. To accurately calculate the actual exhaust velocity, we’d need the thermodynamic properties of the gas upstream of the nozzle as well as the nozzle geometry. That’s not feasible with the current set of parameters used in RocketPy.

If you'd like, I can improve the documentation for this parameter. I propose adding a disclaimer in both the class attribute descriptions and within the exhaust velocity functions themselves.

Would that work for you?

@Gui-FernandesBR Gui-FernandesBR requested a review from Copilot April 7, 2025 13:56
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.

Comments suppressed due to low confidence (1)

rocketpy/motors/motor.py:1063

  • [nitpick] Consider replacing the print statement with a logging mechanism to better control output in production environments.
print("Reference pressure is not set, cannot calculate vacuum thrust")

@phmbressan
Copy link
Collaborator

If you'd like, I can improve the documentation for this parameter. I propose adding a disclaimer in both the class attribute descriptions and within the exhaust velocity functions themselves.

Would that work for you?

Yes, a description in the style you mentioned should be good to clarify everything.

@kevin-alcaniz
Copy link
Collaborator Author

@phmbressan I've already applied the requested changes. I added a warning for the vacuum thrust when no reference pressure is defined, and improved the documentation for the exhaust velocity, both in the class attributes and the related functions.

If you come across anything else that needs attention, feel free to let me know!

@kevin-alcaniz kevin-alcaniz merged commit c674725 into RocketPy-Team:develop Apr 12, 2025
2 of 8 checks passed
@github-project-automation github-project-automation bot moved this from Next Version to Closed in LibDev Roadmap Apr 12, 2025
@kevin-alcaniz kevin-alcaniz deleted the enh/net-thrust branch April 12, 2025 11:41
MateusStano added a commit that referenced this pull request Apr 21, 2025
commit c674725
Author: Kevin Alcañiz <[email protected]>
Date:   Sat Apr 12 13:40:25 2025 +0200

    ENH: Introduce Net Thrust with pressure corrections (#789)

    * wind factor bug corrected

    the wind factor wasn't applied to the env.wind_velocity properties

    * BUG: StochasticModel visualize attributes of a uniform distribution

    It showed the nominal and the standard deviation values and it doesn't make sense in a uniform distribution. In a np.random.uniform the 'nominal value' is the lower bound of the distribution, and the 'standard deviation' value is the upper bound. Now, a new condition has been added for the uniform distributions where the mean and semi range are calculated and showed. This way the visualize_attribute function will show the whole range where the random values are uniformly taken in

    * variable names corrections

    * Corrections requested by the pylint test

    * ENH: Add pressure corrections for thrust in SolidMotor

    The thrust generated by a SolidMotor is now adjusted for the atmospheric pressure. To achieve that, a new attribute, 'vacuum_thrust', has been created. The 'net_thrust' is the result of 'vacuum_thrust' minus the atmospheric pressure multiplied by the nozzle area.

    * ENH: pylint recommendations done

    * ENH: net thrust method extended to the rest of the motor classes

    * BUG: __post_processed_variables inconsistent array

    * ENH: ruff reformatting

    * Update rocketpy/motors/motor.py

    Co-authored-by: Gui-FernandesBR <[email protected]>

    * ENH: Avoid breaking change

    * ENH: Pressure Thrust method added

    * BUG: call to the thrust function wrong

    * BUG: pressure thrust evaluated when motor is turned off

    * ENH: CHANGELOG updated

    * DOC: definition of exhaust velocity improved

    ---------

    Co-authored-by: Gui-FernandesBR <[email protected]>

commit 9f2644a
Author: Lucas Prates <[email protected]>
Date:   Sat Apr 12 11:27:53 2025 +0200

    ENH: Implement Multivariate Rejection Sampling (MRS) (#738)

    * ENH: implementing a draft version of the Multivarite Rejectio Sampler (MRS).

    * MNT: quick notebook to test MRS during development

    * MNT: refactoring class to match review suggestions

    * ENH: add comparison prints, plots and ellipses to MonteCarlo and finally checks in MRS

    * MNT: add MultivariateRejectionSampler class to inits and apply format

    * DOC: writting .rst documentation for MRS

    * MNT: adding pylint flags to skip checks

    * DOC: completing missing sections in mrs.rst

    * DOC: add changelog and apply sugestions in MRS class

    * DOC: apply suggestions to the MRS.rst

    * MNT: use Union instead of | for type hinting since we have to support python3.9

    * TST: adding unit and integration tests to MRS

    * MNT: use pylint flag to fix linter

    * TST: adding tests to MonteCarlo comparison features

    * MNT: applying suggestions in .rst, better handling nested variables in MRS and applying linters

    * MNT: removing TODO comments from monte_carlo_plots

    * MNT: remove useless TODO

    * MNT: inserting pragmas for no cover and resolving changelog conflict

commit d49c40e
Author: ArthurJWH <[email protected]>
Date:   Fri Apr 11 16:11:20 2025 -0400

    ENH: Create a rocketpy file to store flight simulations (#800)

    * ENH: added .rpy file functionality (see issue 668)

    This commit add 'save_to_rpy' and 'load_from_rpy' functions, that allows saving and loading flights.

    * MNT: adjusting minor changes to .rpy functions and tests.

    Formatted docstrings correctly.
    Reverted duplication of `test_encoding.py` files.
    Version warning will be called when loaded version is more recent.

    * MNT: incorporating previous comments

    Change file management from os to Path
    Adjust docstrings

    * DOC: Added comment about outputs in `to_dict` method

    * MNT: Refactoring `RocketPyDecoder` unpacking operation and other small adjustments

    * DOC: update changelog

    * STY: formatted according to ruff

    * MNT: changing `str | Path` operation to support Python 3.9

    * MNT: fixed trailing commas on .rpy and added shield against `ruff` formatting .rpy and .json files

    * MNT: fixing error related to `test_flight_save_load_no_resimulate`

    When `include_outputs` were set to `True`, it would try to include the additional data into the flight, breaking the test

    * MNT: fixing a typo and adding comment on test coverage

    ---------

    Co-authored-by: Gui-FernandesBR <[email protected]>

commit 6bf70f3
Author: Júlio Machado <[email protected]>
Date:   Sat Apr 5 15:08:53 2025 -0300

    ENH: Support for the RSE file format has been added to the library (#798)

    * ENH: Support for the RSE file format has been added to the library. The import_rse method in the Abstract Motor class and the load_from_rse_file method in the GenericMotor class are now available. With this update, the library natively supports Rock Sim software data, eliminating the need for users to manually convert motor files. The implementation was based on the import_eng and load_from_eng_file methods, utilizing Python's standard XML library.

    * ENH: Adding tests to the methods of .rse file treatment.

    * ENH: fixing mistakes on the method and test file

    * MNT: Running ruff

    * MNT: Adding the PR to CHANGELOG.md

commit 220bb59
Merge: 4a41f7a 4df0b38
Author: Gui-FernandesBR <[email protected]>
Date:   Thu Mar 27 06:14:22 2025 -0300

    Merge pull request #797 from RocketPy-Team/master

    Updates develop after 1.9.0

commit 4df0b38
Author: MateusStano <[email protected]>
Date:   Mon Mar 24 17:35:03 2025 +0100

    REL: Update version to 1.9.0 (#795)

commit 5328d66
Author: MateusStano <[email protected]>
Date:   Mon Mar 24 13:07:52 2025 +0100

    DEP: Remove Pending Deprecations and Add Warnings Where Needed (#794)

    * DEP: Add deprecation warnings for outdated methods and functions

    * DEP: Remove deprecated methods for NOAA RUC soundings and power drag plots

    * DEV: changelog

    * MNT: ruff

    * DEP: Update deprecation warning for post_process method to specify removal in v1.10

    * MNT: Remove unused imports

commit 76fb5ef
Merge: a4b42c3 4a41f7a
Author: Gui-FernandesBR <[email protected]>
Date:   Sun Mar 23 19:17:16 2025 -0300

    Merge pull request #793 from RocketPy-Team/develop

    DEV: Master to v1.9.0

commit 4a41f7a
Author: Kevin Alcañiz <[email protected]>
Date:   Sun Mar 23 21:52:51 2025 +0100

    ENH: Introduce the StochasticAirBrakes class (#785)

    * wind factor bug corrected

    the wind factor wasn't applied to the env.wind_velocity properties

    * BUG: StochasticModel visualize attributes of a uniform distribution

    It showed the nominal and the standard deviation values and it doesn't make sense in a uniform distribution. In a np.random.uniform the 'nominal value' is the lower bound of the distribution, and the 'standard deviation' value is the upper bound. Now, a new condition has been added for the uniform distributions where the mean and semi range are calculated and showed. This way the visualize_attribute function will show the whole range where the random values are uniformly taken in

    * variable names corrections

    * Corrections requested by the pylint test

    * ENH: add multiplication for 2D functions in rocketpy.function

    Added the ability to multiply functions with 2D domains in the __mul__ function

    * ENH: StochasticAirBrakes class created

    The StochasticAirBrakes class has been created. The __init__.py files in the stochastic and rocketpy folders have also been modified accordingly to incorporate this new class

    * ENH: set_air_brakes function created

    This functions appends an airbrake and controller objects previuosly created to the rocket

    * ENH: add StochasticAirBrake to rocketpy.stochastic_rocket

    Some functions has been modified and other has been created in order to include the new StochasticAirBrakes feature into the StochasticRocket class. A new function named 'add_air_brakes' has been created to append a StochasticAirBrakes and Controller objects to the StochasticRocket object. A new function '_create_air_brake' has been introduced to create a sample of an AirBrake object through a StochasticAirBrake object. Enventually, the 'create_object' function has been modified to add the sampled AirBrakes to the sampled Rocket

    * BUG: StochasticAirBrake object input in _Controller

    When defining the _Controller object a StochasticAirBrake was input. This is already corrected and a AirBrake object is now introduced

    * ENH: add time_overshoot option to rocketpy.stochastic_flight

    Since the new StochasticAirBrake class is defined, we need the 'time_overshoot' option in the Flight class to ensure that the time step defined in the simulation is the controller sampling rate. The MonteCarlo class has had to be modified as well to include this option.

    * DOC: StochasticAirBrakes related documentation added

    Documentation related to the StochasticAirBrakes implementation has been added in StochasticAirBrakes, StochasticRocket and Rocket classes.

    * ENH: pylint recommendations done

    * ENH: Reformatted files to pass Ruff linting checks

    * ENH: Update rocketpy/stochastic/stochastic_rocket.py

    Unnecessary comment

    Co-authored-by: Gui-FernandesBR <[email protected]>

    * DOC: improve drag curve factor definition in StochasticAirBrakes

    * ENH: Change assert statement to if

    Co-authored-by: Gui-FernandesBR <[email protected]>

    * DOC: better explanation of __mul__ function

    Co-authored-by: MateusStano <[email protected]>

    * ENH: delete set_air_brakes function for simplicity

    * DOC: CHANGELOG file updated

    ---------

    Co-authored-by: Gui-FernandesBR <[email protected]>
    Co-authored-by: MateusStano <[email protected]>

commit 90553f5
Author: Kevin Alcañiz <[email protected]>
Date:   Sun Mar 23 20:31:50 2025 +0100

    ENH: Add Eccentricity to Stochastic Simulations (#792)

    * wind factor bug corrected

    the wind factor wasn't applied to the env.wind_velocity properties

    * BUG: StochasticModel visualize attributes of a uniform distribution

    It showed the nominal and the standard deviation values and it doesn't make sense in a uniform distribution. In a np.random.uniform the 'nominal value' is the lower bound of the distribution, and the 'standard deviation' value is the upper bound. Now, a new condition has been added for the uniform distributions where the mean and semi range are calculated and showed. This way the visualize_attribute function will show the whole range where the random values are uniformly taken in

    * variable names corrections

    * Corrections requested by the pylint test

    * ENH: more intuitive uniform distribution display in StochasticModel

    Co-authored-by: MateusStano <[email protected]>

    * ENH: Eccentricities added to the StochasticRocket class

    A bug has been corrected in Flight class and an enhancement has been performed in the Rocket class as well

    * BUG: thrust eccentricity bug corrected

    eccentricity_y was defined by x coordinate and eccentricity_x was defined by y coordinate

    * BUG: Undo some Rocket class changes

    * ENH: add eccentricities to StochasticRocket

    * BUG: fix MonteCarlo eccentricity inputs

    * ENH: pylint and ruff recommended changes

    * TST: fix tests with eccentricity

    ---------

    Co-authored-by: Gui-FernandesBR <[email protected]>

commit 7348053
Author: Kevin Alcañiz <[email protected]>
Date:   Sun Mar 23 13:49:35 2025 +0100

    BUG: fix the wind velocity factors usage and better visualization of uniform distributions in Stochastic Classes (#783)

    * wind factor bug corrected

    the wind factor wasn't applied to the env.wind_velocity properties

    * BUG: StochasticModel visualize attributes of a uniform distribution

    It showed the nominal and the standard deviation values and it doesn't make sense in a uniform distribution. In a np.random.uniform the 'nominal value' is the lower bound of the distribution, and the 'standard deviation' value is the upper bound. Now, a new condition has been added for the uniform distributions where the mean and semi range are calculated and showed. This way the visualize_attribute function will show the whole range where the random values are uniformly taken in

    * variable names corrections

    * Corrections requested by the pylint test

    * ENH: more intuitive uniform distribution display in StochasticModel

    Co-authored-by: MateusStano <[email protected]>

    ---------

    Co-authored-by: MateusStano <[email protected]>

commit d2f89ba
Author: Leonardo Rosa <[email protected]>
Date:   Fri Mar 21 18:57:49 2025 -0300

    DEV: add requirements-tests.txt on make install target (#791)

    * DEV: adds 'pip install -r requirements-tests.txt' recipe to 'make install' target on Makefile

    Co-authored-by: Gui-FernandesBR <[email protected]>

commit 91ac567
Author: Leonardo Rosa <[email protected]>
Date:   Fri Mar 21 18:53:53 2025 -0300

    BUG: fixes get_instance_attributes for Flight objects containing a Rocket object without rail buttons (#786)

    * DOC: fixed a typo in funcify_method() description

    * TST: created test for get_instante_attributes() with flight without rail buttons

    * BUG: fixed __calculate_rail_button_forces() by assigning a Function(0) to null_force instead of an empty array

    * DEV: updates CHANGELOG

commit 9407470
Author: Leonard <[email protected]>
Date:   Wed Mar 19 16:01:59 2025 +0100

    BUG: fixed AGL altitude in _FlightPrints.events_registered (#788)

    * BUG: fixed AGL altitude in _FlightPrints.events_registered

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

Labels

None yet

Projects

Status: Closed

Development

Successfully merging this pull request may close these issues.

ENH: Add pressure thrust

3 participants