-
Notifications
You must be signed in to change notification settings - Fork 61
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
Feature/bokeh mc bringup #835
Merged
Merged
Conversation
This file contains 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
Natsoulas
force-pushed
the
feature/bokeh-mc-bringup
branch
3 times, most recently
from
October 22, 2024 07:36
4db9f2b
to
c8aa8b2
Compare
Natsoulas
added
documentation
Improvements or additions to documentation
enhancement
New feature or request
labels
Oct 22, 2024
Natsoulas
force-pushed
the
feature/bokeh-mc-bringup
branch
3 times, most recently
from
October 22, 2024 18:56
c9e9a85
to
0dab0f3
Compare
Natsoulas
force-pushed
the
feature/bokeh-mc-bringup
branch
7 times, most recently
from
November 5, 2024 02:59
7747f4d
to
f11f61e
Compare
Natsoulas
force-pushed
the
feature/bokeh-mc-bringup
branch
from
November 5, 2024 05:44
8c90e95
to
d74495b
Compare
schaubh
requested changes
Nov 6, 2024
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.
Great changes. This branch is now much easier to review and check. Good job. Just some small edits to make and then we are good to go!
Natsoulas
force-pushed
the
feature/bokeh-mc-bringup
branch
from
November 6, 2024 20:43
d74495b
to
0e33f5f
Compare
Natsoulas
force-pushed
the
feature/bokeh-mc-bringup
branch
2 times, most recently
from
November 7, 2024 00:54
39bedf1
to
95a7224
Compare
schaubh
approved these changes
Nov 7, 2024
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.
Good to go.
Natsoulas
force-pushed
the
feature/bokeh-mc-bringup
branch
from
November 7, 2024 01:14
95a7224
to
88157a1
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This branch introduces significant improvements to the Monte Carlo plotting capabilities in Basilisk, focusing on enhancing the
AnalysisBaseClass.py
file and related example scenarios. The key changes include:The
MonteCarloPlotter
class:Bokeh Integration:
User Interface Improvements:
Documentation and File Management:
Example Scenario Updates:
scenarioMonteCarloAttRW.py
to showcase the new plotting capabilitiesscenarioRerunMonteCarlo.py
for demonstrating how to rerun specific Monte Carlo simulationsTesting and Compatibility:
test_bskMcTestScript.py
to accommodate the new featuresThese changes significantly improve the visualization and analysis capabilities for Monte Carlo simulations in Basilisk, providing users with more interactive and informative tools to explore simulation results.
Verification
The changes were validated through the following methods:
Manual Testing:
MonteCarloPlotter
class functionalityAutomated Tests:
test_bskMcTestScript.py
to cover new features and ensure backward compatibilityMonteCarloPlotter
class to verify data loading, plot creation, and interactive updatesExample Scenario Testing:
scenarioMonteCarloAttRW.py
to showcase new plotting capabilitiesscenarioRerunMonteCarlo.py
to ensure proper functionality for rerunning specific Monte Carlo simulationsPerformance Testing:
Documentation Review:
No tests were removed or re-baselined. The addition and update of tests were necessary to cover the new functionality and ensure the reliability of the Monte Carlo plotting capabilities.
Documentation
The following documentation has been affected by these changes and should be reviewed for accuracy and completeness:
Docstrings in modified files:
scenarioVisualizeMonteCarlo.py
: Revised docstrings to reflect new command-line options and usage of updated plotting capabilities (bokeh) and this along with the other files was renamed from scenarioAnalyzeMonteCarlo.pyscenarioMonteCarloAttRW.py
: Revised docstrings to reflect new command-line options and usage of updated plotting capabilitiesscenarioRerunMonteCarlo.py
: New file with comprehensive docstrings explaining the purpose and usage of rerunning specific Monte Carlo simulationsoptional-requirements.txt
:Reviewers should pay particular attention to:
Additionally, reviewers should ensure that the addition of Bokeh and Dask to the optional requirements is properly documented in any relevant README files or user guides.
Future work
Further optimization of performance for large datasets:
Refinement of Bokeh plotting tools:
Expansion of rerun capabilities:
scenarioRerunMonteCarlo.py
to support more complex rerun scenariosIntegration with other Basilisk modules:
Documentation and tutorials???:
Note: These future work items are based on anticipated needs and potential improvements. Priorities may be adjusted based on actual user feedback and project requirements.