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

Use one single model module #22

Closed
4 tasks
rabernat opened this issue May 3, 2022 · 2 comments
Closed
4 tasks

Use one single model module #22

rabernat opened this issue May 3, 2022 · 2 comments
Assignees

Comments

@rabernat
Copy link
Contributor

rabernat commented May 3, 2022

The original L96_model.py file has been copied between every different section. This is a potential source of errors and inconsistencies. Here is a list of all of the modules in the repo

./01Intro/L96_model.py
./02type-of-parametrization/L96_model.py
./03Data-Assimilation/DA_methods.py
./03Data-Assimilation/L96_model.py
./04Subgrid-parametrization-pytorch/L96_model_XYtend.py
./05Offline-DA-increments/DA_methods.py
./05Offline-DA-increments/L96_model.py
./06Different-ML-models/L96_model.py
./06Different-ML-models/L96_model_XYtend.py
./07-Interpretability-of-ML/L96_model_XYtend.py
./07-Interpretability-of-ML/LRP_gradient.py
./08-Implementation/L96_model_XYtend.py

We should try to consolidate around a single file.

Steps:

  • Understand the differences between the different L96_model modules in the different sections
  • Create on single "best" version
  • Move everything into a single directory (notebooks and modules)
  • Refactor all the notebooks to use the same model file

Furthermore, we should try to eliminate the use of modules for anything other than L96 itself. This means L96_model_XYtend.py, LRP_gradient.py, DA_methods.py: all of this should be in notebooks, not modules.

@adcroft
Copy link
Contributor

adcroft commented May 3, 2022

Some notes for upcoming PR...

There are three versions of the L96 python module:

$ find . -name "L96*.py" | xargs md5sum | sort                                                                                                       
01fb2a1bb99e26d2a8785d7f30b8e368  ./04Subgrid-parametrization-pytorch/L96_model_XYtend.py
01fb2a1bb99e26d2a8785d7f30b8e368  ./06Different-ML-models/L96_model_XYtend.py
01fb2a1bb99e26d2a8785d7f30b8e368  ./07-Interpretability-of-ML/L96_model_XYtend.py
b527fe57fdd668b473c795d18ecf6519  ./01Intro/L96_model.py
b527fe57fdd668b473c795d18ecf6519  ./02type-of-parametrization/L96_model.py
b527fe57fdd668b473c795d18ecf6519  ./03Data-Assimilation/L96_model.py
b527fe57fdd668b473c795d18ecf6519  ./05Offline-DA-increments/L96_model.py
b527fe57fdd668b473c795d18ecf6519  ./06Different-ML-models/L96_model.py
c4d6e77fd3185920d182f7e113cc34eb  ./08-Implementation/L96_model_XYtend.py

The smallest difference is between 08-Implementation/L96_model_XYtend.py and 06Different-ML-models/L96_model.py:

$ diff 08-Implementation/L96_model_XYtend.py 06Different-ML-models/L96_model_XYtend.py                                                               11c11
< def L96_eq1_xdot(X, F,advect=True):
---
> def L96_eq1_xdot(X, F):
26,29c26                                                                                                                
<     if advect:
<         Xdot = np.roll(X,1) * ( np.roll(X,-1) - np.roll(X,2) ) - X + F
<     else:
<         Xdot = - X + F
>     Xdot = np.roll(X,1) * ( np.roll(X,-1) - np.roll(X,2) ) - X + F

Replacing all L96_model_XYtrend.py with that from 08-Implementation/ seems simple enough.

The larger differences are between L96_model.py and L96_model_XYtrend.py:

diff L96_model.py L96_model_XYtend.py
11c11
< def L96_eq1_xdot(X, F):
---
> def L96_eq1_xdot(X, F, advect=True):
26c26,29
<     Xdot = np.roll(X,1) * ( np.roll(X,-1) - np.roll(X,2) ) - X + F
---
>     if advect:
>         Xdot = np.roll(X,1) * ( np.roll(X,-1) - np.roll(X,2) ) - X + F
>     else:
>         Xdot = - X + F
67c70
<     return Xdot, Ydot
---
>     return Xdot, Ydot, - hcb * Ysummed
186c189
<     time, xhist, yhist = t0+np.zeros((nt+1)), np.zeros((nt+1,len(X0))), np.zeros((nt+1,len(Y0)))
---
>     time, xhist, yhist, xytend_hist = t0+np.zeros((nt+1)), np.zeros((nt+1,len(X0))), np.zeros((nt+1,len(Y0))), np.zeros((nt+1,len(X0)))
189a193
>     xytend_hist[0,:] = 0
199,202c203,206
<             Xdot1,Ydot1 = L96_2t_xdot_ydot(X, Y, F, h, b, c)
<             Xdot2,Ydot2 = L96_2t_xdot_ydot(X+0.5*dt*Xdot1, Y+0.5*dt*Ydot1, F, h, b, c)
<             Xdot3,Ydot3 = L96_2t_xdot_ydot(X+0.5*dt*Xdot2, Y+0.5*dt*Ydot2, F, h, b, c)
<             Xdot4,Ydot4 = L96_2t_xdot_ydot(X+dt*Xdot3, Y+dt*Ydot3, F, h, b, c)
---
>             Xdot1,Ydot1, XYtend = L96_2t_xdot_ydot(X, Y, F, h, b, c)
>             Xdot2,Ydot2, _ = L96_2t_xdot_ydot(X+0.5*dt*Xdot1, Y+0.5*dt*Ydot1, F, h, b, c)
>             Xdot3,Ydot3, _ = L96_2t_xdot_ydot(X+0.5*dt*Xdot2, Y+0.5*dt*Ydot2, F, h, b, c)
>             Xdot4,Ydot4, _ = L96_2t_xdot_ydot(X+dt*Xdot3, Y+dt*Ydot3, F, h, b, c)
206,207c210,211
<         xhist[n+1], yhist[n+1], time[n+1] = X, Y, t0+si*(n+1)
<     return xhist, yhist, time
---
>         xhist[n+1], yhist[n+1], time[n+1],xytend_hist[n+1] = X, Y, t0+si*(n+1), XYtend
>     return xhist, yhist, time, xytend_hist
221a226
>     XYtend = "tendency of X due to small scale vars"
234a240
>         self.XYtend = 0
249c255
<     def set_param(self, dt=None, F=None, h=None, b=None, c=None, t=None):
---
>     def set_param(self, dt=None, F=None, h=None, b=None, c=None, t=0):
276c282
<         X,Y,t = integrate_L96_2t(self.X, self.Y, si, nt, self.F, self.h, self.b, self.c, t0=self.t, dt=self.dt)
---
>         X,Y,t,XYtend = integrate_L96_2t(self.X, self.Y, si, nt, self.F, self.h, self.b, self.c, t0=self.t, dt=self.dt)
279c285
<         return X,Y,t
---
>         return X,Y,t,XYtend
313c319
<     def set_param(self, dt=None, F=None, t=None, method=None):
---
>     def set_param(self, dt=None, F=None, t=0, method=EulerFwd):

I suggest we adopt the extended model and then shorten the name back to L96_mode.py. The earlier notebooks will have to be updated to ignore the extra data being returned from the extended model.

adcroft added a commit to adcroft/L96_demo that referenced this issue May 3, 2022
Two versions of L96_model_XYtrend.py differed due to the addition of an optional argument (advect=True) in the latest version:
```
$ find . -name "L96*trend.py" | xargs md5sum | sort
01fb2a1bb99e26d2a8785d7f30b8e368  ./04Subgrid-parametrization-pytorch/L96_model_XYtend.py
01fb2a1bb99e26d2a8785d7f30b8e368  ./06Different-ML-models/L96_model_XYtend.py
01fb2a1bb99e26d2a8785d7f30b8e368  ./07-Interpretability-of-ML/L96_model_XYtend.py
c4d6e77fd3185920d182f7e113cc34eb  ./08-Implementation/L96_model_XYtend.py
```

This replaces the all versions with a single version.

Partly addresses m2lines#22
adcroft added a commit to adcroft/L96_demo that referenced this issue May 4, 2022
- A second version of L96_model.py had been added solely to obtain the
  tendency of X through the coupling term.
- This adds a new function to calculate the coupling term and an optional
  argument to the `.run()` method that returns the tendency due to
  coupling.
- Removed second copies of L96 model (L96_model_XYtend.py).
- Updated notebooks to use correct model and with necessary optional
  argument.
- All L96_model.py files are now identical.
- Addresses m2lines#22
adcroft added a commit to adcroft/L96_demo that referenced this issue May 4, 2022
Two versions of L96_model_XYtrend.py differed due to the addition of an optional argument (advect=True) in the latest version:
```
$ find . -name "L96*trend.py" | xargs md5sum | sort
01fb2a1bb99e26d2a8785d7f30b8e368  ./04Subgrid-parametrization-pytorch/L96_model_XYtend.py
01fb2a1bb99e26d2a8785d7f30b8e368  ./06Different-ML-models/L96_model_XYtend.py
01fb2a1bb99e26d2a8785d7f30b8e368  ./07-Interpretability-of-ML/L96_model_XYtend.py
c4d6e77fd3185920d182f7e113cc34eb  ./08-Implementation/L96_model_XYtend.py
```

This replaces the all versions with a single version.

Partly addresses m2lines#22
adcroft added a commit to adcroft/L96_demo that referenced this issue May 4, 2022
- A second version of L96_model.py had been added solely to obtain the
  tendency of X through the coupling term.
- This adds a new function to calculate the coupling term and an optional
  argument to the `.run()` method that returns the tendency due to
  coupling.
- Removed second copies of L96 model (L96_model_XYtend.py).
- Updated notebooks to use correct model and with necessary optional
  argument.
- All L96_model.py files are now identical.
- Addresses m2lines#22
adcroft added a commit to adcroft/L96_demo that referenced this issue May 7, 2022
Two versions of L96_model_XYtend.py differed due to the addition of an optional
argument (advect=True) in the last version:
```
md5sum */L96_model_XYtend.py
09796cc98b8707628e40ce23faf4de5e  04Subgrid-parametrization-pytorch/L96_model_XYtend.py
09796cc98b8707628e40ce23faf4de5e  06Different-ML-models/L96_model_XYtend.py
09796cc98b8707628e40ce23faf4de5e  07-Interpretability-of-ML/L96_model_XYtend.py
820867dc282f9837ca1c1b3734978a0d  08-Implementation/L96_model_XYtend.py
```

Simply copying the last version over the earlier three reduces the total
number of version of the L96 model from three to two:
```
md5sum */L96*.py
2ae7140bceff4a350cd79804c3edd6f8  01Intro/L96_model.py
2ae7140bceff4a350cd79804c3edd6f8  02type-of-parametrization/L96_model.py
2ae7140bceff4a350cd79804c3edd6f8  03Data-Assimilation/L96_model.py
820867dc282f9837ca1c1b3734978a0d  04Subgrid-parametrization-pytorch/L96_model_XYtend.py
2ae7140bceff4a350cd79804c3edd6f8  05Offline-DA-increments/L96_model.py
2ae7140bceff4a350cd79804c3edd6f8  06Different-ML-models/L96_model.py
820867dc282f9837ca1c1b3734978a0d  06Different-ML-models/L96_model_XYtend.py
820867dc282f9837ca1c1b3734978a0d  07-Interpretability-of-ML/L96_model_XYtend.py
820867dc282f9837ca1c1b3734978a0d  08-Implementation/L96_model_XYtend.py
```

This is one step toward addressing m2lines#22 goal of having one single L96
module file.
adcroft added a commit to adcroft/L96_demo that referenced this issue May 7, 2022
There were two versions of the L96 model module that differd in whether
they returned the coupling term. The coupling term is now returned from
the L96s.run() method if the option return_coupling=True is provided.
- Updated all L96_model*.py to be identical
- Updated notebook where changes were needed
- Removed one unused copy of L96_model.py

Still todo for m2lines#22:
[ ] change name of L96 module py files to be the same
[ ] move all files to the same directory
adcroft added a commit to adcroft/L96_demo that referenced this issue May 7, 2022
There were two versions of the L96 model module that differd in whether
they returned the coupling term. The coupling term is now returned from
the L96s.run() method if the option return_coupling=True is provided.
- Updated all L96_model*.py to be identical
- Updated notebook where changes were needed
- Removed one unused copy of L96_model.py
- Notebook 04Subgrid-parametrization-pytorch/Neural_network_for_Lorenz96.ipynb
  was incomplete due to a path issue so has new figures for some cells

Still todo for m2lines#22:
[ ] change name of L96 module py files to be the same
[ ] move all files to the same directory
adcroft added a commit to adcroft/L96_demo that referenced this issue May 7, 2022
There were two versions of the L96 model module that differd in whether
they returned the coupling term. The coupling term is now returned from
the L96s.run() method if the option return_coupling=True is provided.
- Updated all L96_model*.py to be identical
- Updated notebook where changes were needed
- Removed one unused copy of L96_model.py
- Notebook 04Subgrid-parametrization-pytorch/Neural_network_for_Lorenz96.ipynb
  was incomplete due to a path issue so has new figures for some cells

Still todo for m2lines#22:
[ ] change name of L96 module py files to be the same
[ ] move all files to the same directory
adcroft added a commit to adcroft/L96_demo that referenced this issue May 7, 2022
There were two versions of the L96 model module that differd in whether
they returned the coupling term. The coupling term is now returned from
the L96s.run() method if the option return_coupling=True is provided.
- Updated all L96_model*.py to be identical
- Updated notebook where changes were needed
- Removed one unused copy of L96_model.py
- Notebook 04Subgrid-parametrization-pytorch/Neural_network_for_Lorenz96.ipynb
  was incomplete due to a path issue so has new figures for some cells

Still todo for m2lines#22:
[ ] change name of L96 module py files to be the same
[ ] move all files to the same directory
adcroft added a commit to adcroft/L96_demo that referenced this issue May 7, 2022
- All notebooks moved to top directory
- Associated files (e.g. network files or figures) moved upwards also
- Redundant files removed (copies of L96_model.py)
- PDF files grouped into pdf/ directory

Still todo for m2lines#22
[ ] do something about LRP_gradient.py and DA_methods.py (which should
    be in notebooks)
adcroft added a commit to adcroft/L96_demo that referenced this issue May 7, 2022
- All notebooks moved to top directory
- Associated files (e.g. network files or figures) moved upwards also
- Redundant files removed (copies of L96_model.py)
- PDF files grouped into pdf/ directory
- Fixed two relative paths

Note: requirements.txt moved form 04Subgrid-parameterization-pytorch to
top directory.

Still todo for m2lines#22
[ ] do something about LRP_gradient.py and DA_methods.py (which should
    be in notebooks)
adcroft added a commit to adcroft/L96_demo that referenced this issue May 7, 2022
- All notebooks moved to single directory ("notebooks")
- Associated files (e.g. network files or figures) moved also
- Redundant files removed (copies of L96_model.py)
- PDF files grouped into pdf/ directory
- Fixed two relative paths

Note: requirements.txt moved form 04Subgrid-parameterization-pytorch to
top directory.

Still todo for m2lines#22
[ ] do something about LRP_gradient.py and DA_methods.py (which should
    be in notebooks)
adcroft added a commit to adcroft/L96_demo that referenced this issue May 7, 2022
commit 1af111e
Author: Alistair Adcroft <aadcroft@princeton.edu>
Date:   Sat May 7 17:17:45 2022 -0400

    Moved notebooks into same directory

    - All notebooks moved to single directory ("notebooks")
    - Associated files (e.g. network files or figures) moved also
    - Redundant files removed (copies of L96_model.py)
    - PDF files grouped into pdf/ directory
    - Fixed two relative paths

    Note: requirements.txt moved form 04Subgrid-parameterization-pytorch to
    top directory.

    Still todo for m2lines#22
    [ ] do something about LRP_gradient.py and DA_methods.py (which should
        be in notebooks)

commit b3219fd
Author: Alistair Adcroft <aadcroft@princeton.edu>
Date:   Sat May 7 16:48:58 2022 -0400

    Added generated files to .gitignore

    Some notebooks are storing data and images that are not reproducible and
    do not need to be version controlled. I've added them to .gitignore but
    we might want to stop storing/saving some of these files instead.

commit d3a937a
Author: Alistair Adcroft <aadcroft@princeton.edu>
Date:   Sat May 7 15:47:25 2022 -0400

    Renamed L96_model_XYtend.py to L96_model.py

    Renames four files and updates associated notebooks. This is in
    preparation for moving all notebooks into the same directory.

commit a34f3a3
Author: Alistair Adcroft <aadcroft@princeton.edu>
Date:   Sat May 7 14:56:15 2022 -0400

    Reconciled all versions of L96 models

    There were two versions of the L96 model module that differd in whether
    they returned the coupling term. The coupling term is now returned from
    the L96s.run() method if the option return_coupling=True is provided.
    - Updated all L96_model*.py to be identical
    - Updated notebook where changes were needed
    - Removed one unused copy of L96_model.py
    - Notebook 04Subgrid-parametrization-pytorch/Neural_network_for_Lorenz96.ipynb
      was incomplete due to a path issue so has new figures for some cells

    Still todo for m2lines#22:
    [ ] change name of L96 module py files to be the same
    [ ] move all files to the same directory

commit 6bb2122
Author: Alistair Adcroft <aadcroft@princeton.edu>
Date:   Sat May 7 12:32:42 2022 -0400

    Replaced L96_model_XYtend.py variants with same version

    Two versions of L96_model_XYtend.py differed due to the addition of an optional
    argument (advect=True) in the last version:
    ```
    md5sum */L96_model_XYtend.py
    09796cc98b8707628e40ce23faf4de5e  04Subgrid-parametrization-pytorch/L96_model_XYtend.py
    09796cc98b8707628e40ce23faf4de5e  06Different-ML-models/L96_model_XYtend.py
    09796cc98b8707628e40ce23faf4de5e  07-Interpretability-of-ML/L96_model_XYtend.py
    820867dc282f9837ca1c1b3734978a0d  08-Implementation/L96_model_XYtend.py
    ```

    Simply copying the last version over the earlier three reduces the total
    number of version of the L96 model from three to two:
    ```
    md5sum */L96*.py
    2ae7140bceff4a350cd79804c3edd6f8  01Intro/L96_model.py
    2ae7140bceff4a350cd79804c3edd6f8  02type-of-parametrization/L96_model.py
    2ae7140bceff4a350cd79804c3edd6f8  03Data-Assimilation/L96_model.py
    820867dc282f9837ca1c1b3734978a0d  04Subgrid-parametrization-pytorch/L96_model_XYtend.py
    2ae7140bceff4a350cd79804c3edd6f8  05Offline-DA-increments/L96_model.py
    2ae7140bceff4a350cd79804c3edd6f8  06Different-ML-models/L96_model.py
    820867dc282f9837ca1c1b3734978a0d  06Different-ML-models/L96_model_XYtend.py
    820867dc282f9837ca1c1b3734978a0d  07-Interpretability-of-ML/L96_model_XYtend.py
    820867dc282f9837ca1c1b3734978a0d  08-Implementation/L96_model_XYtend.py
    ```

    This is one step toward addressing m2lines#22 goal of having one single L96
    module file.
adcroft added a commit to adcroft/L96_demo that referenced this issue May 7, 2022
- All notebooks moved to single directory ("notebooks")
- Associated files (e.g. network files or figures) moved also
- Redundant files removed (copies of L96_model.py)
- PDF files grouped into pdf/ directory
- Fixed two relative paths

Note: requirements.txt moved form 04Subgrid-parameterization-pytorch to
top directory.

Still todo for m2lines#22
[ ] do something about LRP_gradient.py and DA_methods.py (which should
    be in notebooks)
@rabernat
Copy link
Contributor Author

This was fixed by #32.

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

No branches or pull requests

2 participants