ENH: Callback function for collecting additional data from Monte Carlo sims by emtee14 · Pull Request #702 · RocketPy-Team/RocketPy
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.mdhas 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
- 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.
emtee14
changed the base branch from
master
to
develop
Thanks for opening the PR @emtee14 , we will get back to you after the EuRoC competition, when our development team will be more available.
@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.
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.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
have you thought about the possibility of using both export_list and export_function at the same time?
have you thought about the possibility of using both
export_listandexport_functionat the same time?
You mean using both in an example or replacing export_list functionality completely using export_function?
have you thought about the possibility of using both
export_listandexport_functionat the same time?You mean using both in an example or replacing
export_listfunctionality completely usingexport_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?
No problem at all! Indeed, the example I provided in the notebook uses both export_list and export_function simultaneously.
Sorry for the delay, but this PR is ready for review again. The following changes were made:
- The
export_functionargument had its name changed todata_collector; data_collectoris a now adictof pythoncallables, reflecting the idea that we have several distinct callback functions instead of a "big" callback function as implemented previously;- The check if computing the summary made sense by checking for a
floatwas removed; - The refactoring allowed for the validation of the
data_collectorto be done in the initialization of theMonteCarloclass; - Docs and tests were updated accordingly.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters