ENH: Callback function for collecting additional data from Monte Carlo sims by emtee14 · Pull Request #702 · RocketPy-Team/RocketPy

@emtee14

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.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

  • 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 emtee14 changed the base branch from master to develop

October 6, 2024 21:18

@Gui-FernandesBR

…xisting functions for compatability

@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

@Lucas-Prates Lucas-Prates linked an issue

Nov 5, 2024

that may be closed by this pull request

@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.

@Lucas-Prates

@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

@Lucas-Prates

… any kind (attribute, logical, etc)

@Lucas-Prates

@Lucas-Prates

@Lucas-Prates

@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.

Gui-FernandesBR

Gui-FernandesBR

Gui-FernandesBR

Gui-FernandesBR

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?

@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

phmbressan

@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

@phmbressan

@Lucas-Prates