Skip to content

BUG: report missing impact roots explicitly - #1148

Merged
Gui-FernandesBR merged 1 commit into
RocketPy-Team:developfrom
ting-hong-shieh:codex/bug-impact-root-error
Aug 14, 2026
Merged

BUG: report missing impact roots explicitly#1148
Gui-FernandesBR merged 1 commit into
RocketPy-Team:developfrom
ting-hong-shieh:codex/bug-impact-root-error

Conversation

@ting-hong-shieh

Copy link
Copy Markdown

Pull request type

  • Code changes (bugfix)
  • Code maintenance
  • ReadMe, Docs and GitHub updates
  • Other

Closes #1147.

Current behavior

Flight.__handle_impact_event() reports multiple valid interpolation roots explicitly, but assumes the list is non-empty before reading its first element.

At merge base cb6106a717207dd8fc2dfe1446d80ff75022f21b, a one-second step whose cubic roots are [-1 + 0j, 2 + 0j] produces:

IndexError: list index out of range

New behavior

The impact handler checks the zero-root case before indexing and reports:

ValueError: No valid roots found when solving for impact time.

This matches the existing rail-exit error style. The single-root path is unchanged, and multiple valid roots retain their existing ValueError.

At head e511a80b2cb0c8140db6818af0dcff2245e217b0, focused tests exercise all three root counts. The single-root case also verifies impact time, velocity, phase updates, time-node updates, and solver termination.

Verification

  • .venv/bin/python -m pytest tests/unit/simulation/test_flight.py -k handle_impact_event -q: 3 passed, 60 deselected
  • .venv/bin/python -m pytest tests/unit/simulation/test_flight.py -q: 59 passed, 4 skipped
  • .venv/bin/ruff check rocketpy/simulation/flight.py tests/unit/simulation/test_flight.py: passed
  • .venv/bin/ruff format --check rocketpy/simulation/flight.py tests/unit/simulation/test_flight.py: passed
  • git diff --check: passed

The full integration and slow test suites were not run locally.

Environment: RocketPy 1.13.0; Python 3.12.6; NumPy 2.5.2; SciPy 1.18.0; pytest 9.1.1; Ruff 0.16.3; macOS 26.5.2 arm64.

Breaking change

  • No

@codecov

codecov Bot commented Aug 14, 2026

Copy link
Copy Markdown

Codecov Report

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

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #1148      +/-   ##
===========================================
+ Coverage    82.18%   83.29%   +1.11%     
===========================================
  Files          122      130       +8     
  Lines        16355    17082     +727     
===========================================
+ Hits         13441    14229     +788     
+ Misses        2914     2853      -61     

☔ 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
ting-hong-shieh marked this pull request as ready for review August 14, 2026 02:34
@ting-hong-shieh
ting-hong-shieh requested a review from a team as a code owner August 14, 2026 02:34
@Gui-FernandesBR
Gui-FernandesBR merged commit 4b53bf1 into RocketPy-Team:develop Aug 14, 2026
19 checks passed
@Gui-FernandesBR Gui-FernandesBR linked an issue Aug 14, 2026 that may be closed by this pull request
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.

BUG: report missing impact roots explicitly

2 participants