ENH: acceleration data to trigger parachutes#911
ENH: acceleration data to trigger parachutes#911ViniciusCMB wants to merge 17 commits intoRocketPy-Team:developfrom
Conversation
…Team#XXX) * ENH: add u_dot parameter computation inside parachute trigger evaluation. * ENH: add acceleration_noise_function parameter to Flight class for realistic IMU simulation. * ENH: implement automatic detection of trigger signature to compute derivatives only when needed. * TST: add unit tests for parachute trigger with acceleration data and noise injection. * TST: add test runner for trigger acceleration validation without full test suite dependencies.
…tion This enhancement enables realistic avionics algorithms by providing access to acceleration data (u_dot) within parachute trigger functions, simulating how real flight computers use accelerometers (IMU) to detect flight phases. * ENH: Pass acceleration data (u_dot) to parachute trigger callbacks - Flight class now computes and injects u_dot derivatives into triggers - Automatic detection of trigger signatures to optimize performance - Only calculates u_dot when trigger explicitly requires it * ENH: Add built-in acceleration-based trigger functions - detect_apogee_acceleration: Detects apogee via vz ≈ 0 and az < 0 - detect_motor_burnout: Detects motor shutdown by acceleration drop - detect_freefall: Detects free-fall condition (low total acceleration) - detect_liftoff: Detects liftoff by high total acceleration - altitude_trigger_factory: Factory for altitude-based triggers * ENH: Implement optional accelerometer noise injection - New parameter acceleration_noise_function in Flight.__init__ - Simulates MEMS accelerometer noise (typical 0.1-0.3 m/s²) - Applied transparently to all acceleration-based triggers * TST: Add comprehensive unit tests for trigger evaluation - Validates u_dot computation and noise injection - Tests backward compatibility with legacy 3-parameter triggers - Ensures performance optimization skips u_dot for non-accelerating triggers Notes ----- - Triggers can now use signatures: (p, h, y) or (p, h, y, u_dot/acc/acceleration) - Built-in string triggers: apogee, apogee_acc, burnout, freefall, liftoff - Performance: u_dot computation doubles physics evaluations at trigger points - Fully backward compatible with existing altitude and custom triggers
Allow parachute triggers to accept (p, h, y, sensors, u_dot); Flight passes sensors+u_dot, computing u_dot on demand with noise; numeric/apogee legacy triggers carry metadata.\n\nTests: pytest tests/unit/test_parachute_trigger_acceleration.py -v\nLint: black rocketpy tests && ruff check rocketpy tests
|
Fixed the linting/test errors reported by the CI workflow in the latest commit. |
|
@ViniciusCMB how do you compare your implementation against #854, is there any overlap we should be worried about? |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #911 +/- ##
===========================================
+ Coverage 80.27% 81.09% +0.82%
===========================================
Files 104 107 +3
Lines 12769 13918 +1149
===========================================
+ Hits 10250 11287 +1037
- Misses 2519 2631 +112 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
I have reviewed the definition of Controllers and analyzed the __simulate loop in flight.py. My Conclusion: While Controllers allow for dynamic logic, the RocketPy simulation engine iterates over them in separate loops:
Impact Assessment:
However, since both modify the critical __simulate method and #854 involves significant changes to the Controller architecture (10 files), I cannot guarantee zero friction without running an integration test merging both branches. I am available to rebase and test my branch on top of #854 once it is merged to ensure stability. |
|
@ViniciusCMB I have merged the #854 so you can rebase your work on top of develop branch again. Pls fix conflicts and linters so we can proceed with the review. |
…iggers - TestTriggerSignatures: Validates 3, 4, and 5-parameter trigger signatures - TestBuiltInTriggers: Tests all built-in triggers (apogee_acc, burnout, freefall, liftoff) - TestParachuteInitialization: Verifies parachute creation with different trigger types - TestEdgeCases: Covers NaN/Inf handling, low altitude, and error conditions Tests use AAA pattern (Arrange, Act, Assert) and follow NumPy docstring style. All edge cases for realistic avionics simulation are covered. Fixes codecov patch coverage from 42.85% to >85%.
… triggers - Created docs/user/parachute_triggers.rst with complete guide - Enhanced Parachute class docstring with trigger signature details - Added parachute_triggers.rst to docs/user/index.rst Documentation covers: - Built-in triggers (burnout, apogee_acc, freefall, liftoff) - Custom trigger examples (3, 4, and 5-parameter signatures) - IMU noise simulation - Performance considerations - Best practices and complete dual-deploy example
- Reorder imports: standard library before third-party - Prefix unused arguments with underscore - Add broad-exception-caught disable for test runner
Direct calls to parachute.triggerfunc() were missing the u_dot parameter, causing TypeError in unit tests. Added u_dot=None to non-overshoot and overshoot trigger evaluation paths where u_dot is not computed. Fixes test failures in: - tests/unit/simulation/test_flight.py - tests/unit/test_utilities.py - tests/unit/test_rail_buttons_bending_moments.py - tests/unit/test_flight_data_exporter.py
There was a problem hiding this comment.
do you really need try/except blocks here?
There was a problem hiding this comment.
Pull request overview
This pull request adds acceleration-based parachute triggers with IMU sensor simulation to RocketPy, enabling realistic avionics algorithms that mimic real-world flight computers. The implementation allows users to trigger parachute deployment based on accelerometer data (motor burnout, apogee, freefall, liftoff) while maintaining full backward compatibility with existing altitude and velocity-based triggers.
Key Changes:
- Four built-in acceleration-based trigger functions with edge case handling (NaN/Inf protection)
- Flexible trigger function signatures supporting 3, 4, or 5 parameters for legacy and new use cases
- Optional IMU noise simulation via
acceleration_noise_functionparameter in Flight class - Performance optimization through lazy evaluation (u_dot computed only when needed)
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 21 comments.
Show a summary per file
| File | Description |
|---|---|
tests/unit/test_parachute_triggers.py |
New test file with basic trigger tests, but uses non-standard test naming and custom runner |
tests/unit/test_parachute_trigger_acceleration.py |
Comprehensive test suite for acceleration triggers with proper pytest structure and AAA pattern |
rocketpy/simulation/flight.py |
Adds _evaluate_parachute_trigger helper method and acceleration_noise_function parameter; includes new import for inspect module |
rocketpy/rocket/parachute.py |
Implements four built-in trigger functions and flexible wrapper for multi-signature support |
docs/user/parachute_triggers.rst |
Comprehensive documentation with examples, best practices, and performance considerations |
docs/user/index.rst |
Adds parachute triggers documentation to table of contents |
| def _test_trigger_with_u_dot_only(): | ||
| """Test trigger that only expects u_dot (no sensors).""" | ||
|
|
||
| def derivative_func(_t, _y): | ||
| return np.array([0, 0, 0, -1.0, -2.0, -3.0, 0, 0, 0, 0, 0, 0, 0]) | ||
|
|
||
| recorded = {} | ||
|
|
||
| def user_trigger(_p, _h, _y, u_dot): | ||
| recorded["u_dot"] = np.array(u_dot) | ||
| return False | ||
|
|
||
| parachute = Parachute( | ||
| name="test_u_dot_only", | ||
| cd_s=1.0, | ||
| trigger=user_trigger, | ||
| sampling_rate=100, | ||
| ) | ||
|
|
||
| dummy = type("D", (), {})() | ||
| dummy.acceleration_noise_function = lambda: np.array([0.0, 0.0, 0.0]) | ||
|
|
||
| res = Flight._evaluate_parachute_trigger( | ||
| dummy, | ||
| parachute, | ||
| pressure=0.0, | ||
| height=5.0, | ||
| y=np.zeros(13), | ||
| sensors=[], | ||
| derivative_func=derivative_func, | ||
| t=1.234, | ||
| ) | ||
|
|
||
| assert res is False | ||
| assert "u_dot" in recorded | ||
| assert np.allclose(recorded["u_dot"][3:6], np.array([-1.0, -2.0, -3.0])) |
There was a problem hiding this comment.
This test function does not follow the project's test naming convention. The leading underscore prevents pytest from automatically discovering and running this test. According to the project guidelines, test names should follow the pattern test_methodname_expectedbehaviour.
The function should be renamed to test_trigger_with_u_dot_only to ensure proper test execution.
| name="Flight", | ||
| equations_of_motion="standard", | ||
| ode_solver="LSODA", | ||
| acceleration_noise_function=None, | ||
| simulation_mode="6 DOF", | ||
| weathercock_coeff=0.0, | ||
| ): |
There was a problem hiding this comment.
The acceleration_noise_function parameter is missing from the Flight.init docstring Parameters section. This is a new parameter introduced in this PR that should be properly documented in the docstring above this code section.
The docstring should include documentation for acceleration_noise_function describing:
- Its type (callable or None)
- What it should return (array-like of length 3 with noise values for [ax_noise, ay_noise, az_noise])
- Its purpose (simulating IMU accelerometer noise for realistic avionics simulation)
- Units for the returned values (m/s²)
- Default value (None, which results in zero noise)
docs/user/parachute_triggers.rst
Outdated
| flight = Flight( | ||
| rocket=my_rocket, | ||
| environment=env, | ||
| rail_length=5.2, | ||
| inclination=85, | ||
| heading=0, | ||
| acceleration_noise_function=lambda: np.random.normal(0, 0.5, 3) | ||
| ) |
There was a problem hiding this comment.
The code examples reference undefined variables my_rocket and env. While these are common placeholder names in documentation, the examples should either define them beforehand or include a comment indicating they are placeholders.
Consider adding a comment like # Assume my_rocket and env are already defined before the first usage, or provide minimal example definitions to make the code runnable.
| def run_all(): | ||
| tests = [ | ||
| _test_trigger_receives_u_dot_and_noise, | ||
| _test_trigger_with_u_dot_only, | ||
| _test_legacy_trigger_does_not_compute_u_dot, | ||
| ] | ||
| failures = 0 | ||
| for t in tests: | ||
| name = t.__name__ | ||
| try: | ||
| t() | ||
| print(f"[PASS] {name}") | ||
| except Exception: # pylint: disable=broad-exception-caught | ||
| failures += 1 | ||
| print(f"[FAIL] {name}") | ||
| traceback.print_exc() | ||
| if failures: | ||
| print(f"{failures} test(s) failed") | ||
| raise SystemExit(1) | ||
| print("All tests passed") | ||
|
|
||
|
|
||
| if __name__ == "__main__": | ||
| run_all() |
There was a problem hiding this comment.
This custom test runner implementation is unnecessary and deviates from pytest best practices. The RocketPy project uses pytest as its test framework, and these tests should be run using pytest's standard test discovery and execution mechanisms.
The run_all() function and if __name__ == "__main__" block should be removed. Instead, rely on pytest to discover and execute tests by ensuring test functions start with test_ (without leading underscores).
| def altitude_trigger_factory(target_altitude, require_descent=True): | ||
| """Return a trigger that deploys when altitude <= target_altitude. | ||
|
|
||
| If require_descent is True, also require vertical velocity negative | ||
| (descending) to avoid firing during ascent. | ||
| """ | ||
|
|
||
| def trigger(_pressure, height, state_vector, _u_dot=None): | ||
| vz = float(state_vector[5]) | ||
| if require_descent: | ||
| return (height <= target_altitude) and (vz < 0) | ||
| return height <= target_altitude | ||
|
|
||
| return trigger |
There was a problem hiding this comment.
The docstring for this factory function is incomplete according to the project's documentation standards. It lacks the NumPy-style docstring format with proper Parameters, Returns, and Examples sections.
The docstring should be formatted to include:
- A comprehensive Parameters section documenting both parameters with types and units
- A Returns section describing the returned trigger function and its signature
- An Examples section showing typical usage
- Documentation of physical units (e.g., "meters" for target_altitude)
rocketpy/rocket/parachute.py
Outdated
| def detect_liftoff(_pressure, _height, _state_vector, u_dot): | ||
| """Detect liftoff by high total acceleration. | ||
|
|
||
| Liftoff is characterized by a sudden increase in acceleration as the motor | ||
| ignites and begins producing thrust. | ||
| """ | ||
| try: | ||
| if u_dot is None or len(u_dot) < 6: | ||
| return False | ||
|
|
||
| ax = float(u_dot[3]) | ||
| ay = float(u_dot[4]) | ||
| az = float(u_dot[5]) | ||
| if not all(np.isfinite([ax, ay, az])): | ||
| return False | ||
|
|
||
| total_acc = np.sqrt(ax * ax + ay * ay + az * az) | ||
| if not np.isfinite(total_acc): | ||
| return False | ||
|
|
||
| return total_acc > 15.0 | ||
| except (ValueError, TypeError, IndexError): | ||
| return False |
There was a problem hiding this comment.
The docstring for this function is incomplete according to the project's documentation standards. While it describes what the function does, it lacks the NumPy-style docstring format with proper Parameters, Returns, and Examples sections.
The docstring should be formatted to include:
- A comprehensive Parameters section documenting all four parameters with types and units
- A Returns section describing the return value and type
- An Examples section showing typical usage
- Documentation of physical units (e.g., "m/s²" for accelerations)
rocketpy/rocket/parachute.py
Outdated
| raise ValueError( | ||
| f"Unable to set the trigger function for parachute '{self.name}'. " | ||
| + "Trigger must be a callable, a float value or one of the strings " | ||
| + "('apogee','burnout','freefall','liftoff'). " |
There was a problem hiding this comment.
The error message lists invalid trigger string options but is missing 'apogee_acc' which is a valid trigger option according to the code. The mapping on lines 456-461 includes 'apogee_acc' as a valid key, but the error message doesn't mention it.
The error message should be updated to include all valid string options: ('apogee', 'apogee_acc', 'burnout', 'freefall', 'liftoff').
| + "('apogee','burnout','freefall','liftoff'). " | |
| + "('apogee','apogee_acc','burnout','freefall','liftoff'). " |
|
|
||
| # Use acceleration data | ||
| az = u_dot[5] | ||
|
|
There was a problem hiding this comment.
The example code references an undefined variable threshold. This will cause a NameError if users try to run this example as-is.
The example should either define threshold before use (e.g., threshold = 100.0 # Example threshold) or use a concrete numeric value in the comparison.
| # Define threshold for IMU reading (example value) | |
| threshold = 100.0 # Example threshold, adjust as needed |
rocketpy/simulation/flight.py
Outdated
| params = list(sig.parameters.keys()) | ||
| acc_names = {"u_dot", "udot", "acc", "acceleration"} | ||
| expects_udot = any(p.lower() in acc_names for p in params[3:]) | ||
| expects_sensors = any(p.lower() == "sensors" for p in params[3:]) |
There was a problem hiding this comment.
Variable expects_sensors is not used.
rocketpy/simulation/flight.py
Outdated
| expects_sensors = any(p.lower() == "sensors" for p in params[3:]) | ||
| except (ValueError, TypeError): | ||
| expects_udot = False | ||
| expects_sensors = True |
There was a problem hiding this comment.
Variable expects_sensors is not used.
rocketpy/rocket/parachute.py
Outdated
| - A callable function that can take 3, 4, or 5 arguments: | ||
|
|
||
| `[x, y, z, vx, vy, vz, e0, e1, e2, e3, wx, wy, wz]`. | ||
| **3 arguments** (legacy): |
There was a problem hiding this comment.
This isn't legacy, it is still a valid option
| **3 arguments** (legacy): | |
| **3 arguments**: |
rocketpy/rocket/parachute.py
Outdated
| def detect_motor_burnout(_pressure, height, state_vector, u_dot): | ||
| """Detect motor burnout by sudden drop in acceleration. | ||
|
|
||
| Returns True when vertical acceleration becomes significantly negative | ||
| (indicating end of propulsion phase) OR when total acceleration drops below | ||
| a threshold indicating coasting/free-fall has begun. | ||
| """ | ||
| try: | ||
| if u_dot is None or len(u_dot) < 6: | ||
| return False | ||
|
|
||
| ax = float(u_dot[3]) | ||
| ay = float(u_dot[4]) | ||
| az = float(u_dot[5]) | ||
|
|
||
| # Defensive checks for NaN/Inf | ||
| if not all(np.isfinite([ax, ay, az])): | ||
| return False | ||
|
|
||
| total_acc = np.sqrt(ax * ax + ay * ay + az * az) | ||
| if not np.isfinite(total_acc): | ||
| return False | ||
|
|
||
| # Additional safety: ignore spurious low-accel readings at t~0 by | ||
| # requiring the rocket to be above a small altitude and still ascending | ||
| vz = float(state_vector[5]) if len(state_vector) > 5 else 0 | ||
| if not np.isfinite(vz): | ||
| return False | ||
|
|
||
| if height < 5.0 or vz <= 0.5: | ||
| return False | ||
|
|
||
| # Burnout detected when: | ||
| # 1. Vertical acceleration becomes very negative (end of thrust phase) | ||
| # 2. OR total acceleration drops below 2.0 m/s² (coasting detected) | ||
| return az < -8.0 or total_acc < 2.0 | ||
| except (ValueError, TypeError, IndexError): | ||
| return False |
There was a problem hiding this comment.
I don't know how useful a function to deploy a parachute right after burnout really is, but I still have a few issues with this:
- There are a few hard-coded values that should be user defined
- The try except here is doing basically nothing
- No need to check Nan of Inf, those values should never be returned in either u_dot nor state_vector
In the end I think this isn't really useful to be included in the code, but it would be a cool example to include in the docs somewhere
rocketpy/rocket/parachute.py
Outdated
|
|
||
| # Additional safety: ignore spurious low-accel readings at t~0 by | ||
| # requiring the rocket to be above a small altitude and still ascending | ||
| vz = float(state_vector[5]) if len(state_vector) > 5 else 0 |
There was a problem hiding this comment.
state_vector always has the same length. No need to check it
rocketpy/rocket/parachute.py
Outdated
| if not np.isfinite(total_acc): | ||
| return False |
There was a problem hiding this comment.
Acceleration will never be infinite
rocketpy/rocket/parachute.py
Outdated
| if u_dot is None or len(u_dot) < 6: | ||
| return False |
There was a problem hiding this comment.
this will never happen. No need for this check
rocketpy/simulation/flight.py
Outdated
| u_dot = None | ||
| if expects_udot: | ||
| try: | ||
| u_dot = np.array(derivative_func(t, y), dtype=float) |
There was a problem hiding this comment.
no need to convert to array
rocketpy/simulation/flight.py
Outdated
| except (ValueError, TypeError, RuntimeError): | ||
| # If u_dot computation fails, leave as None | ||
| u_dot = None |
There was a problem hiding this comment.
u_dot computation should never fail, if it does the simulation is broken anyway. Please remove this try except
rocketpy/simulation/flight.py
Outdated
| if not expects_udot and not expects_sensors: | ||
| trig_original = getattr(parachute, "trigger", None) | ||
| if callable(trig_original): | ||
| try: | ||
| sig = inspect.signature(trig_original) | ||
| params = list(sig.parameters.keys()) | ||
| acc_names = {"u_dot", "udot", "acc", "acceleration"} | ||
| expects_udot = any(p.lower() in acc_names for p in params[3:]) | ||
| expects_sensors = any(p.lower() == "sensors" for p in params[3:]) | ||
| except (ValueError, TypeError): | ||
| expects_udot = False | ||
| expects_sensors = True |
There was a problem hiding this comment.
expects_udot and expects sensors should always be defined in parachutes.py. There should be no need to perform extra checks in the Flight class.
rocketpy/simulation/flight.py
Outdated
| height_above_ground_level, | ||
| self.y_sol, | ||
| self.sensors, | ||
| None, # u_dot not computed in non-overshoot path |
There was a problem hiding this comment.
Why not? u_dot needs to be here to make the triggerfunc work properly in case someone uses the new udot param
rocketpy/rocket/parachute.py
Outdated
| try: | ||
| return fn(p, h, y, sensors, u_dot) | ||
| except TypeError: | ||
| try: | ||
| return fn(p, h, y, u_dot) | ||
| except TypeError: | ||
| try: | ||
| return fn(p, h, y, sensors) | ||
| except TypeError: | ||
| return fn(p, h, y) |
There was a problem hiding this comment.
This is not necessary. We need to be able to detect exactly what the user fn function needs in terms of parameters
|
@MateusStano I willtake a look at your comments and try to get back to you in case @ViniciusCMB doesn't do. However, Vinicius, if you are still available to contribute, please take a look at the comments,try to solve all of them and push it back here. |
Almost done! |
ViniciusCMB
changed the title
|
@Gui-FernandesBR, @MateusStano, I was really focused on the original issue and ended up implementing the "liftoff", "burnout", and other triggers as built-ins, even though it doesn't make much sense for the parachute (I believe it might be useful for other deployables, though). As for the noise sensor, I just read about it in the issue and implemented it; I wasn't aware it had already been done. I removed the excessive checks because, at the time, I hadn't fully read the documentation and was worried about avoidable errors showing up. |
|
@ViniciusCMB you have mark every comment as "resolved" before we start to review again. This makes the review process much easier. |
Pull Request: Add Acceleration-Based Parachute Triggers with IMU Sensor Simulation
Pull request type
Checklist
black) has been run locallyWhy this matters
This is a critical feature for advanced users simulating realistic avionics.
Real flight computers rely heavily on accelerometer signals (IMU) for event
detection (liftoff, burnout, coast/free-fall behavior), while pressure/GPS-only
logic is often noisier or slower in practice.
Until now, parachute triggers primarily relied on pressure/height/state-vector
inputs. This made it difficult to reproduce avionics-style event logic that
depends on acceleration profiles.
What this PR changes
This PR adds first-class support for acceleration-aware trigger callables by
passing state derivatives (
u_dot) to user-defined trigger functions.1) Trigger callable signatures support
u_dotIn
rocketpy/rocket/parachute.py, callable dispatch supports:3 args:(pressure, height, state_vector)4 args:(pressure, height, state_vector, u_dot)or(…, sensors)5 args:(pressure, height, state_vector, sensors, u_dot)Dispatch is deterministic, and trigger wrappers expose metadata
(
_expects_udot,_expects_sensors) used byFlight.2)
Flightcomputesu_dot(t, y)inside trigger evaluation when neededIn
rocketpy/simulation/flight.py, trigger checks consistently route through_evaluate_parachute_trigger(...), andu_dotis computed only when requiredby trigger metadata.
This follows the architecture discussed in Issue #156: acceleration is not in
the solver state vector and must be obtained from derivative evaluation at the
event-check instant.
3) Docs now include acceleration-based trigger examples
docs/user/parachute_triggers.rstwas updated with custom trigger examples for:Thresholds are intentionally mission-specific and configurable by users.
4) Final API scope from review
Per review direction, this PR keeps the API focused on custom callables and
does not introduce acceleration-specific built-in trigger strings.
Acceptance criteria coverage (Issue #156)
Flightto calculateu_dotinside event checking flowu_dot) to user-defined triggersFiles changed
rocketpy/rocket/parachute.pyu_dot/sensor-aware trigger wrapper behaviorrocketpy/simulation/flight.pyu_dotcomputationdocs/user/parachute_triggers.rsttests/unit/test_parachute_triggers.pyu_dotdeliverytests/unit/test_parachute_trigger_acceleration.pyBreaking change
No breaking change in trigger usage: existing numeric,
"apogee", and legacy3-argument callables remain supported.
Validation performed
blackrun on modified Python filespytest tests/unit/test_parachute_triggers.py -qpassingRelated issue