search

ocean-eddy-cpt / gcm-filters-paper

Manuscript on spatial filtering method
1 stars 0 forks source link

Updating filterSpec #20

Closed iangrooms closed 3 years ago

iangrooms commented 3 years ago

I've created a branch where I updated filter.py in several ways:

  1. Added a default calculation of N based on the coarsening factor (filter scale vs grid scale). Default value results in errors between .001 and .01 in the polynomial approximation of the target filter.
  2. Added an optional argument d for the dimension of the grid. @jakesteinberg is using 1D data, and it helps to have an option for 1d instead of 2d.
  3. Changed the meaning of Lf for the Gaussian filter so that it's now Lf=standard devation of Gaussian kernel. (Changed by a factor of root 2.)
  4. Changed the meaning of Lf for the Taper filter so that it's now 2*pi/Lf is the wavenumber set to zero. (Changed by a factor of 2.)

What do you think about these changes? We have a recipe for connecting the "target" grid scale to Lf. For the "Taper" filter Lf is now the same as the target grid scale, but for the Gaussian the target grid scale is \sqrt{12}*Lf.

rabernat commented 3 years ago

I really like these changes. They make the filter calculation more user friendly.

I would like to note that I am basically maintaining a mirror of this code here: https://github.com/ocean-eddy-cpt/gcm-filters/blob/master/gcm_filters/filter.py

In particular, I have renamed some of the input arguments to be more verbose and follow standard python naming conventions. I've also attempted to document them to the best of my understanding: https://github.com/ocean-eddy-cpt/gcm-filters/blob/04ad83562a670db8d194d7ad672059d74197a109/gcm_filters/filter.py#L157-L175

My hope was that the actual code for the filters could live in the package. The package is basically ready for use and testing and only lacks kernels (see https://github.com/ocean-eddy-cpt/gcm-filters/issues/8, https://github.com/ocean-eddy-cpt/gcm-filters/issues/9, https://github.com/ocean-eddy-cpt/gcm-filters/issues/10).

With your new changes to filterSpec in this repo, much of that is now out of date. If the filterSpec code in this repo (gcm-filters-paper) continues to change, it will be a lot of repetitive work to keep them in sync. So I see two ways forward:

  1. We stop working on the gcm-filters package until the details here converge more
  2. The people working on this paper start using the gcm-filters package for the analysis in this paper and contribute any further changes as pull requests over there

I'm open to whatever you think is best. I just want to communicate clearly to avoid further inefficient duplication between the two repos.

iangrooms commented 3 years ago

I'm in favor of moving development of the filtering code over to the gcm-filters repo. But I'm not actually using the code, so I will defer to @NoraLoose, @jakesteinberg, and @ElizabethYankovsky.

NoraLoose commented 3 years ago

Moving the code development over to the gcm-filters repo sounds good to me, too. I'm hoping to get to some of the kernel issues (e.g., https://github.com/ocean-eddy-cpt/gcm-filters/issues/9) next week. @iangrooms, are you going to submit a pull request over there with your changes to filterSpec?

rabernat commented 3 years ago

Let me know how I can help with this. The documentation for the package has not really been written, making it hard to know where to start. I can try to do some of that today.