Modify how to load prior distributions from edmf data #199
Conversation
There was a problem hiding this comment.
- Can we remove the manifest completely? I don't like having them hanging around. (Or is there a reason we had one in this example? I can't recall)
- Is the difference in the EDMF data just the way it's saved? Otherwise for reference could you post the resulting plot you get from the new data on your PR message. Thanks!
I don't know why we had Manifest for this example, I have deleted it. The EDMF data has also changed (as I don't know exactly how Ignacio generated the data), and I have updated the commit message with the plot. |
Manifest files are useful for reproducibility |
Ah, yes, I did have problems with jld2 compatibility when using a different version of Julia. |
|
Oh definitely, I get the general use for Manifests. But we have had many more compatatbility issues trying to maintain them, when we had them fixed on GH, than reproducibility issues since we have removed them. The JLD2 could also be sorted with a compat? Or by saving the data with a newer version of JLD2? |
Codecov ReportBase: 88.65% // Head: 88.65% // No change to project coverage 👍
Additional details and impacted files@@ Coverage Diff @@
## main #199 +/- ##
=======================================
Coverage 88.65% 88.65%
=======================================
Files 4 4
Lines 388 388
=======================================
Hits 344 344
Misses 44 44
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
|
@szy21 when you changed the JLD2 stuff, what did you do about the other dataset - the 5 dimensional one? Is it still compatible? Edit - I just saw your PR comment. I would like to keep the 5D one or have a replacement please! |
|
@odunbar I updated the 5 parameter calibration data. The results are in the PR message. The parameters do not seem to be well constrained, but that's as expected I think. |
There was a problem hiding this comment.
@szy21 Thanks!
As discussed, the example data is not ideal - however, CES seems to be working fine on it. To make plots smoother we can gather more samples:
- Could you change (and ideally rerun) L193 to
chain = MarkovChainMonteCarlo.sample(mcmc, 200_000; stepsize = new_step, discard_initial = 10_000) - Could you double (or measure) the approx run-time in the README.md
After this, merge is good!
|
bors r+ |
|
Build succeeded: |
Purpose
This PR modifies how prior distributions are loaded to be consistent with recent changes in CalibrateEDMF. It also updates the EDMF data in
ent-det-calibration.zipandent-det-tked-tkee-stab-calibration.Example output
exp_name = ent-det-calibrationexp_name = ent-det-tked-tkee-stab-calibration