Skip to content

ENH: Implementing 3-dof-simulation #745

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

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

Conversation

aZira371
Copy link
Collaborator

@aZira371 aZira371 commented Dec 3, 2024

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

for issue #655

New behavior

(DRAFT)
ENH: adds 3 DOF simulation capability to rocketpy.Flight.

*ENH: added "u_dot_3dof" for "solid_propulsion"
mode

*ENH: adding "u_dot_generalized_3dof" for "standard" mode (still incomplete)

*ENH: new parameter "simulation_mode" for swtiching between 3 dof and 6 dof

*ENH: updated conditions for "__init_equations_of_motion"

*ENH: 2 new example files have been created to test 3 dof model "test_bella_lui_flight_sim" and "test_camoes_flight_sim"

Breaking change

  • No

Additional information

This is a draft pull request!
Equations of motion have been modified in such a way that values related to various rotational dofs is set to zero.

ENH: adds 3 DOF simulation capability to rocketpy.Flight.

*ENH: added "u_dot_3dof" for "solid_propulsion"
mode

*ENH: adding "u_dot_generalized_3dof" for "standard"
mode (still incomplete)

*ENH: new parameter "simulation_mode" for swtiching
between 3 dof and 6 dof

*ENH: updated conditions for "__init_equations_of_motion"

*ENH: 2 new example files have been created to test 3 dof model
"test_bella_lui_flight_sim" and "test_camoes_flight_sim"
@Lucas-Prates Lucas-Prates added Enhancement New feature or request, including adjustments in current codes Flight Flight Class related features labels Dec 4, 2024
@Lucas-Prates Lucas-Prates linked an issue Dec 4, 2024 that may be closed by this pull request
@Lucas-Prates
Copy link
Contributor

Lucas-Prates commented Dec 4, 2024

Thank you for your interest in implementing 3-DOF in RocketPy, @aZira371 . I will try to take a better look into it, but I notice one thing already: use 6 DOF as the default input for simulation_mode for two reasons:

  1. most people will use 6 DOF;
  2. changing it to 3 DOF introduces breaking changes, and we really want to avoid that. You can see that a breaking change occurred because the tests failed.

@Lucas-Prates Lucas-Prates changed the title DRAFT: for ENH/3-dof-simulation (See #655) ENH: Implementing 3-dof-simulation Dec 4, 2024
ENH: adds 3 DOF simulation capability to rocketpy.Flight.

*ENH: added "u_dot_3dof" for "solid_propulsion"
mode

*ENH: added "u_dot_generalized_3dof" for "standard"
mode

*ENH: new parameter "simulation_mode" for swtiching
between 3 dof and 6 dof

*ENH: updated conditions for "__init_equations_of_motion"
ENH: fixed standard 3 dof

*MNT: Cleaned up the "u_dot_3dof" and "u_dot_generalized_3_dof"

*MNT: Corrected Typos in "simulation_mode"
description

*ENH: "u_dot_generalized_3_dof" fixed and tested
on examples.
@Gui-FernandesBR
Copy link
Member

Gui-FernandesBR commented Feb 8, 2025

  • Git rebase develop
  • Revert changes in the jupyter notebook
  • Create PointMassMotor
  • Create BaseRocket
  • Create PointMassRocket
  • Modify Rocket class where it's needed (here other people can help you, including Gui)
  • Modify the Flight class again, if needed (delete u_dot_3dof)
  • Include integration tests (here you can ask for help)
  • Format and lint the code (see the Makefile)

…otor

ENH: added "BaseRocket" and "PointMassRocket" to rocket class

ENH: added "PointMassMotor" to motor class with various input cases of thrust values
Copy link

codecov bot commented Feb 25, 2025

Codecov Report

Attention: Patch coverage is 25.00000% with 159 lines in your changes missing coverage. Please review.

Project coverage is 78.73%. Comparing base (4a41f7a) to head (0673076).
Report is 12 commits behind head on develop.

Files with missing lines Patch % Lines
rocketpy/simulation/flight.py 10.00% 108 Missing ⚠️
rocketpy/rocket/rocket.py 24.32% 28 Missing ⚠️
rocketpy/motors/PointMassMotor.py 56.60% 23 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop     #745      +/-   ##
===========================================
- Coverage    79.09%   78.73%   -0.37%     
===========================================
  Files           96       98       +2     
  Lines        11583    12118     +535     
===========================================
+ Hits          9162     9541     +379     
- Misses        2421     2577     +156     

☔ 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 and others added 9 commits March 23, 2025 19:17
…tPy-Team#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
…e tests

ENH: Added a new jupyter notebook for a simplified 3 DOF example

MNT: PointMassMotor intialization error fixed

MNT: PointMassRocket properties enhanced based on the example runs
MNT: Cleaned up the flight class u_dot_generalized_3dof

TODO: Add info for 3 dof cases and fix some new issues when parachute is added.
@aZira371 aZira371 marked this pull request as ready for review May 10, 2025 17:51
@aZira371 aZira371 requested a review from a team as a code owner May 10, 2025 17:51
self,
thrust_source,
dry_mass,
thrust_curve=None,
Copy link
Collaborator

@phmbressan phmbressan May 16, 2025

Choose a reason for hiding this comment

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

What is this parameter, thrust_curve? It seems not to be used in any of the attributes of the class (besides the validation schema). Have I misunderstood anything?

"""
if isinstance(thrust_source, (Function, np.ndarray, str)):
if thrust_curve is None or propellant_initial_mass is None:
raise ValueError("thrust_curve and propellant_initial_mass are required for csv, Function, or np.array inputs.")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Of course this is just aesthetics and is not a priority since the PR only came out of draft state, but this string is too long. In order to comply to the 80 chars limit, I would break it into more lines, I believe the linter does not handle it automatically.

The same is valid for the other raises.

mach = free_stream_speed / speed_of_sound

# Drag computation
if t < self.rocket.motor.burn_out_time:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Shouldn't the time be in the interval of burn start and burn out? Do you know @MateusStano ?

Copy link
Member

Choose a reason for hiding this comment

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

It should do whatever the normal udot does. I think we changed this recently and this branch is not updated though

@aZira371 I would say you should merge develop into this branch and check if this u_dot is updated with the newer standard one

"""Returns the thrust of the motor as a function of time."""
return self.thrust_source

@funcify_method("Time (s)", "Acceleration (m/s^2)")
Copy link
Collaborator

Choose a reason for hiding this comment

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

The output annotation and units are not correct.

return self._propellant_initial_mass

@property
def is_point_mass(self):
Copy link
Collaborator

Choose a reason for hiding this comment

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

This property does not make too much sense to me if it is not shared among all motors (e.g. using solid_motor.is_point_mass -> False). My recommendation would be using isinstance, unless it is used in a performance critical scenario (e.g. a long loop).

Comment on lines +95 to +98
motor_mass_func = (
self.motor.total_mass if hasattr(self.motor.total_mass, "get_value_opt")
else Function(lambda t: self.motor.total_mass)
)
Copy link
Collaborator

Choose a reason for hiding this comment

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

This check seems to be in place in order to convert to a Function the self.motor.total_mass if it is not one. However, the attribute from the motor class is annotated with funcify, so it will always be a Function.

@funcify_method("Time (s)", "Acceleration (m/s^2)")
def total_mass(self):
"""Returns the constant total mass of the point mass motor."""
return self.dry_mass
Copy link
Collaborator

Choose a reason for hiding this comment

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

To me it makes sense to evaluate the mass_flow_rate of a point mass motor if the user inputs a thrust curve. The schema would be quite similar to what is already done in the Motor parent class (e.g. Motor.total_mass_flow_rate). Let me know what you think about it.

Comment on lines +2570 to +2574
warnings.warn(
"This function is deprecated and will be removed in v1.10.0. "
"Please use the new method `tools.geodesic_to_utm` instead.",
DeprecationWarning,
)
Copy link
Member

Choose a reason for hiding this comment

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

Did you add this or is this a synchronization issue? If so please merge develop into this branch

Comment on lines +6 to +23
class PointMassMotor(Motor):
"""Class representing a motor modeled as a point mass.
Inherits from the Motor class and simplifies the model to a thrust-producing
object without detailed structural components."""

def __init__(
self,
thrust_source,
dry_mass,
thrust_curve=None,
propellant_initial_mass=None,
propellant_final_mass=None,
burn_time=None,
center_of_dry_mass_position=0,
reshape_thrust_curve=False,
interpolation_method="linear",
coordinate_system_orientation="nozzle_to_combustion_chamber",
):
Copy link
Member

Choose a reason for hiding this comment

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

Seems there are a few parameters here that can be removed since it does not matter for the point mass case:

  • coordinate_system_orientation
  • center_of_dry_mass_position
  • thrust_source and thrust_curve are redundant. Just keep whichever has the same name used in the other Motor Classes

Copy link
Member

Choose a reason for hiding this comment

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

Name of this file needs to be snake case

Comment on lines +11 to +23
def __init__(
self,
thrust_source,
dry_mass,
thrust_curve=None,
propellant_initial_mass=None,
propellant_final_mass=None,
burn_time=None,
center_of_dry_mass_position=0,
reshape_thrust_curve=False,
interpolation_method="linear",
coordinate_system_orientation="nozzle_to_combustion_chamber",
):
Copy link
Member

Choose a reason for hiding this comment

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

All arguments of the init have to be saved to an attribute of the same name. So you have to do self.propellant_final_mass = propellant_final_mass and so on.

Just be careful that some of the parameters are already saved in the Motor class

Comment on lines +91 to +93
@funcify_method("Time (s)", "Propellant Mass (kg)")
def propellant_initial_mass(self):
return self._propellant_initial_mass
Copy link
Member

Choose a reason for hiding this comment

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

There is no need to make this a property in this class. Just a standard attribute is fine

Comment on lines +27 to +40
class BaseRocket:
"""Base class for a rocket model with minimal attributes and methods."""
def __init__(self, mass):
self.mass = mass
self.motor = None
def add_motor(self, motor):
self.motor = motor
self.evaluate_total_mass()
def evaluate_total_mass(self):
if self.motor:
return self.mass + self.motor.total_mass
return self.mass


Copy link
Member

Choose a reason for hiding this comment

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

This class seems useless right now. You can just remove it

Comment on lines +41 to +42
class PointMassRocket(BaseRocket):
"""Rocket modeled as a point mass for 3-DOF simulations."""
Copy link
Member

Choose a reason for hiding this comment

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

Maybe having this inherit from the Rocket class will make the implementation easier. Then all_info and plots and prints will already be defined

Comment on lines +1226 to +1231
elif self.equations_of_motion == "solid_propulsion" and self.simulation_mode == '3 DOF':
# NOTE: The u_dot is faster, but only works for solid propulsion
self.u_dot_generalized = self.u_dot_3dof
elif self.simulation_mode == '3 DOF':
# NOTE: The u_dot is faster, but only works for solid propulsion
self.u_dot_generalized = self.u_dot_generalized_3dof
Copy link
Member

Choose a reason for hiding this comment

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

Can the PointMassRocket run with 6dof? If not then I think we should just check if the rocket is a PointMassRocket and set the equations of motion to 3dof

Comment on lines -1603 to -1674
M1 += M
M2 += N
M3 += L
# Off center moment
M3 += self.rocket.cp_eccentricity_x * R2 - self.rocket.cp_eccentricity_y * R1

# Calculate derivatives
# Angular acceleration
alpha1 = (
M1
- (
omega2
* omega3
* (
rocket_dry_I_33
+ motor_I_33_at_t
- rocket_dry_I_11
- motor_I_11_at_t
- mu * b**2
)
+ omega1
* (
(
motor_I_11_derivative_at_t
+ mass_flow_rate_at_t
* (rocket_dry_mass - 1)
* (b / total_mass_at_t) ** 2
)
- mass_flow_rate_at_t
* ((nozzle_radius / 2) ** 2 + (c - b * mu / rocket_dry_mass) ** 2)
)
)
) / (rocket_dry_I_11 + motor_I_11_at_t + mu * b**2)
alpha2 = (
M2
- (
omega1
* omega3
* (
rocket_dry_I_11
+ motor_I_11_at_t
+ mu * b**2
- rocket_dry_I_33
- motor_I_33_at_t
)
+ omega2
* (
(
motor_I_11_derivative_at_t
+ mass_flow_rate_at_t
* (rocket_dry_mass - 1)
* (b / total_mass_at_t) ** 2
)
- mass_flow_rate_at_t
* ((nozzle_radius / 2) ** 2 + (c - b * mu / rocket_dry_mass) ** 2)
)
)
) / (rocket_dry_I_11 + motor_I_11_at_t + mu * b**2)
alpha3 = (
M3
- omega3
* (
motor_I_33_derivative_at_t
- mass_flow_rate_at_t * (nozzle_radius**2) / 2
)
) / (rocket_dry_I_33 + motor_I_33_at_t)
# Euler parameters derivative
e0dot = 0.5 * (-omega1 * e1 - omega2 * e2 - omega3 * e3)
e1dot = 0.5 * (omega1 * e0 + omega3 * e2 - omega2 * e3)
e2dot = 0.5 * (omega2 * e0 - omega3 * e1 + omega1 * e3)
e3dot = 0.5 * (omega3 * e0 + omega2 * e1 - omega1 * e2)

Copy link
Member

Choose a reason for hiding this comment

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

Why are these changes made to u_dot?

mach = free_stream_speed / speed_of_sound

# Drag computation
if t < self.rocket.motor.burn_out_time:
Copy link
Member

Choose a reason for hiding this comment

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

It should do whatever the normal udot does. I think we changed this recently and this branch is not updated though

@aZira371 I would say you should merge develop into this branch and check if this u_dot is updated with the newer standard one

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement New feature or request, including adjustments in current codes Flight Flight Class related features
Projects
Status: Mid-Term
Development

Successfully merging this pull request may close these issues.

ENH: 3-DOF simulation
5 participants