ENH: Introduce the StochasticAirBrakes class #785
Conversation
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
Added the ability to multiply functions with 2D domains in the __mul__ function
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
This functions appends an airbrake and controller objects previuosly created to the rocket
ENH: Merge enh/set-air-brakes-function
ENH: Merge enh/multiply_2D_functions
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
When defining the _Controller object a StochasticAirBrake was input. This is already corrected and a AirBrake object is now introduced
…ontroller BUG: StochasticAirBrake object input in _Controller
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.
Documentation related to the StochasticAirBrakes implementation has been added in StochasticAirBrakes, StochasticRocket and Rocket classes.
|
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 ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #785 +/- ##
===========================================
- Coverage 79.31% 79.11% -0.20%
===========================================
Files 96 96
Lines 11537 11581 +44
===========================================
+ Hits 9150 9162 +12
- M
10000
isses 2387 2419 +32 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
It looks good to me.
The modifications made to the Function class were nice, ideally they should be in a separate PR, but it is not a problem.
@MateusStano should be reviewing this PR, in my opinion, since he is the best person to analyze the Air Brakes
Unnecessary comment Co-authored-by: Gui-FernandesBR <63590233+Gui-FernandesBR@users.noreply.github.com>
Co-authored-by: Gui-FernandesBR <63590233+Gui-FernandesBR@users.noreply.github.com>
There was a problem hiding this comment.
Incredible that you did this! Just one question:
Currently you added the drag_coefficient_curve_factor parameter, which is the same as we do with wind profiles. We have seen before that just this factor application sometimes does not give the best possible variation of wind profiles. I wonder if this would also be the case for aerodynamic coefficients.
Do you know or can think of of any other useful way to consider uncertainties in aerodynamic coefficients?
rocketpy/rocket/rocket.py
Outdated
| def set_air_brakes( | ||
| self, | ||
| air_brakes, | ||
| controller, | ||
| ): | ||
| """Adds an air brake with a controller to the rocket. | ||
|
|
||
| Parameters | ||
| ---------- | ||
| air_brakes : AirBrakes | ||
| Air brake to be added to the rocket. | ||
| controller : _Controller | ||
| Air brake controller | ||
|
|
||
| Returns | ||
| ------- | ||
| None | ||
| """ | ||
| self.air_brakes.append(air_brakes) | ||
| self._add_controllers(controller) | ||
|
|
There was a problem hiding this comment.
This method is not needed, please remove it
rocketpy/rocket/rocket.py
Outdated
| def set_air_brakes( | ||
| self, | ||
| air_brakes, | ||
| controller, | ||
| ): | ||
| """Adds an air brake with a controller to the rocket. | ||
|
|
||
| Parameters | ||
| ---------- | ||
| air_brakes : AirBrakes | ||
| Air brake to be added to the rocket. | ||
| controller : _Controller | ||
| Air brake controller | ||
|
|
||
| Returns | ||
| ------- | ||
| None | ||
| """ | ||
| self.air_brakes.append(air_brakes) | ||
| self._add_controllers(controller) | ||
|
|
There was a problem hiding this comment.
I'd say to just remove this method. We use the set_ word for things you can only have one of in the rocket, which currently is only the railbuttons.
I get what this is trying to do, but for simplicity I would just remove it
| rocket.set_air_brakes( | ||
| air_brakes=air_brake, | ||
| controller=_controller, | ||
| ) | ||
|
|
There was a problem hiding this comment.
| rocket.set_air_brakes( | |
| air_brakes=air_brake, | |
| controller=_controller, | |
| ) | |
| self.air_brakes.append(air_brakes) | |
| self._add_controllers(controller) | |
To keep in line with the other comment, you should change this as I suggested as well
There was a problem hiding this comment.
I created the set_air_brakes function because when using the add_air_brakes function, we have to input the drag_coefficient_curve, which isn't modified by the drag_coefficient_curve_factor (since it's a string). The factor actually affects the air_brake.drag_coefficient, which is the function you can multiply. If we use the add_air_brakes function, the sampled air brakes would all be the same, as they are created from the same drag_coefficient_curve. To address this, I created a function where you can directly add the sampled air brake with reshaped coefficients.
I really like the way you suggested adding the sampled air brake to the rocket object. It's very clean and simple! I wish I had thought of that! However, instead of using self.airbrakes, it should be rocket.air_brakes, since you're in the StochasticRocket class, right?
There was a problem hiding this comment.
However, instead of using
self.airbrakes, it should berocket.air_brakes, since you're in theStochasticRocketclass, right?
Oh yeah, you're right my bad
I created the
set_air_brakesfunction because when using theadd_air_brakesfunction, we have to input thedrag_coefficient_curve, which isn't modified by thedrag_coefficient_curve_factor(since it's a string). The factor actually affects theair_brake.drag_coefficient, which is the function you can multiply. If we use theadd_air_brakesfunction, the sampled air brakes would all be the same, as they are created from the samedrag_coefficient_curve. To address this, I created a function where you can directly add the sampled air brake with reshaped coefficients.
Yep I know exactly what you are doing because I've done similar things before hahah. But these add_ and set_ methods are simple at a first glance, but we had a lot of problems with them before so I just want to avoid modifying or creating new ones as much as possible
There was a problem hiding this comment.
I understand, for sure!! Let me do those changes
There was a problem hiding this comment.
Done! @MateusStano
Co-authored-by: MateusStano <69485049+MateusStano@users.noreply.github.com>
…/kevin-alcaniz/RocketPy into enh/stochastic-airbrakes-feature
I think multiplying by a factor is a quite satisfying approximation for drag coefficients, unlike wind profile. Normally, our drag estimations can fail because we either overestimate or underestimate them, and that's what the factor represents. If we're being strict, it's also true that this deviation can vary with the Mach number, but I consider this a second-order effect (not as impactful on the results). However, if I had to account for this effect, I would reshape the drag coefficient function by multiplying each discrete point from the source function by a random value. I didn’t attempt to do this in this PR because I think it would significantly increase the simulation time in the Monte Carlo simulations. Maybe I'm wrong, but that's what my instinct says. Since I consider it a second-order effect, I thought it wasn't worth dedicating too much effort to. Maybe you think differently. If you do and decide to implement it, we’d be more than happy to receive it! |
There was a problem hiding this comment.
Code looks good, congratulations on this one, @kevin-alcaniz !!
There are two conflicting files that must be resolved before merging.
We can work on our side to create the unit tests for the Function class, as suggested by @MateusStano
|
One question: I didn't add any of my PRs to the CHANGELOG.md file. I saw you didn't either. Should I have done it?? I can add the three PRs before you merge this one. |
|
I already resolved the conflicts!! |
|
@kevin-alcaniz please update the CHANGELOG with this PR and other which were already merged. |
|
@MateusStano just waiting for your re-review then |
Yes! I understood how it works!! 😂 |
commit c674725 Author: Kevin Alcañiz <kevinalcaniz22@gmail.com> 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 <63590233+Gui-FernandesBR@users.noreply.github.com> * 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 <63590233+Gui-FernandesBR@users.noreply.github.com> commit 9f2644a Author: Lucas Prates <57069366+Lucas-Prates@users.noreply.github.com> 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 <167456467+ArthurJWH@users.noreply.github.com> 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 <63590233+Gui-FernandesBR@users.noreply.github.com> commit 6bf70f3 Author: Júlio Machado <85506246+juliomachad0@users.noreply.github.com> 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 <63590233+Gui-FernandesBR@users.noreply.github.com> 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 <69485049+MateusStano@users.noreply.github.com> Date: Mon Mar 24 17:35:03 2025 +0100 REL: Update version to 1.9.0 (#795) commit 5328d66 Author: MateusStano <69485049+MateusStano@users.noreply.github.com> 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 <63590233+Gui-FernandesBR@users.noreply.github.com> 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 <kevinalcaniz22@gmail.com> 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 <63590233+Gui-FernandesBR@users.noreply.github.com> * DOC: improve drag curve factor definition in StochasticAirBrakes * ENH: Change assert statement to if Co-authored-by: Gui-FernandesBR <63590233+Gui-FernandesBR@users.noreply.github.com> * DOC: better explanation of __mul__ function Co-authored-by: MateusStano <69485049+MateusStano@users.noreply.github.com> * ENH: delete set_air_brakes function for simplicity * DOC: CHANGELOG file updated --------- Co-authored-by: Gui-FernandesBR <63590233+Gui-FernandesBR@users.noreply.github.com> Co-authored-by: MateusStano <69485049+MateusStano@users.noreply.github.com> commit 90553f5 Author: Kevin Alcañiz <kevinalcaniz22@gmail.com> 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 <69485049+MateusStano@users.noreply.github.com> * 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 <guilherme_fernandes@usp.br> commit 7348053 Author: Kevin Alcañiz <kevinalcaniz22@gmail.com> 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 <69485049+MateusStano@users.noreply.github.com> --------- Co-authored-by: MateusStano <69485049+MateusStano@users.noreply.github.com> commit d2f89ba Author: Leonardo Rosa <leogabriel3@gmail.com> 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 <63590233+Gui-FernandesBR@users.noreply.github.com> commit 91ac567 Author: Leonardo Rosa <leogabriel3@gmail.com> 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 <74966503+L30-stack@users.noreply.github.com> 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
Pull request type
Checklist
black rocketpy/ tests/) has passed locallypytest tests -m slow --runslow) have passed locallyCHANGELOG.mdhas been updated (if relevant)Current behavior
Currently, it’s not possible to account for air brakes control in the Monte Carlo simulations. This feature would be useful to validate the control algorithm under uncertainty, such as variations in drag curves or rocket mass.
New behavior
Has been created to define the stochastic parameters of the airbrake and create a sample of that object using the 'create_object' function.
A new function named 'add_air_brakes' has been created to add the StochasticAirbrakes and the _Controller to the StochasticRocket. Another function named '_create_air_brake' has been created to define a sample of an AirBrakes object. This object is added to the sampled Rocket in the 'create_object' function. Other minor changes has been done.
An extension of the 'mul' function has been developed to allow the multiplication of functions with 2D domains. This is necessary to modify the air brake drag curve, which is function of the deployment level and the mach number.
A new function 'set_air_brakes' has been created to add to the Rocket an air brake and controller previously defined. This is used in StochasticRocket class.
The 'time_overshoot' option has been added since now it's possible to add controllers.
The 'time_overshoot' option has been added since now it's possible to add controllers.
Breaking change