Bug: Issue 115 static margin stability

[Lucas-KB]

Pull request type

Please check the type of change your PR introduces:

• Code base additions (bugfix, features)
• Code maintenance (refactoring, formatting, renaming, tests)
• ReadMe, Docs and GitHub maintenance

Pull request checklist

Please check if your PR fulfills the following requirements, depending on the type of PR:

• Code base additions (for bug fixes / features):

  • Tests for the changes have been added
  • Docs have been reviewed and added / updated if needed
  • Lint (black rocketpy) has passed locally and any fixes were made
  • All tests (pytest --runslow) have passed locally

What is the current behavior?

Negative static margins result in stable flights (see issue #115).

What is the new behavior?

RocketPy will no longer support airfoiled fins (for now) and the changes in the pull request #47 were removed. The stability calculations work as expected now. See the image below

Does this introduce a breaking change?

• Yes
• No

Simulations considering airfoiled fins will no longer work (until it is reimplemented).

[Gui-FernandesBR]

Is the automatic tests already working properly, @giovaniceotto ? Or do you need more time to finnish it?

[Lucas-KB]

I have implemented the tests me, @giovaniceotto, @FranzYuri, and @lucasfourier discussed yesterday. I was just unsure on what to check when the static margin is zero, so I just didn't check anything, but feel free to suggest alternatives and comment on my solution.

[giovaniceotto]

@Lucas-KB, would you like to keep the stability_check.ipynb file or can we remove it?

[giovaniceotto]

@Lucas-KB, I have added the following changes:

• Refactored the test implementation using pytest parametrize so that it is shorter and easier to understand.
• Checked if M1 or M2 were very small (< 1e-10 in abs value) in case the static margin was zero.
• Fixed an error where the angle of attack appeared twice in the lift calculation (probably a typo when reverting changes).

Found the last one when checking conflicts before merging (phew...). Now, the plots look much better. In the near future, I would say we need to implement tests that check the natural frequency and damping coefficient, so that errors like these are also caught. However, as the bug we are solving is quite important, I believe we need to merge this PR asap and leave that for another one.

Feel free to delete or keep the stability_check.ipynb file, as you wish, and then merge this PR as soon as @Gui-FernandesBR reviews it!

(By the way, fantastic work!)
(Man... how annoying is the 2 ** 2 vs. 2**2 change due to black when review code... 😠)

[Lucas-KB]

Nice work @giovaniceotto really more organized. About the stability_check.ipynb file, I'm not sure if we should keep. I may remove it later today (if someone else wants to, feel free).

(Man... how annoying is the 2 ** 2 vs. 2**2 change due to black when review code... 😠)

Yes, very annoying, it is very hard to review changes because most of them are just this random thing. Could you nail down the reason for this to happen?

[Gui-FernandesBR]

It's fine by me,

but please let's wait 1 day until merging because Stano asked us to merge # 122 first, I believe there will be no problem.

Regarding the stability_check, I deleted from this PR but uploaded on cloud so we don't loose that masterpiece