Skip to content
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

Sinusoidal example #287

Merged
merged 4 commits into from
Mar 22, 2024
Merged

Sinusoidal example #287

merged 4 commits into from
Mar 22, 2024

Conversation

lm2612
Copy link
Collaborator

@lm2612 lm2612 commented Mar 5, 2024

This will close issue #283 on reproducibility of sinusoid example

Content

  • random number generator rng saved and re-opened between each script (calibrate.jl, emulate.jl, sample.jl) so that running the example scripts gives same results as running the tutorial. This changed some of the plots and numbers.
  • Missing imports Statistics and Distributions added
  • Emulator had some missing definitions (init_sample, input_dim, ...) and typo emulator->emulator_gp.
    See preview here

  • I have read and checked the items on the review checklist.

…usoidal example. This example is 2d->2d for a model of a sinusoid that depends on 2 parameters (amplitude, vertical shift) and given a system where we make noisy observations of its range and mean position.

New files in example:
    * Setup script (functions defined for the true model)
    * Calibrate (using ensemble kalman inversion)
    * Emulate  (using GP, RF and emulator validation plots)
    * Sample (with mcmc and plotting 2d posteriors).

New docs under page Simple example walkthrough: a walkthrough of these scripts with plots for each step.
@lm2612 lm2612 self-assigned this Mar 6, 2024
Copy link

codecov bot commented Mar 6, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.26%. Comparing base (a621a04) to head (f7ea64d).
Report is 20 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #287      +/-   ##
==========================================
+ Coverage   88.09%   88.26%   +0.16%     
==========================================
  Files           7        8       +1     
  Lines        1134     1184      +50     
==========================================
+ Hits          999     1045      +46     
- Misses        135      139       +4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

… in src/examples/Sinusoid/, so now the docs page can be run and is reproducible. This also changed a few numbers slightly that were initially run through the examples pages rather than the docs pages.

* `rng` is now saved into `.jld2` files at the end of the example scripts and then opened at the start of the next script, so entire workflow agrees with results of running snippets from the docs pages.
* I increased the nugget term from 1e-12 to 1e-9 to help stability and consistency.
* Added missing imports Statistics and Distributions in docs and in example file.
* `emulator` -> `emulator_gp` in docs page
* defined `init_sample` and other variables that go into the random features emulator in docs page.
Copy link
Collaborator

@odunbar odunbar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you remove the rng stuff in the documentation page. It's nice to have it in the examples/Sinusoid files for reproducibility, but I personally think it just adds mess to the docs explanation.

Once this is done, LGTM!
Althought the PR seems to be logging a lot of changes on main as coming from this PR - perhaps that will update when the PR is no longer a draft...

@lm2612 lm2612 marked this pull request as ready for review March 22, 2024 18:41
@lm2612 lm2612 merged commit 87c069c into main Mar 22, 2024
9 of 10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants