Enh/acceleration data to trigger parachutes #911
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.01% +0.74%
===========================================
Files 104 107 +3
Lines 12769 13957 +1188
===========================================
+ Hits 10250 11307 +1057
- Misses 2519 2650 +131 ☔ 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)
| 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)
| 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)
| 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 |
| 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.
| 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.
2 participants
Pull Request: Add Acceleration-Based Parachute Triggers with IMU Sensor Simulation
Pull request type
Checklist
black rocketpy/ tests/) has passed locallypytest tests -m slow --runslow) have passed locallyCurrent behavior
Currently, RocketPy forces users to trigger parachute deployment based on state variables (altitude
z, vertical velocityvz) or pressure. This simulates "God Mode" avionics rather than realistic flight computer behavior.Real-world rocket avionics (flight computers) trigger events based on accelerometer data (IMU):
Users had no way to access acceleration (
u_dot) inside trigger functions, making it impossible to simulate realistic avionics algorithms.Related Issue: RocketPy#156
New behavior
This PR introduces acceleration-based parachute triggers with the following features:
1. Acceleration Data Access in Triggers
Parachute trigger functions now optionally receive acceleration data (
u_dot):2. Built-in Acceleration-Based Triggers
New triggers on
rocketpy/rocket/parachute.pyprovides realistic avionics functions:detect_apogee_acceleration: Detects apogee when vertical velocity ≈ 0 AND acceleration becomes negativedetect_motor_burnout: Detects motor shutdown by sudden acceleration dropdetect_freefall: Detects free-fall condition (total acceleration < 1.5 m/s²)detect_liftoff: Detects liftoff by high total acceleration (> 15 m/s²)Usage:
3. Optional IMU Sensor Noise Simulation
New parameter
acceleration_noise_functioninFlight.__init__allows simulating realistic accelerometer noise:4. Performance Optimization
The Flight class automatically detects trigger signatures:
u_dotparameter → computes acceleration at event checks5. Full Backward Compatibility
trigger=300) continue workingTechnical Details
Implementation Changes
rocketpy/rocket/parachute.py__evaluate_trigger_function()now wraps triggers with signature detection_expects_udotattribute for Flight optimization"apogee","burnout","freefall","liftoff"flight.py
_evaluate_parachute_trigger()now computesu_dotwhen neededacceleration_noise_functionTests Added
test_parachute_triggers.py
test_trigger_receives_u_dot_and_noise: Validates acceleration computation and noise injectiontest_trigger_with_u_dot_only: Tests acceleration-only triggerstest_legacy_trigger_does_not_compute_u_dot: Ensures performance optimization worksTrade-offs and Considerations
Performance Impact
u_dotat event points doubles physics evaluations locallyCoordinate System
u_dot[3:6]follow state vector convention:[ax, ay, az]Breaking change
Additional information
Examples of Usage
Realistic apogee detection:
Simulating sensor noise:
Custom acceleration trigger:
Testing Performed
Documentation Notes