Skip to content

Conversation

@Rafit345
Copy link

Pull request type

  • Code changes (bugfix, features)
  • Code maintenance (refactoring, formatting, tests)
  • ReadMe, Docs and GitHub updates
  • Other (please describe):

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

Refs #28

New behavior

This commit was made as a submission
to the selective process deliverables challenge.

The method udot_rail2 functions as an intermediate flight phase
before the rocket has fully left the guide rail, allowing for
3 degrees of freedom (linear motion along the rail, pitch and yaw).

Flight init includes a feature to run a simulation without udot_rail2.
Numerical values enabling udot_rail2 are very close to 1 DOF flight.
Flight phase transitions smoothly from 1 DOF rail phase to 3DOF
and from 3 DOF to 6 DOF free flight.

Current equations of motion inside udot_rail2 rely heavily on
udot_generalized, ensuring 3 DOF through vector operations.

--Summary--
Add preliminary udot_rail2 (3-DOF tip-off) support and safe,
deterministic phase-insertion handling during rail → 6DOF transitions.
Add a feature flag to enable/disable udot_rail2 on Flight init.
Add a Hermite-root fallback to avoid hard failures when rail-exit root
filtering returns no valid root (warn + midpoint fallback).
Add comprehensive unit tests (alignment, no-roll, insertion-order,
CSV comparisons) and sample CSV output for comparison runs with udot_rail2
enabled vs disabled.

Breaking change

  • Yes
  • No

Additional information

Still working on the implementation of proper lagrangean expansion
/derivation of equations of motion. Articles
"Tip-off effect analysis of a vehicle moving along an inclined guideway
by considering dynamic interactions" by Chou et al and
"ANALYSIS OF MISSILE LAUNCHERS PART Q
Tipoff Effects in Helical Rail Launchers" by Hosken et al
are proving useful.

Some tests are still not passing (10/1561)

…is)"

Refs RocketPy-Team#28.  This commit was made as a submission
to the selective process deliverables challenge.

The method udot_rail2 functions as an intermediate flight phase
before the rocket has fully left the guide rail, allowing for
3 degrees of freedom (linear motion along the rail, pitch and yaw).

Flight init includes a feature to run a simulation without udot_rail2.
Numerical values enabling udot_rail2 are very close to 1 DOF flight.
Flight phase transitions smoothly from 1 DOF rail phase to 3DOF
and from 3 DOF to 6 DOF free flight.

Current equations of motion inside udot_rail2 rely heavily on
udot_generalized, ensuring 3 DOF through vector operations.
Still working on the implementation of proper lagrangean expansion
/derivation of equations of motion. Articles
"Tip-off effect analysis of a vehicle moving along an inclined guideway
by considering dynamic interactions" by Chou et al and
"ANALYSIS OF MISSILE LAUNCHERS PART Q
Tipoff Effects in Helical Rail Launchers" by  Hosken et al
are proving useful.

--Summary--
Add preliminary udot_rail2 (3-DOF tip-off) support and safe,
deterministic phase-insertion handling during rail → 6DOF transitions.
Add a feature flag to enable/disable udot_rail2 on Flight init.
Add a Hermite-root fallback to avoid hard failures when rail-exit root
 filtering returns no valid root (warn + midpoint fallback).
Add comprehensive unit tests (alignment, no-roll, insertion-order,
CSV comparisons) and sample CSV output for comparison runs with udot_rail2
 enabled vs disabled.
@Rafit345 Rafit345 requested a review from a team as a code owner December 10, 2025 23:47
@Rafit345 Rafit345 changed the base branch from master to develop December 10, 2025 23:47
@aZira371
Copy link
Collaborator

Hey @Rafit345 Thanks for this PR! You have done some solid work here. I am still reviewing your code changes. But I'll suggest to run make lint and make format with your commits!

@aZira371 aZira371 self-requested a review December 12, 2025 16:37
print(f"Current Simulation Time: {self.t:3.4f} s", end="\r")

# check for the first time the rocket is between the two rail buttons for tip-off analysis
if len(self.between_rails_state) == 1 and (
Copy link
Collaborator

Choose a reason for hiding this comment

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

The condition len(self.betweenrailsstate) == 1 checks whether the state has been set once, not whether the rocket is currently between the buttons. Once betweenrailsstate is populated, subsequent integration steps will not re-enter this block, so the phase is only added at the exact instant the lower button clears, not maintained throughout the tip-off interval. This means the udot_rail2 phase may execute for only a single time step or not propagate correctly.

Copy link
Collaborator

@aZira371 aZira371 left a comment

Choose a reason for hiding this comment

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

I have the following comments for the preliminary implementation. In general looks to be going in a good direction as of now. Will wait for u_dotrail2 implementation to be fully complete in order to review the tests and involved physics.

raise ValueError(
"Multiple roots found when solving for rail exit time."
)
if len(valid_t_root) == 0: # pragma: no cover
Copy link
Collaborator

Choose a reason for hiding this comment

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

this fallback uses an arbitrary midpoint guess when the Hermite interpolation fails to find a valid root. This fallback has no physical justification, if no root exists in the bracket, taking the midpoint may place the transition at a point where the rocket is not at effective1rl, violating the state's physical meaning. The warning is issued, but the simulation proceeds with potentially incorrect rail-exit kinematics, and downstream analysis (e.g., launch angle) will be corrupted.​
Instead of a fallback, the code should either raise an exception (forcing the user to investigate convergence issues) or attempt a refined bracket search with tighter tolerances.

self.out_of_rail_time = self.initial_solution[0]
self.out_of_rail_time_index = 0
# save out of rail 2 state and time with the same data as out of rail
self.between_rails_state = self.initial_solution[1:]
Copy link
Collaborator

Choose a reason for hiding this comment

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

In init, the code initializes self.betweenrailsstate and self.betweenrailstime in multiple branches (3 times once in if elif and else each). These three branches set identical values for betweenrailsstate and betweenrailstime. This is redundant. The initialization should be factored out after the branching logic.


def udot_rail2(self, t, u, post_processing=False): # pragma: no cover
"""[Still not implemented] Calculates derivative of u state vector with
"""[WIP] Calculates derivative of u state vector with
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggestion for WIP:

True 3-DOF tip-off on a rail with one button requires tracking the normal contact force and the reaction moment. The state vector for udot_rail2 appears to reuse the 13-state format from udot_generalized (x, y, z, vx, vy, vz, e0, e1, e2, e3, ω1, ω2, ω3). However, single-button dynamics are inherently unstable without proper constraint enforcement. The angular velocities ω1 and ω2 will grow due to aerodynamic and thrust misalignment torques, but the code does not implement any constraint forces to stabilize the solution. Without these, the integrator may produce non-physical angular accelerations.


def udot_rail2(self, t, u, post_processing=False): # pragma: no cover
"""[Still not implemented] Calculates derivative of u state vector with
"""[WIP] Calculates derivative of u state vector with
Copy link
Collaborator

Choose a reason for hiding this comment

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

The existing udot_rail1 (1-DOF rail phase) computes thrust, drag, and rail reaction forces. The new udot_rail2 must account for:

Off-axis aerodynamic forces and moments (due to angle of attack during tip-off)

Thrust misalignment effects

Gravity components in the body frame

Inertia tensor evolution as the rocket rotates

None of these appear in the provided code stub.

)


def test_udot_rail2_csv_comparison_generation(
Copy link
Collaborator

Choose a reason for hiding this comment

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

test_udot_rail2_csv_comparison_generation generates CSV files comparing enabled/disabled runs and computes pitch/yaw deltas, but does not assert any expected behavior—it only checks that files exist and contain headers. This is a structural test, not a validation test

@Gui-FernandesBR
Copy link
Member

@Rafit345 thank you for your submission! Please address all the comments and ask for a re-review whenever this PR is ready again.

We look forwarding to receiving updates from you soon!

@Gui-FernandesBR Gui-FernandesBR marked this pull request as draft December 14, 2025 17:42
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.

Pull request overview

This PR implements a preliminary 3-DOF single rail button flight phase (tip-off analysis) for RocketPy, introducing an intermediate udot_rail2 phase that operates between the initial 1-DOF rail phase and the 6-DOF free flight phase. The implementation adds a feature flag use_udot_rail2 to enable/disable this behavior, includes a Hermite-root fallback for numerical robustness, and provides comprehensive unit tests to verify correctness.

  • Adds udot_rail2 method implementing 3-DOF equations of motion (linear motion along rail + pitch/yaw, enforcing zero roll)
  • Introduces use_udot_rail2 parameter (default True) to control the intermediate rail phase activation
  • Implements between-rails event detection and smooth phase transitions from 1-DOF → 3-DOF → 6-DOF
  • Adds fallback handling for edge cases where rail-exit root filtering returns no valid roots

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 15 comments.

File Description
rocketpy/simulation/flight.py Implements udot_rail2 3-DOF rail phase with equations of motion, adds use_udot_rail2 parameter, implements between-rails event detection, adds fallback for root finding failures, initializes attitude_unit and between_rails_state tracking
tests/unit/simulation/test_udot_rail2_feature.py Adds unit tests verifying phase insertion order, zero roll enforcement, rail alignment constraints, and CSV output generation for enabled/disabled comparison

return K @ Vector([0.0, 0.0, 1.0])


def test_udot_rail2_inserts_phase_in_order(calisto_robust, example_spaceport_env):
Copy link

Copilot AI Dec 14, 2025

Choose a reason for hiding this comment

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

The test name doesn't follow the project's naming convention "test_methodname_expectedbehaviour". It should be renamed to something like "test_udot_rail2_phase_insertion_order_correct" or "test_flight_with_udot_rail2_inserts_phase_before_generalized" to better match the convention and clearly indicate what behavior is expected.

Copilot generated this review using guidance from repository custom instructions.
Comment on lines +128 to +130
def test_udot_rail2_csv_comparison_generation(
calisto_robust, example_spaceport_env, tmp_path
):
Copy link

Copilot AI Dec 14, 2025

Choose a reason for hiding this comment

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

The test name doesn't follow the project's naming convention "test_methodname_expectedbehaviour". It should be renamed to something like "test_udot_rail2_csv_output_generated_correctly" to better match the convention and clearly indicate what behavior is expected.

Copilot generated this review using guidance from repository custom instructions.
if verbose:
print(f"Current Simulation Time: {self.t:3.4f} s", end="\r")

# check for the first time the rocket is between the two rail buttons for tip-off analysis
Copy link

Copilot AI Dec 14, 2025

Choose a reason for hiding this comment

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

The comment should use proper capitalization and end with a period for consistency with other comments in the codebase.

Suggested change
# check for the first time the rocket is between the two rail buttons for tip-off analysis
# Check for the first time the rocket is between the two rail buttons for tip-off analysis.

Copilot uses AI. Check for mistakes.
Comment on lines +539 to 544
use_udot_rail2 : bool, optional
If True, enable the intermediate 3-DOF rail phase `udot_rail2`.
If False, the flight remains in the rail1 phase until the
upper rail button exit and then transitions directly to the
generalized 6-DOF dynamics. Default is True.
w1_init, w2_init, w3_init
Copy link

Copilot AI Dec 14, 2025

Choose a reason for hiding this comment

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

The use_udot_rail2 parameter documentation is incorrectly positioned in the middle of the initial_solution parameter's code block example. This breaks the docstring structure. The parameter documentation should be placed after the equations_of_motion parameter and before ode_solver, matching the actual parameter order in the function signature.

Copilot uses AI. Check for mistakes.
self.flight_phases.add_phase(
self.t_initial, self.initial_derivative, clear=False
)
# self.flight_phases.add_phase()
Copy link

Copilot AI Dec 14, 2025

Choose a reason for hiding this comment

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

This commented-out line appears to be leftover debug code or an incomplete implementation. It should either be removed or completed with a meaningful comment explaining its purpose if it's intentionally left for future work.

Suggested change
# self.flight_phases.add_phase()

Copilot uses AI. Check for mistakes.
y1,
yp1,
)
a += 1e-5 # TODO: why??
Copy link

Copilot AI Dec 14, 2025

Choose a reason for hiding this comment

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

The TODO comment "why??" indicates unclear or unexplained code logic. This addition of a small epsilon (1e-5) to the coefficient appears to be a workaround for a numerical issue, but the reason is not documented. This should be investigated and properly explained, or the workaround should be replaced with a more robust solution. Consider documenting the specific numerical issue this addresses or removing it if it's not necessary.

Suggested change
a += 1e-5 # TODO: why??
# Add a small epsilon to the cubic coefficient to avoid
# degenerate or nearly-degenerate cubic equations, which can
# cause numerical instability or missed roots in the root-finding
# process. This helps ensure that the cubic Hermite interpolation
# produces a valid solution for the rail exit time, especially in
# cases where the trajectory is nearly tangential to the rail.
a += 1e-5

Copilot uses AI. Check for mistakes.

return [vx, vy, vz, ax, ay, az, 0, 0, 0, 0, 0, 0, 0]

def udot_rail2(self, t, u, post_processing=False): # pragma: no cover
Copy link

Copilot AI Dec 14, 2025

Choose a reason for hiding this comment

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

The pragma comment "# pragma: no cover" on this method indicates it's excluded from test coverage, but the PR adds comprehensive unit tests for this feature. Consider removing this pragma since the method is now tested and should be included in coverage metrics.

Copilot uses AI. Check for mistakes.
Comment on lines +2272 to +2276
e_dot = [
0.5 * (-omega1 * e1 - omega2 * e2), # - omega3 * e3),
0.5 * (omega1 * e0 - omega2 * e3), # omega3 * e2
0.5 * (omega2 * e0 + omega1 * e3), # - omega3 * e1
0.5 * (omega2 * e1 - omega1 * e2), # +omega3 * e0
Copy link

Copilot AI Dec 14, 2025

Choose a reason for hiding this comment

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

The commented-out portions of the Euler parameter derivatives (e.g., "# - omega3 * e3", "# omega3 * e2") suggest incomplete implementation or uncertainty about the correct equations. For a 3-DOF rail phase that enforces zero roll, these omega3 terms should indeed be omitted, but this should be clearly documented in a comment explaining why they're excluded rather than leaving them as commented code. Consider either removing them entirely or adding a clear explanation.

Suggested change
e_dot = [
0.5 * (-omega1 * e1 - omega2 * e2), # - omega3 * e3),
0.5 * (omega1 * e0 - omega2 * e3), # omega3 * e2
0.5 * (omega2 * e0 + omega1 * e3), # - omega3 * e1
0.5 * (omega2 * e1 - omega1 * e2), # +omega3 * e0
# In the 3-DOF rail phase, roll is constrained and omega3 is always zero.
# Therefore, the omega3 terms are omitted from the quaternion derivative equations.
e_dot = [
0.5 * (-omega1 * e1 - omega2 * e2),
0.5 * (omega1 * e0 - omega2 * e3),
0.5 * (omega2 * e0 + omega1 * e3),
0.5 * (omega2 * e1 - omega1 * e2),

Copilot uses AI. Check for mistakes.

# u_dot layout for udot_rail2: [r_dot_x, r_dot_y, r_dot_z, v_dot_x, v_dot_y, v_dot_z, e_dot..., w_dot_x, w_dot_y, w_dot_z]
r_dot = Vector(u_dot[0:3])
v_dot = Vector(u_dot[3:6])
Copy link

Copilot AI Dec 14, 2025

Choose a reason for hiding this comment

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

Variable v_dot is not used.

Suggested change
v_dot = Vector(u_dot[3:6])

Copilot uses AI. Check for mistakes.
import math
import os

import numpy as np
Copy link

Copilot AI Dec 14, 2025

Choose a reason for hiding this comment

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

Import of 'np' is not used.

Suggested change
import numpy as np

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants