Enh/liquid motors optimization

[phmbressan]

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
• Other (please describe):

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?

The liquid motor simulations were accurate, but performed really poorly (20+ seconds to generate inertia tensor plots).

What is the new behavior?

The main focus is optimize the Tank code in order to speed up the implementation. The results are now in the sub 1 second range.

Does this introduce a breaking change?

• Yes
• No

Other information

Important docs: docs/notebooks/example_liquid, docs/notebooks/example_hybrid.

[review-notebook-app]

Check out this pull request on 

See visual diffs & provide feedback on Jupyter Notebooks.

---

Powered by ReviewNB

[Gui-FernandesBR]

Great one!

I'm a little bit out of what has been doing in the enh/liquid-motors branch later, therefore I'm letting my name out of the reviewers list for now.

[giovaniceotto]

Just waiting on #353 before reviewing this one, since this branch has the #353 branch already merged.

[MateusStano]

I get this warning when running example_liquid.ipynb

[MateusStano]

Btw @phmbressan you are gonna have to fix this someday heheh

[phmbressan]

I get this warning when running example_liquid.ipynb

@MateusStano
This is "expected" and handled in code: the invalid value occurs, but is substituted by a valid one. However the way it is currently done leave numpy warning traces of the invalid values.

This occurs due the fact that, in this notebook, the oxidizer tank gas_mass is zero. If you change it to a small value e.g. 0.1 the warning goes away. I am still thinking on a better solution, since I don't like the current approach (i.e. letting the calculations be made even if it returns an invalid value and substituting a posteriori), however for now it ensures correctness.

[giovaniceotto]

Great work @phmbressan. Loved how you made use of Function composition to get even the most complicated properties to carry on as array-defined Function instead of having to implement them as a function of time.

The complexity of the new motor classes is only possible thanks to great advances you and other collaborators made to the Function class. Most notably, Function composition and Function integrals solved most of our problems here.

I am very satisfied performance-wise. Made a couple of enhancement suggestions such as typo-fixes, docstring suggestions or minor optimizations. Take a quick look and ask for a new review so that I can accept this PR.

[giovaniceotto]

Any ideas why unrelated tests are failing?

[phmbressan]

Any ideas why unrelated tests are failing?

Please, see the comment on the original Liquid Motors PR.

[phmbressan]

I get this warning when running example_liquid.ipynb

@MateusStano

I "fixed" this on 5374a6e. The issue was that, for some reason, numpy considers 0/0 as an invalid value error and not a divide error (even though the error message is quite ambiguous).

Let me know what you think. The warning should not pop up anymore.

[MateusStano]

I get this warning when running example_liquid.ipynb

@MateusStano

I "fixed" this on 5374a6e. The issue was that, for some reason, numpy considers 0/0 as an invalid value error and not a divide error (even though the error message is quite ambiguous).

Let me know what you think. The warning should not pop up anymore.

Seems all good to me now!

[MateusStano]

This one is approved by me. Just don't forget to merge beta/v1.0.0 into enh/liquid-motors before merging this one, it is currently carrying over some changes from there.