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

Sinusoid example not fully reproducible #283

Closed
4 tasks done
matt-graham opened this issue Feb 27, 2024 · 6 comments
Closed
4 tasks done

Sinusoid example not fully reproducible #283

matt-graham opened this issue Feb 27, 2024 · 6 comments
Assignees

Comments

@matt-graham
Copy link

matt-graham commented Feb 27, 2024

Raising as part of JOSS review openjournals/joss-reviews/issues/6372

The sinusoid example in the documentation https://clima.github.io/CalibrateEmulateSample.jl/dev/examples/sinusoid_example/ (and partially included in the JOSS paper), is not fully reproducible. I think this is at least in part due to the use of unseeded random number generators when sampling the observation noise for the simulated observations in

white_noise = MvNormal(zeros(dim_output), Γ)
y_obs = [y1_true, y2_true] .+ rand(white_noise)

and when simulating the Markov chain in
mcmc = MCMCWrapper(RWMHSampling(), y_obs, prior, emulator; init_params = init_sample)
# First let's run a short chain to determine a good step size
new_step = optimize_stepsize(mcmc; init_stepsize = 0.1, N = 2000, discard_initial = 0)
# Now begin the actual MCMC
println("Begin MCMC - with step size ", new_step)
chain = MarkovChainMonteCarlo.sample(mcmc, 100_000; stepsize = new_step, discard_initial = 2_000)

so that we get different simulated observations from the values mentioned in the tutorial and different Markov chain samples.

There are also a few other small issues which mean the tutorial isn't runnable as is / a complete self-contained example

  • Missing imports - for example Statistics for the use of mean and Distributions for the use of MvNormal.
  • emulator variable passed to MCMCWrapper not defined (presumably one of emulator_gp or emulator_random_features should be aliased to this?)
  • init_sample variable passed as init_params argument to MCMCWrapper is not defined
  • In the random features example there are lots of undefined variables - nugget, n_features, input_dim, output_dim (presumably the last two might map to dim_params and dim_output)
@lm2612 lm2612 self-assigned this Mar 5, 2024
lm2612 added a commit that referenced this issue Mar 5, 2024
* Copied over code that tracks random number generator `rng` that was 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.
* Added missing imports Statistics and Distributions in docs and in example file.
* `emulator` -> `emulator_gp`
* defined `init_sample`
* defined variables that go into the random features emulator
@lm2612 lm2612 mentioned this issue Mar 5, 2024
1 task
@odunbar
Copy link
Collaborator

odunbar commented Mar 6, 2024

Hi @matt-graham, thanks for raising these issues!

I'd like to ask was if it clear that the example on our docs page is simply documenting code within examples/Sinusoid on the github? I find that our example code here can be run completely in a self-contained manner, and reproduces all plots etc.

For example

We do want the docs to be good teaching points, and be instructive, but I would say the walkthrough, though featuring snippets, is not intended to be used to reproduce the example, whereas the code in the examples directory is intended to reproduce everything (inc. plots etc).

In light of this I think perhaps we should also add the item

  • Add a clear link from "documentation of example" -> "full code of example" at the top of example documentation page.

What do you think?

@nluetts
Copy link

nluetts commented Mar 16, 2024

Hi 👋

I am replying to this also in the context of the JOSS review openjournals/joss-reviews#6372

When I looked at your Sinusoid example in the docs, it was in fact not immediately clear to me that it was intended to describe the Sinusoid example in your examples folder, because I was looking at the docs first and the repository only a bit later, and there seems to be no mention of the examples folder in the respective section in the docs.
So I think a link in the docs plus an explicit statement about the examples folder would be very helpful indeed.

@matt-graham
Copy link
Author

Apologies for the delay replying - here. I agree with @nluetts that having a link to the full code in the documentation would be useful - I also missed the examples folder initially.

@odunbar
Copy link
Collaborator

odunbar commented Mar 22, 2024

@nluetts @matt-graham

The latest docs here

I believe we have cleared up some reproducibility in the example file (PR287) and also a link at the top from the docs page to the examples (PR290) let us know if you want more done here.

@nluetts
Copy link

nluetts commented Mar 26, 2024

The info box in the docs is perfect, now this can hardly be overlooked.

@matt-graham
Copy link
Author

@odunbar thanks - #287 and #290 address all my points so closing this.

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

No branches or pull requests

4 participants