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

Reproducing grudzien2020 #77

Merged
merged 61 commits into from
Aug 22, 2021
Merged

Reproducing grudzien2020 #77

merged 61 commits into from
Aug 22, 2021

Conversation

cgrudz
Copy link
Contributor

@cgrudz cgrudz commented Jul 15, 2021

Re #70 I'm not sure that my new commits made it into my old pull request -- if this is redundant we can kill this request. In any case, I have an updated example for you to take a look at.

cgrudz and others added 17 commits June 15, 2021 16:28
This will introduce the L96s model as a unique model in the colllection
so that we can integrate some special features here first as a test
case.  However, integration.py can be minimally adjusted to rigorously
include the Euler-Maruyama and four-stage Runge-Kutta schemes for SDEs
with additive noise with minimal changes.
Initial simplifications have been introduced to homogenize the code with
the rest of the repo.  The simple case of a Fourier truncation with p=1
is emphasized, as this is all that matters for order 2.0 convergence.
The general scheme discussed in the manuscript is not of particular
interest for DA so we neglect this implementation, included in that
manucript's repo.
Using the stock Jacobian implementation in the extras file, changing
normal sampler to the standard univariate normal of appropriate
dimension for broadcasting.  Yet to be run on test cases, tomorrow will
most likely put additional time developing the model and testing it for
the overal integration with DAPPER for merge / push tutorial worksheets.
This is a very rough draft, needs to be discussed in the following days
for how to integrate this into the DAPPER framework further
Co-authored-by: Patrick N. Raanes <[email protected]>
Co-authored-by: Patrick N. Raanes <[email protected]>
The initial results reproduce the basic conclusions of
Grudzien2020numerical as in the bib entry.  In short, we acheive
adequate performance with Ne=100, pert obs EnKF, no inflation or
localization when using a statistically robust SDE solver like
Runge-Kutta with dt=0.1, but we experience filter divergence
with the Euler-Maruyama scheme (also order 1.0) in the same setting.
Full analysis and a wider simulation range for parameters are in
the reference.
Needed to make a re-install of some components with updates in
Matplotlib, re-ran the update of the bib files to sync all changes.
Copy link
Collaborator

@patnr patnr left a comment

Choose a reason for hiding this comment

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

I will just close the other PR anyway.

Hint: you can apply the suggested changes (if you're fine with them) in the GitHub interface. Probably you want to add them to a single batch before committing (still in the GitHub interface). Then you can pull from origin to get those changes locally. Then you can add your own changes locally (for example to address comments without suggested changes) and push, which should update this PR.

dapper/mods/L96s/Grudzien2020.py Outdated Show resolved Hide resolved
dapper/mods/L96s/Grudzien2020.py Outdated Show resolved Hide resolved
dapper/mods/L96s/Grudzien2020.py Outdated Show resolved Hide resolved
dapper/mods/L96s/Grudzien2020.py Outdated Show resolved Hide resolved
dapper/mods/L96s/Grudzien2020.py Outdated Show resolved Hide resolved
dapper/mods/L96s/__init__.py Outdated Show resolved Hide resolved
dapper/mods/integration.py Outdated Show resolved Hide resolved
examples/perfect_random_model_example.py Outdated Show resolved Hide resolved
dapper/mods/integration.py Outdated Show resolved Hide resolved
dapper/mods/integration.py Outdated Show resolved Hide resolved
cgrudz and others added 2 commits July 16, 2021 10:08
@patnr
Copy link
Collaborator

patnr commented Jul 16, 2021

I have made the document generation automatic. So please remove all html files listed under "Conflicting files" right below. Also, there is a conflict in dapper/mods/integration.py since I just made some changes there. In order to resolve, pull from upstream, fix conflicts, and push to origin (which will automatically update this PR).

cgrudz and others added 3 commits July 16, 2021 10:10
@cgrudz
Copy link
Contributor Author

cgrudz commented Jul 16, 2021

I have made the document generation automatic. So please remove all html files listed under "Conflicting files" right below. Also, there is a conflict in dapper/mods/integration.py since I just made some changes there. In order to resolve, pull from upstream, fix conflicts, and push to origin (which will automatically update this PR).

Nice, will do. I have plans to finish off some of this merger today, along with hopefully having time after meetings to build some exercises and further examples. Take a look at the example if you got a chance -- the simulation is looking good, confirming the results of the GMD work.

@patnr
Copy link
Collaborator

patnr commented Jul 16, 2021

Noice!

dpr

@cgrudz
Copy link
Contributor Author

cgrudz commented Jul 16, 2021

Yeah, I think it should be fun to play with some of these time series, especially tuning the amount of stochasticity. It begins to look more and more like a Brownian motion with enough noise, or more like the standard Lorenz 96 in the small noise, there should be some nice exercises to look at in an Jupyter notebook.

Merging with Patrick's integrator changes
@cgrudz
Copy link
Contributor Author

cgrudz commented Jul 16, 2021

Sorry my bad, incorrectly was merging changes. Let's see how the new pushes look.

@patnr
Copy link
Collaborator

patnr commented Jul 16, 2021

The tests are crashing coz it still says L96s in grudzien2020

I'm offline rest of the evening! Will check in tomorrow

cgrudz and others added 2 commits July 16, 2021 12:45
Co-authored-by: Patrick N. Raanes <[email protected]>
Some notations for the ensemble and state dimensions didn't match post
merger, these are now fixed along with references in the example.
@patnr
Copy link
Collaborator

patnr commented Jul 25, 2021

Sorry, I am on holiday and am not active. How's this looking? AFAICT there's still some suggested changes and comments have not been addressed.

@cgrudz
Copy link
Contributor Author

cgrudz commented Jul 25, 2021 via email

__init__.py has been edited mostly for style changes, grudzien example
has been edited but remains somewhat incomplete -- will push additional
sections including mutiple stages for the example as I have a chance to
add some more docs and a chance to play with the visualizations further,
itegration has been modified to include more information in the doc
string with a full stage / order of convergence table.
dapper/mods/integration.py Outdated Show resolved Hide resolved
@patnr
Copy link
Collaborator

patnr commented Jul 27, 2021

Apart from the suggested changes in rk4 everything looks great now.

The interactive plot is set for a toy setting and parameter range is now
set in order to reproduce the reference.  Plotting is incomplete but it
seems like the results for the Euler-Maruyama are much more pessimistic
than in the reference when using a low-precision setting in a noise
driven dynamical system.  This may need additional debugging in
addition to finishing the plotting scripts.
@patnr
Copy link
Collaborator

patnr commented Jul 28, 2021

I just upgraded the documentation for setup quite a bit. Please have a look.

Also added to bib.py, unfortunately causing conflict. Please pull, resolve, push.

I also think you should mainly build on basic_2, not basic_3. Because although basic_3 illustrates the use of setup it also contains other complications. So start with basic_2. Use a simple for-loop to generate several xps, not the methods shown in basic_3. Sorry I don't have time right now to look in more detail.

@cgrudz
Copy link
Contributor Author

cgrudz commented Jul 28, 2021 via email

@patnr
Copy link
Collaborator

patnr commented Jul 28, 2021

Yeah, it just seems to me like there might be some confusion still, and it is much easier to consult on this when the script follows the template of basic_2, without the complications of basic_3. Maybe just stash the plotting stuff for later?

cgrudz added 2 commits July 30, 2021 08:42
Need to work on plotting scripts still, but the data generation based on
combinator is working correctly now and has fully reproduced the results
of the reference.   This has now been added in the README along with a
few additional references that I think are worth mentioning.  Will work
on plotting scripts next week more than likely.
@cgrudz
Copy link
Contributor Author

cgrudz commented Jul 30, 2021 via email

cgrudz and others added 3 commits August 9, 2021 11:46
Initial commit to remove the table, will edit description slightly more shortly.

Co-authored-by: Patrick N. Raanes <[email protected]>
Note, I'm not sure if the two or three stage Runge-Kutta schemes
extend the same way for SDEs as they do for deterministic systems,
so for lack of a better answer I only specify one stage and four stage
options in the description.
@cgrudz
Copy link
Contributor Author

cgrudz commented Aug 9, 2021

Will probably spend some time on plotting scripts later this week, minor changes have addressed items above. If you have any extra docs or comments to push to the plotting example based on basic 3, let me know, otherwise I'll just try tinkering with this more.

@cgrudz
Copy link
Contributor Author

cgrudz commented Aug 19, 2021

In fact, I'm getting bogged down a bit on the plotting code, I'm thinking to remove the draft that I have currently, push the change and go for a merger of branches for now. Does that sound good? I'll get back to this again shortly but I'll need a bit of lead time as I'm prepping for the semester.

Will redo some plotting of this later, for now the results are fully
reproduced and I want to finish the initial merger of the additional
functionality into the upstream branch.
@patnr patnr changed the title Updated Pull Request Reproducing grudzien2020 Aug 22, 2021
@patnr patnr merged commit fd8c134 into nansencenter:master Aug 22, 2021
@patnr patnr mentioned this pull request Aug 22, 2021
patnr added a commit that referenced this pull request Aug 31, 2021
patnr added a commit that referenced this pull request Sep 5, 2021
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