-
Notifications
You must be signed in to change notification settings - Fork 16
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
CES Lorenz example for JOSS paper #250
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #250 +/- ##
==========================================
+ Coverage 88.09% 88.16% +0.07%
==========================================
Files 7 7
Lines 1134 1124 -10
==========================================
- Hits 999 991 -8
+ Misses 135 133 -2 ☔ View full report in Codecov by Sentry. |
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.
Thanks for the work! Looking good so far, I had a few small comments and this main point:
- I feel the walkthrouh is unfinished, could you add the points detailed below, such as adding snippets for building of machine learning tools / emulators, and building and running of MCMC? Including the "get_posterior" call to obtain the results from the chain object.
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.
Thanks for the update! This looks much better! The main points of this review:
- There is still a gap where we jump from defining the prior, to defining the emulator, perhaps it would be good to discuss (briefly) EKI in between and move up the calibration results to come before the discussion of emulators
- You can be a bit more judicious with the snippets, some have unnecessary lines (e.g.
savefig
orprintln
statements, while other snippets mention items that aren't explained (e.g.optimizer_options = overrides
inRF-scalar
)
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.
Awesome! Thanks for all the work.
LGTM
docs build docs build docs build docs build some updates to Lorenz example documentation commit before rebase removed manifest.toml files added results figures to docs added results figures to docs remove manifest finished description of emulate_sample.jl in docs quick format fix quick typo fix reorganized to more clearly explain the calibrate and emulate_sample stages small typo on file title
NB I have rebased and force-pushed to enable merge into main. No other changes have been made |
Purpose
Closes #255
Content
Docs can be found here.