ENH: Callback function for collecting additional data from Monte Carlo sims

[emtee14]

trafficstars

Pull request type

• [x] 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 behaviour

Implementation for this feature suggestion #699 . At the moments users are limited to set variables when exporting data from Monte Carlo experiments.

New behaviour

Now a function can be passed to the MonteCarlo class which takes a Flight object as an argument and returns a dict to be appended to the default outputs.

Breaking change

• [x] No

Additional information

At the moment the implementation works but there is an existing issue with the RocketPyEncoder for writing the inputs to the .txt file as JSON strings. I've also made it check if the function overwrites existing variables though I feel this may not be necessary. I also had to fix how the previous results are imported as they are errors since text results can now be exported, see line 671.

[Gui-FernandesBR]

Thanks for opening the PR @emtee14 , we will get back to you after the EuRoC competition, when our development team will be more available.

[codecov[bot]]

Codecov Report

Attention: Patch coverage is 36.36364% with 14 lines in your changes missing coverage. Please review.

Project coverage is 75.99%. Comparing base (5856353) to head (e1a2061).

Files with missing lines	Patch %	Lines
rocketpy/simulation/monte_carlo.py	33.33%	12 Missing :warning:
rocketpy/prints/monte_carlo_prints.py	50.00%	2 Missing :warning:

Additional details and impacted files

@@             Coverage Diff             @@
##           develop     #702      +/-   ##
===========================================
- Coverage    76.08%   75.99%   -0.09%     
===========================================
  Files           95       95              
  Lines        10997    11015      +18     
===========================================
+ Hits          8367     8371       +4     
- Misses        2630     2644      +14     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[Lucas-Prates]

@emtee14 thank you very much for opening this PR! I have been assigned to help you out with this feature. Your description in #699 and the (nice) implementation here give me a good idea of what you want to achieve.

To be more thorough, could you provide me an example of how you want to use it? Possibly a snippet of the code creating the export_function and calling the MonteCarlo class. It would be even better if it is an actual (maybe simplified?) example of what you are doing, e.g. exporting the environmental data mentioned in #699. If you want to give an example that is easy to run, consider the monte_carlo_class_usage.ipynb notebook and assume that this snippet would replace the code in the cell that calls the MonteCarlo class.

This would ensure that the design is best suited for the goal, and we could also add it as an example in the Monte Carlo usage notebook.

[emtee14]

Hi yes, I'll make an example that can be added to the docs. Another one of the uses we've found for it is exporting sensor data which we then use to run on our flight computer emulator allowing us to test firmware.

[Lucas-Prates]

This PR is ready for review. Along with the callback implementation provided by @emtee14, some error checks and tests were implemented. I provided a first simplified example on how to use this feature in the end of monte_carlo_class_usage.ipynb notebook, but am eager to see the example of @emtee14. Of course, his example can be provided in another PR as well, so there is no need to rush with the example.

Note: for some reason, codecov is not reflecting the test suite on the most recent commits. I am pretty sure I have covered most implemented lines.

[Lucas-Prates]

have you thought about the possibility of using both export_list and export_function at the same time?

You mean using both in an example or replacing export_list functionality completely using export_function?

[Gui-FernandesBR]

have you thought about the possibility of using both export_list and export_function at the same time?

You mean using both in an example or replacing export_list functionality completely using export_function?

have you thought about what would happen if someone uses both the export_list and export_function arguments at the same time (i.e. simultaneously)? Would that be a problem?

[Lucas-Prates]

No problem at all! Indeed, the example I provided in the notebook uses both export_list and export_function simultaneously.

[Lucas-Prates]

Sorry for the delay, but this PR is ready for review again. The following changes were made:

1. The export_function argument had its name changed to data_collector;
2. data_collector is a now a dict of python callables, reflecting the idea that we have several distinct callback functions instead of a "big" callback function as implemented previously;
3. The check if computing the summary made sense by checking for a float was removed;
4. The refactoring allowed for the validation of the data_collector to be done in the initialization of the MonteCarlo class;
5. Docs and tests were updated accordingly.

[phmbressan]

Also, @Lucas-Prates could you add this PR to the CHANGELOG.md?