ocean-eddy-cpt / gcm-filters

Diffusion-based Spatial Filtering of Gridded Data
https://gcm-filters.readthedocs.io/
Other
37 stars 24 forks source link

Application to 1-dim data #133

Open dhruvbalwada opened 2 years ago

dhruvbalwada commented 2 years ago

Is it possible to apply the package to 1D data? I noticed that there is an example in the paper applying to 1D satellite data (@jakesteinberg). However, when I try to apply to a 1D data, I get the following error:

---------------------------------------------------------------------------
AssertionError                            Traceback (most recent call last)
Input In [45], in <module>
----> 1 test_data_filtered = filter_c.apply(test_data, dims=['XC',])

File ~/miniconda/envs/base_new/envs/GM-Redi-offline-env/lib/python3.10/site-packages/gcm_filters/filter.py:432, in Filter.apply(self, field_or_dataset, dims)
    430     return filtered
    431 else:
--> 432     return self.apply_to_field(field_or_dataset, dims=dims)

File ~/miniconda/envs/base_new/envs/GM-Redi-offline-env/lib/python3.10/site-packages/gcm_filters/filter.py:444, in Filter.apply_to_field(self, field, dims)
    442 filter_func = _create_filter_func(self.filter_spec, self.Laplacian)
    443 grid_args = [self.grid_ds[name] for name in self.Laplacian.required_grid_args()]
--> 444 assert len(dims) == 2
    445 n_args = 1 + len(grid_args)
    446 field_smooth = xr.apply_ufunc(
    447     filter_func,
    448     field,
   (...)
    453     dask="parallelized",
    454 )

AssertionError: 

Seems like 2-Dimensionality is asserted in the code. Am I missing something?

rabernat commented 2 years ago

No. 🙃

The method is applicable to 1D data. But this package assumes everywhere that the data are 2D. That could be changed, but that may be out of scope.

iangrooms commented 2 years ago

I think all we need is a kernel for a 1D Laplacian. You can set ndim=1 in the Filter class, but we don't have a 1D Laplacian that goes with it.

dhruvbalwada commented 2 years ago

Ah, I see. Is there a way to use the package to reproduce plots like Figure 11 in the paper? or was that done using code that is not here? Would creating a dimension of size 1 solve it?

iangrooms commented 2 years ago

The code for that figure is not here, and it looks like it didn't make it onto the source repository for that paper. That said, all that would be needed to reproduce within gcm-filters it is to define a 1D Laplacian kernel. I think @jakesteinberg might also have the original code on one of his repositories.

dhruvbalwada commented 2 years ago

Ok. Thank you for such a quick response. :)

jakesteinberg commented 2 years ago

Hi! Yes, I feel late to the party haha. I used an older version of the code to make that figure (pre gcm-filters but with the same filter kernel idea/process, thanks to @iangrooms help way back at the beginning of this). I can help you make this work if you'd like. I guess I should also try and package up the 1D Laplacian kernel I have and make a pull request. The functions I built are in here and the main one 'Filter()' is applied to 1D data here. It is all a bit simpler (and messier) than gcm-filters, ha.

dhruvbalwada commented 2 years ago

It is not something essential to me right now. I was trying to test out gcm-filters, and thought doing it on periodic 1D data would be simplest case, when I ran into this. I just ended up stepping up 1 level and using 2D data.

However, thinking broadly I feel like this example should be within the scope of the package since the paper literally has a section titled "Application to 1-D observational data". Also if we want to use this package with most obs data, we will need it.

iangrooms commented 2 years ago

I agree that we should have this. It shouldn't be hard to convert existing 2D kernels to 1D, but we might want to add some extra errors/warnings. E.g. if the user tries to use a kernel that is not consistent with the value of ndim then the package should throw an error.