-
-
Notifications
You must be signed in to change notification settings - Fork 167
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ENH: Callback function for collecting additional data from Monte Carlo sims #702
ENH: Callback function for collecting additional data from Monte Carlo sims #702
Conversation
…xisting functions for compatability
565cebd
to
a25c2ee
Compare
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 ReportAttention: Patch coverage is
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 ☔ View full report in Codecov by Sentry. |
@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 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. |
…ates/impact_state)
… any kind (attribute, logical, etc)
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. |
There was a problem hiding this comment.
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?
You mean using both in an example or replacing |
have you thought about what would happen if someone uses both the |
No problem at all! Indeed, the example I provided in the notebook uses both |
Sorry for the delay, but this PR is ready for review again. The following changes were made:
|
Also, @Lucas-Prates could you add this PR to the |
Pull request type
Checklist
black rocketpy/ tests/
) has passed locallypytest tests -m slow --runslow
) have passed locallyCHANGELOG.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
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.