-
Notifications
You must be signed in to change notification settings - Fork 17
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
Timesteppers for EKP, from ParameterEstimocean #285
Conversation
Note tests failing due to upgrades to Julia v1.9.0 - they do pass in v1.8.5. This will be addressed shortly |
016c9f2
to
82a2827
Compare
cb6e42f
to
9958b4c
Compare
401a171
to
b6d32b9
Compare
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 work on this! I made a few comments in the code. I think the main issues are lack of tests for UKI and sparse EKI.
|
||
|
||
|
||
# build EKP and eki objects |
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.
I think we need tests for UKI and sparse EKI as well.
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.
I have added both sets of tests. The Sparse inversion tests are in the SparseInversion/ test directory.
Currently UKI tests with the DataMisfitController
do not appear to converge, I will investigate later what is going on.
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.
I've added UKI example in the docs too now, and here it seems to perform ok. I wonder if there is some quirk in the test setup that's giving it problems
Leaving UKI alone, as I have made some easier tests and it still seems to have poor performance when solving nonlinear problems. I suspect a bug rather than anything to do with schedulers (causes issues on fixed timestep testing too). Commenting out UKI test, to be uncommented when performance has been thoroughly assessed. I would like to merge this PR to introduce the new features, and have created a new issue #287 to investigate the UKI [The example + docs are still working and so I have left them. I have only removed the unit tests for schedulers+UKI for now.] |
12e2973
to
0c5a2e5
Compare
9d2615a
to
50aaff9
Compare
Looks great to me! Thank you for all your work on this. Not sure why a test is failing on Windows but I think this should be ready to merge after that passes. |
b8f779c
to
66397a5
Compare
bors try |
The windows test passed on rerun - I'll give it a couple more times to see if it is recurring |
tryBuild failed: |
loop branch for mutable timestepper added parameters to EKSStabilityTimestepper added DMC timestepper timestepper interface bugfixes format externalize dim-check error, bugfix stableEKS timestep Unscented consistent, and tests pass update Delt =... to timestepper = ... in unit tests` update example unit test for constructors format consistency wiith 1,9 (not-yet-refined) timestepper example with various plots Timestepper termination condition upheld docstrings unit tests loosen tol typo improved output example loosen tolerance cleaned up plots and experiment repeated for many runs format removed nothing # hide, and added more strings rename timestepper to scheduler added continue options for DMC run test longer preliminary docs for scheduler comparison example improve plot legend and docs typos etc examples/LearningRateSchedulers/compare_schedulers.jl move docs page out of examples subfolder move docs in contents modify conclusions API docstrings docs and example improvements added SEKI and UKI tests for schedulers (UKI fails), fixed dangerous pattern where G applied to untransformed coordinates format uki example and docs consistency with seeding for plots separate posterior vs optimize, add uki descriptions format typo easier tests for UKI, but it still fails extend num steps robust test UKI comments return nothing explicitly
4ea2d81
to
4ea03c6
Compare
bors try |
bors r+ |
tryBuild succeeded! The publicly hosted instance of bors-ng is deprecated and will go away soon. If you want to self-host your own instance, instructions are here. If you want to switch to GitHub's built-in merge queue, visit their help page. |
Build succeeded! The publicly hosted instance of bors-ng is deprecated and will go away soon. If you want to self-host your own instance, instructions are here. If you want to switch to GitHub's built-in merge queue, visit their help page. |
Coauthored with @eviatarbach
Purpose
Closes #253
Closes #154
Content
EnsembleKalmanProcess(...; Deltat = x, ...)
In favour of the new constructor
EnsembleKalmanProcess(...; scheduler = DefaultScheduler(x), ...)
and updated all unit tests to the latter.
LearningRateScheduler
type and Objects in backwards compatible way, by adding the keywordscheduler = ...
DefaultScheduler(k)
(always does stepk
unless user overrides at a given step),MutableScheduler()
(initially does step1
, until user overrides withk
, then doesk
thereafter until user next overrides)EKSStableScheduler()
- performs the stable EKS step calculation externally, in place of the original internal one.DataMisfitController()
- based on Iglesias Yan 2021's Bayesian tempering scheme.DefaultScheduler(1)
usually, andEKSStableScheduler
for EKS.Deltat= x
expressions in constructors of EKP object.calculate_timestep!
can output a flag if a termination condition is met, this will prevent further EKP updates (e.g. forDataMisfitController
on_terminate = stop
continue
orcontinue_fixed
to either terminate, continue, or continue with latest timestep fixed.Also Changed
G
transformed parameters internally, rather than using the parameter distributions to to return "constrained parameters" which is the proper way these should be used0.0
not\Delta t
. Allows for constructor cleanup and clean interface with new schedulers.Unscented{FT,IT}
types, not unparameterizedUnscented
types.Example experiment - explained more clearly in docs
Learning
(A,B)
fromf = exp(Asin(t) + B)
, from observing onlymax(f)
andmean(f)
Final model solutions, as well as the max and mean of the solution
left: black = true model (not the observed data)
right: black = observed truth, grey = mean/2*sd of true model observations
Individual experiment behaviour
mean ensemble member error to truth, (error), and mean ensemble member error to ensemble mean (spread).
Avoiding ensemble collapse => dashed and solid lines are similar.
Green - constant step 0.02 for 50 steps (finishes at T=1)
Orange - constant step 0.5 for 50 steps (finishes at T=25)
Red - adaptive step, usually used for EKS (finishes at T=10^13?)
Purple - adaptive DMC step terminates at 12 steps (finishes at T=1)
Brown - adaptive DMC step with
on_terminate = continue
(finishes at T = 10^15)Average convergence over 100 experiments
50 ensemble members and 50 iterations (DMC typically terminates ~14-20 iterations)
Method : DefaultScheduler{Float64}(0.5)
Final misfit: 0.011919044450605893
Final error : 0.027449304379020753
Final spread: 0.0031725385439674517
Method : DefaultScheduler{Float64}(0.02)
Final misfit: 1.0919040792305175
Final error : 0.09912866440799409
Final spread: 0.07396181868428042
Method : EKSStableScheduler{Float64}(1.0, 2.220446049250313e-16)
Final misfit: 5.648615687716918e-5
Final error : 0.03018464453288676
Final spread: 1.9601185362422715e-5
Method : DataMisfitController{...}(..., "stop")
Final misfit: 1.1677896425071728
Final error : 0.09817906335167692
Final spread: 0.08739534124484036
Method : DataMisfitController{...}(..., "continue")
Final misfit: 8.52533406253277e-17
Final error : 0.030283499635998162
Final spread: 5.974713527239014e-17
Conclusion?
These results imply that
0->1
, but is far more efficient with regards to step size0->1
both retain better spread in the ensemble than other options,