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

remove positive definiteness constraints, allow user defined additive inflation #360

Merged
merged 2 commits into from
Feb 6, 2024

Conversation

odunbar
Copy link
Collaborator

@odunbar odunbar commented Nov 18, 2023

Purpose

Closes #359

Content

  • Change all MvNormal(m,C) to m .+ sqrt(C) * MvNormal(0,I), which can handle semi-definite "C"
  • bugfix: Add rng into sample_empirical_gaussian
  • Remove DMC test for Sparse inversion as it no longer seems to converge. I believe this was likely unstable even in previous version of the code, stick with constant timesteppers
  • allow users to provide an additive inflation matrix, defaults to prior covariance. additive_inflation_cov = ... in update to provide this. This allows for users to break the subspace property
  • Unit tests for the new interface
  • updated the inflation docs page here

Testing

E.g in Examples/LorenzAccelerated/lorenz_accelerated.jl, on main the following breaks

update_ensemble!(...,additive_inflation=true, s=1e-2)

while it works on the current branch (with different choices of inflation matrix too)

update_ensemble!(...,additive_inflation=true, s=1e-2)
X = 0.1*I
update_ensemble!(...,additive_inflation=true, additive_inflation_cov = X, s=1e-2)

Interface Note

Before the inflation default was effectively to inflate using additive_inflation_cov = get_cov_u_final(ekp) now it defaults to the additive_inflation_cov = get_cov_u_prior(ekp). This is because you are likely not going to use the former situation - this is better completed using multiplicative inflation.


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

@odunbar odunbar changed the title get_u_cov always corrects positive definiteness [WIP] get_u_cov always corrects positive definiteness Nov 18, 2023
@odunbar odunbar force-pushed the orad/posdef-covs branch 3 times, most recently from 063726e to 532b7f0 Compare November 22, 2023 23:57
@odunbar odunbar changed the title [WIP] get_u_cov always corrects positive definiteness [WIP] remove positive definiteness constraints Nov 29, 2023
@odunbar odunbar force-pushed the orad/posdef-covs branch 2 times, most recently from 0d56bab to 76b83a0 Compare December 5, 2023 18:56
@odunbar odunbar requested review from eviatarbach and costachris and removed request for eviatarbach and costachris December 5, 2023 19:00
@odunbar odunbar changed the title [WIP] remove positive definiteness constraints remove positive definiteness constraints, allow user defined additive inflation Dec 6, 2023
!posdef fix

format

correct scaled matrix

typo

format

try without get_u_cov posdefs

re-add posdefs as old tests still fail

try making tests more recent

try 1.8 LTS

add pd correct after sqrt

remove sqrt, bugfix: add rng into sample empirical gaussian

LTS 1.6 revert

remove repeated call to posdef_correct

adapt sparse tests to pass

format

 removed DMC with sparse inversion

format

remove unstable case

allow user-defined inflation matrices

format

docs typo

codecov

format
Copy link
Contributor

@eviatarbach eviatarbach left a comment

Choose a reason for hiding this comment

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

LGTM! Thank you for working on this.

@odunbar odunbar merged commit b420d83 into main Feb 6, 2024
12 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.

Positive definite corrections in get_u_cov
2 participants