Skip to content

BUG: evaluate parachute triggers once per time node (#1086) - #1121

Open
thatrandomasiandev wants to merge 1 commit into
RocketPy-Team:developfrom
thatrandomasiandev:bug/1086-parachute-double-eval
Open

BUG: evaluate parachute triggers once per time node (#1086)#1121
thatrandomasiandev wants to merge 1 commit into
RocketPy-Team:developfrom
thatrandomasiandev:bug/1086-parachute-double-eval

Conversation

@thatrandomasiandev

Copy link
Copy Markdown

Pull request type

  • Code changes (bug fix, features)

Checklist

  • Tests for the changes have been added (if needed)
  • Docs have been reviewed and added / updated
  • Lint (black rocketpy/ tests/ / make lint) has passed locally
  • All tests (pytest tests -m slow --runslow) have passed locally
  • CHANGELOG.md — no action needed; an LLM workflow auto-updates it after merge

Current behavior

Each time node evaluated parachute triggers twice: an inline loop in Flight.__simulate and again in __check_and_handle_parachute_triggers.

Fixes #1086

New behavior

The inline loop is removed; only __check_and_handle_parachute_triggers runs per node. Regression test asserts one call per sampled height.

Breaking change

  • No

Additional information

Focused unit tests added/extended; full slow suite not run in this contribution pass.

Remove the duplicate inline parachute loop in Flight.__simulate; keep
only __check_and_handle_parachute_triggers.
@codecov

codecov Bot commented Aug 12, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.59%. Comparing base (e0ff281) to head (d062c37).
⚠️ Report is 49 commits behind head on develop.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #1121      +/-   ##
===========================================
+ Coverage    82.18%   82.59%   +0.41%     
===========================================
  Files          122      128       +6     
  Lines        16355    16603     +248     
===========================================
+ Hits         13441    13714     +273     
+ Misses        2914     2889      -25     

☔ View full report in Codecov by Harness.
📢 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.

@ting-hong-shieh

Copy link
Copy Markdown

I ran the same never-deploying 10 Hz trigger on current develop (cb6106a717207dd8fc2dfe1446d80ff75022f21b) and on this head (d062c37f6c7f5d3e8037f4673744784183323d4b). The flight inputs were identical.

Measurement develop PR head
trigger calls 600 300
unique rounded node heights 300 300
clean pressure rows 600 300
noise rows, including the initial row 601 301

This confirms that the duplicate path was evaluating the trigger and appending pressure data twice at each node. The head keeps one evaluation and one pressure row per node.

The committed regression file also passes independently: pytest tests/unit/test_parachute_triggers.py -q4 passed.

Environment: Python 3.12.6; NumPy 2.5.2; pytest 9.1.1; macOS 26.5.2 arm64.

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.

Parachute triggers are evaluated twice per time node in Flight.__simulate()

2 participants