Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ENH: Add the Coriolis Force to the Flight class #799

Open
wants to merge 46 commits into
base: develop
Choose a base branch
from

Conversation

kevin-alcaniz
Copy link
Collaborator

@kevin-alcaniz kevin-alcaniz commented Mar 31, 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

Currently, RocketPy doesn't account for the Coriolis force, even though the Flight Coordinate System is not truly an inertial reference frame.

New behavior

  • Environment class:
    A new attribute has been added: the Earth's angular velocity vector. Additionally, a new function has been implemented to compute it.

  • Flight class:
    The Coriolis acceleration has been added to the vdot vector in the u_dot_generalized function. A theoretical explanation justifying this implementation is attached below.

Breaking change

  • Yes
  • No

Theory

image
image

kevin-alcaniz and others added 30 commits March 14, 2025 18:46
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
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.
Unnecessary comment

Co-authored-by: Gui-FernandesBR <[email protected]>
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.

Looking good to me...

Please don't forget to update the CHANGELOG as well.
I hope one day we implement the autochangelog tool in this repo :)

@github-project-automation github-project-automation bot moved this from Backlog to Next Version in LibDev Roadmap Apr 3, 2025
@Gui-FernandesBR Gui-FernandesBR linked an issue Apr 3, 2025 that may be closed by this pull request
@kevin-alcaniz
Copy link
Collaborator Author

@Gui-FernandesBR All good on my side!!

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.

Great work, @kevin-alcaniz !

From my point of view, this PR adds great value to RocketPy.
@MateusStano would like to review this PR before we proceed to merging it onto develop.

Tests are failing, this happens because some tests have values that were made by running flight and capturing the results. Since the flight simulation has changed with this PR, we have to update the values.
@kevin-alcaniz I know you've been busy, so don't worry, I believe we can work on this problem from our side.

image

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

@Copilot 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 3 out of 3 changed files in this pull request and generated no comments.

Comments suppressed due to low confidence (2)

rocketpy/simulation/flight.py:1696

  • [nitpick] Consider explicitly indicating the discarded x component (perhaps with a named placeholder like '_x') for clarity. This would help ensure that the component order is immediately clear to readers and maintainers.
_, w_earth_y, w_earth_z = self.env.earth_rotation_vector

rocketpy/simulation/flight.py:1921

  • [nitpick] Verify that the coordinate transformation via Kt applied here produces results consistent with the manual component handling in the u_dot method. Ensuring consistency in handling the Earth's rotation vector across methods is crucial for correct Coriolis force computation.
w_earth = Kt @ Vector(self.env.earth_rotation_vector)

@kevin-alcaniz
Copy link
Collaborator Author

Thank you, @Gui-FernandesBR , I appreciate it!

Sure. I'm looking forward to the third check. If you have any doubts or suggestions, please, let me know.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Next Version
Development

Successfully merging this pull request may close these issues.

ENH: Add the Coriolis Acceleration
3 participants