ENH: unstable rocket error #764

Gui-FernandesBR · 2025-02-08T16:58:20Z

Pull request type

Code changes (bugfix, features)

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

A common mistake on rocketpy is to try running a flight simulation when the rocket is unstable.
The simulation will likely fail or never stops.
It is true that not always it will fail, but I believe we should be more restricted in order to avoid these case scenarios;

New behavior

Throws a UnstableRocketException everytime a flight is instantiated using a rocket with static margin lower than 0.
Added rocketpy official exceptions

Breaking change

Yes and No. As you can see by the tests, some code may break. However, no current function will be

Additional information

Future PRs should add more custom exceptions if needed.

In this PR:

Update docs: (1) explaining about rocketpy exceptions and (2) explaining about the stability check.
CHANGELOG and slow tests.

codecov · 2025-02-08T19:28:49Z

Codecov Report

Attention: Patch coverage is 91.66667% with 1 line in your changes missing coverage. Please review.

Project coverage is 80.11%. Comparing base (83aa20e) to head (04d2354).
Report is 17 commits behind head on develop.

Files with missing lines	Patch %	Lines
rocketpy/exceptions.py	85.71%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           develop     #764      +/-   ##
===========================================
+ Coverage    76.42%   80.11%   +3.69%     
===========================================
  Files           95       96       +1     
  Lines        11090    11359     +269     
===========================================
+ Hits          8475     9100     +625     
+ Misses        2615     2259     -356

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

GabrielBarberini · 2025-02-09T04:41:43Z

rocketpy/exceptions.py

+
+
+class UnstableRocketError(RocketPyException):
+    """Raised when the rocket jas negative static margin."""


MateusStano · 2025-02-09T18:59:54Z

rocketpy/simulation/flight.py

+    def __validate_rocket_static_margin(self):
+        """
+        Avoids running a flight simulation if the rocket stability margin is
+        negative. This is a common mistake that can lead to unstable flights,
+        which usually runs indefinitely.
+        """
+        if (s := self.rocket.static_margin(0)) < 0:
+            raise UnstableRocketError(s)
+


I think this error check might not be suitable right now. I see a few points as to why:

If a user wants to simulate an unstable rocket, for any reason, we should not make that impossible. There could always be a rocket that has a negative static margin, but the stability margin could quickly become positive with fuel burn or other effects

When in monte carlo simulations, what would happen if one of the iterations has an unstable rocket? Will all the simulations break because of it?

The generic aerodynamic coefficients don't calculate static margin correctly. This essentially means that all rockets with custom coefficients will have a negative static margin.

Maybe raising a warning is the best we can do?

I agree with the first point you raised: not all rockets that are instantaneously unstable are necessarily unsimulatable.

A possible compromise would be either a warning or letting the user set a variable, such as ALLOW_UNSTABLE.

I like the Idea of having a flag with default to false (so current API is respected).

Then we can simply turn It on for infinity

Would ALLOW_UNSTABLE be by default True or False?

And if the default is False would that be a breaking change? Since someone who has a sim with an unstable rocket might start getting an error with this SM check

And if the default is True is it really helping the user that does not know about the negative static margin problem?

Sorry, I meant default true

Converting from an error to a warning is the easiest option right now.

However, I think we should better understand the cases where an unstable rocket can fly.
Perhaps the problem/bug happens inside the u_dot function (after rail exit) ?

If setting ALLOW_UNSTABLE to false, we should describe this behavior in the error message.
Setting ALLOW_UNSTABLE to true would not make much help.

I think setting It to true by default mantains current behavior while allowing clients to deliberatively switch.

IMHO a simple warning would not be easy to consume for safe guarding purposes

I understand.

My point is that maybe we should not keep the current behavior at all.

I will think more about this during the week

…ption`

…ons in Flight class

…gration tests

Gui-FernandesBR · 2025-02-19T00:06:25Z

Gotta think a better way of solving this problem.

Gui-FernandesBR requested a review from a team as a code owner February 8, 2025 16:58

github-actions bot assigned Gui-FernandesBR Feb 8, 2025

Gui-FernandesBR linked an issue Feb 8, 2025 that may be closed by this pull request

ENH: Custom Exception errors and messages #285

Open

Gui-FernandesBR force-pushed the enh/unstable-rocket-error branch from d64217b to 37c5505 Compare February 8, 2025 18:52

Gui-FernandesBR force-pushed the develop branch from 2931c30 to 7864590 Compare February 8, 2025 19:00

Gui-FernandesBR force-pushed the enh/unstable-rocket-error branch from 37c5505 to e48a442 Compare February 8, 2025 19:03

GabrielBarberini reviewed Feb 9, 2025

View reviewed changes

MateusStano reviewed Feb 9, 2025

View reviewed changes

Gui-FernandesBR force-pushed the enh/unstable-rocket-error branch from 05ba950 to 2793a62 Compare February 13, 2025 21:48

Gui-FernandesBR added 5 commits February 14, 2025 09:49

ENH: Add custom exceptions for RocketPy library - `UnstableRocketExce…

e914b4f

…ption`

ENH: Add static margin validation to prevent unstable rocket simulati…

7228980

…ons in Flight class

ENH: Update flight tests to avoid unstable rocket errors

269b1e6

TST: Increase soft tolerance for freestream speed test in Flight inte…

f5d2559

…gration tests

TST: Add test for UnstableRocketError in flight integration tests

04d2354

Gui-FernandesBR force-pushed the enh/unstable-rocket-error branch from 2793a62 to 04d2354 Compare February 14, 2025 12:49

Gui-FernandesBR closed this Feb 19, 2025



		class UnstableRocketError(RocketPyException):
		"""Raised when the rocket jas negative static margin."""

Uh oh!

ENH: unstable rocket error #764

ENH: unstable rocket error #764

Uh oh!

Conversation

Gui-FernandesBR commented Feb 8, 2025

Pull request type

Checklist

Current behavior

New behavior

Breaking change

Additional information

Uh oh!

codecov bot commented Feb 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

phmbressan Feb 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MateusStano Feb 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Gui-FernandesBR commented Feb 19, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

codecov bot commented Feb 8, 2025 •

edited

Loading

phmbressan Feb 9, 2025 •

edited

Loading

MateusStano Feb 9, 2025 •

edited

Loading