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

NICE sample-error correction #367

Merged
merged 16 commits into from
May 9, 2024
Merged

NICE sample-error correction #367

merged 16 commits into from
May 9, 2024

Conversation

odunbar
Copy link
Collaborator

@odunbar odunbar commented Feb 13, 2024

Purpose

Implements the NICE sample-error correction (Morzfeld, Vishny, et al 2024)

Content

  • added the Localizer: SECNice() with a couple of constructors SECNice(), SECNice(delta_ug, delta_gg) and SECNice(n_samples, delta_ug, delta_gg) n_samples are the number of samples to approximate the standard deviation through Fisher transformation and deltas are "fudge factors" on the discrepancy principle tolerance
  • added this to the example Localization/localization_example_lorenz96.jl
  • docstrings and API for variable parameters
  • Added L96 example to docs page, prettified plots and add recommendation for SECNice here
  • unit tests
  • Improved implementation efficiency for calc correlations, based on creating an interpolant on first call. NB this requires adding Interpolations into the Project toml (though we could do without if necessary)
  • added a branch for N_ens<=6 for a crude sampling scheme, (still performant on the lorenz example even with 5 particles)

Examples

Implement Vishny, Morzfeld, 2024 localization. Using Fisher transformation to avoid lookup tables,
Instead we create a linear interpolation of the std of correlation space on first call, which is reused in future calls.

There is a gentle decline in performance as the iteration is decreased down to ~20 samples

Example: learning a 200dim state x(t) from a future time of x(t+T) where T=1, using 20 ensemble members.
SECNice(n_samples, delta_ug, delta_gg)

Result - SECNice(5000,1.0,1.0) and SECNice(5000,0.8,1.0)

Result - SECNice(1000,1.0,1.0) and SECNice(1000,0.8,1.0)

Results for even smaller samples = 200,50,20


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

@odunbar odunbar changed the title [WIP] Orad/nice localizer [WIP] NICE sample-error correction Feb 13, 2024
@odunbar odunbar changed the title [WIP] NICE sample-error correction NICE sample-error correction Apr 30, 2024
index fix

bugfix

formatting

plots comparing to inflation-only

docstrings

docstrings in API docs

improve docs example

code for docs example

basic SECNice test

localizers

format

link fix
@odunbar odunbar mentioned this pull request May 1, 2024
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.

This is great, thank you for all the work on this Ollie! I suggested some changes as comments. The most substantive are:

  1. What should we do for N_ens < 6? Right now a warning is issued, but it seems to me that it may be more useful to hard-code a look-up table for this case.
  2. Shouldn't corr_psd = corr_tmp .^ α_min_exceeded[1] be corr_psd = corr_tmp .^ (α_min_exceeded[1] + 1), and similar for corr_psd_prev? Please check if I got it wrong.

docs/src/localization.md Outdated Show resolved Hide resolved
examples/Sinusoid/Project.toml Outdated Show resolved Hide resolved
src/Localizers.jl Outdated Show resolved Hide resolved
src/Localizers.jl Outdated Show resolved Hide resolved
src/Localizers.jl Outdated Show resolved Hide resolved
src/Localizers.jl Outdated Show resolved Hide resolved
src/Localizers.jl Outdated Show resolved Hide resolved
src/Localizers.jl Outdated Show resolved Hide resolved
src/Localizers.jl Outdated Show resolved Hide resolved
src/Localizers.jl Outdated Show resolved Hide resolved
src/Localizers.jl Outdated Show resolved Hide resolved
@odunbar odunbar requested a review from eviatarbach May 8, 2024 22:39
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.

Looks good, thank you for making these changes! I only made two minor additional comments.

src/Localizers.jl Outdated Show resolved Hide resolved
src/Localizers.jl Outdated Show resolved Hide resolved
@odunbar odunbar requested a review from eviatarbach May 9, 2024 17:31
@odunbar
Copy link
Collaborator Author

odunbar commented May 9, 2024

Offline changes were approved

@odunbar odunbar merged commit fb45f75 into main May 9, 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.

2 participants