ENH: Flight simulation speed up

[Gui-FernandesBR]

Pull request type

• Code maintenance (refactoring, formatting, tests)

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

Simulation is taking too long to run!

New behavior

Read the commit history, one by one.

Some comments:

• The get_value_opt method can be twice as faster as the normal get_value method.
• After the implementation, I'm seeing a total simulation time of around 3s, while before we had a simulation time of ~4s. I will add my tests here soon enough.

Breaking change

• No

Additional information

I am still running some tests within this branch.

[Gui-FernandesBR]

Ok, here is more detailed comment regarding the performance of flight simulation.

• Everything was only possible due to cProfile and snakeviz, amazing tools!
• Pieces of code that we still need to improve:
  • Flight.u_dot_generalized: this method is at least x2 slower than the traditional u_dot method, still present. This is hard to speedup tho.
  • Function.set_source: it is calling the _check_user_input, which really slows down the whole Function.init
  • Function._check_user_input: I think it is time to discuss if we really need this method running as default with rocketpy. It is taking a significant time of simulation.
  • I would also suggest the discussion on the usage of np.arg_sort() method to sort the x_array in the Function class. This also consumes time in order to avoid problems in the Function data source.
  • Function.__mul__: I tried my best, but there is still a lot of if statements slowing this function. I'm open to suggestions here.
  • Function.get_value_opt: It is already very optimized, but it is not enough. Now this is even more crucial for the Flight class, this method is responsible for ~8% of simulation time.
• Is there any numerical integration scheme faster than LSODA? It seems to be taking around 60% of the simulation time. Plus it cannot be parallelized.
• I created a simple test to compare the simulation times before and after changes:

Before the improvements done in this PR:
========================================

All 20 simulations done, it took 95.40774 seconds
Mean time: 4.77039
Standard deviation: 0.71902

All 20 simulations done, it took 92.84953 seconds
Mean time: 4.64248
Standard deviation: 0.66002

After the improvements done in this PR:
=======================================

All 20 simulations done, it took 50.86210 seconds
Mean time: 2.54310
Standard deviation: 0.12359

All 20 simulations done, it took 50.47766 seconds
Mean time: 2.52388
Standard deviation: 0.10919

Finally, I think this one is ready for review now, @RocketPy-Team/code-owners

[Gui-FernandesBR]

This PR, if merged, should solve the #481 issue.

[Gui-FernandesBR]

Hey, I'm requesting new reviews from @MateusStano and @phmbressan !!
I believe we are closer to merging this PR now.

I hope you like it.

[MateusStano]

Pretty good!

Question: How much is the simulation sped up now?

[Gui-FernandesBR]

Pretty good!

Question: How much is the simulation sped up now?

Running 100 simulations with the Calisto rocket:

Before this PR:

• Mean: 5.24839 s
• Std deviation: 1.82163 s

After this PR:

• Mean: 1.45942 s
• Std deviation: 0.45128 s

Overall, code much faster now!
I even added a new if statement to force the code to use the old u_dot method in case the rocket motor is a SolidMotor.
I remember that we agreed with this in a previous meeting.
@MateusStano can you approve it again please?

Tests are not passing due to a problem of hdf5 in python3.8 running on macOS. We will need to discuss it this week.

[MateusStano]

I even added a new if statement to force the code to use the old u_dot method in case the rocket motor is a SolidMotor.

This might be breaking the tests btw

[Gui-FernandesBR]

I even added a new if statement to force the code to use the old u_dot method in case the rocket motor is a SolidMotor.

This might be breaking the tests btw

Yes, this is exactly what is breaking these tests.

The way I see there are 2 options:

• Revert this change, let it be implemented in a future PR.
• Change the tests values to accommodate these changes. Apparently, the results from u_dot and u_dot_generalized are not identical, although they give very similar results.

I started thinking about option 2, but IMO this PR is long enough already, I see option 1 as more beneficial for now.

[Gui-FernandesBR]

I believe the tests should be passing now.

@phmbressan @MateusStano I would appreciate a last approve/review so we can finally merge this one.
Moving forward I will be able to work on the Monte Carlo PR again.

[phmbressan]

Amazing work done here, simulations are much quicker.

I re-reviewed the PR and the whole "extend vs append" discussion and I think the implementation here pretty good.

[Gui-FernandesBR]

Hello, I'd like to request re-review @MateusStano or @phmbressan .

Everything seems to be working correctly now. Tests were only slightly changed in this PR, returned values are practically identical.