harmonica icon indicating copy to clipboard operation
harmonica copied to clipboard

Put padding inside the transformation function

Open LL-Geo opened this issue 1 year ago • 6 comments

Description of the desired feature:

Right now the padding process is done externally before passing it to the grid transformation padding -> hm.filter -> unpadding I'm thinking to put the padding and unpadding inside the transformation, so we can use one-line code to do the filter where paddling and unpadding are handled inside the transformation.

Probably we can also work on different type of padding method, for example Fedi, 2012. But I guess this will be better to implement on the xrft side? What do you guys think?

Are you willing to help implement and maintain this feature? Happy to implement this, but also love to see contributors to tackle this one

LL-Geo avatar Mar 03 '23 04:03 LL-Geo

Yeah I think this would be a good addition. It is a little annoying having to remember the specifics for how to pad and unpad everytime.

Out of curiosity, have you tried any other padding options besides xrft? Such as xr.Dataarray.pad()? The xrft pad seems to work fine.

mdtanker avatar Mar 15 '23 22:03 mdtanker

Yeah I think this would be a good addition. It is a little annoying having to remember the specifics for how to pad and unpad everytime.

Out of curiosity, have you tried any other padding options besides xrft? Such as xr.Dataarray.pad()? The xrft pad seems to work fine.

I haven't tried the xr.Dataarray.pad() yet, but I guess that would be similar. Yeah, Santi write the pad function in xrft, hahaha.

LL-Geo avatar Mar 17 '23 11:03 LL-Geo

That's a +1 from me on this.

We had a bunch of padding options in the old fatiando here if anyone is curious: https://github.com/fatiando/fatiando/blob/master/fatiando/gridder/padding.py Most of those are covered by xarray already.

leouieda avatar Mar 17 '23 12:03 leouieda

I agree too! It's annoying to having to handle the padding/unpadding steps every time we need to use these transformation functions.

Regarding the choice of xr.DataArray.pad() and xrft.pad(). They both use numpy.pad under the hood. The main difference is that the xarray method doesn't apply a consistent padding to the coordinates, something that we really need to do if we want to compute the FFT of the padded grid. That's why I wrote the xrft.pad() function. If you read its docstring you'll see that it offers all the padding methods that numpy.pad does (and by extension, the old fatiando padding functions).

In summary, I would make the apply_filter() function to handle the padding and unpadding steps before computing the fft and after computing the ifft, respectively. And I would use xrft.pad() for it.

Now we need to think how we would like to pass the options for the padding function.

One way could be to ask for a pad_kwargs dictionary. It introduces only one more argument, but it doesn't offer autocompletion. As an example, it will be use like this:

pad_kwargs = dict(
    pad_width={"easting": 50, "northing": 50},
    constant_values=0,
)

hm.upward_continuation(grid, height_displacement=500, pad_kwargs=pad_kwargs)

I think this decision is somewhat related to the idea that @mdtanker shared in #396. If we want the apply_filter() function to be smarter and be able to apply padding or to fill nans in the grid, then we should be clever about how we handle its arguments.

santisoler avatar Mar 17 '23 18:03 santisoler

Heh, I wish I had had thought up that idea -- it's a good one! But I think you might mean @mdtanker ?

matthewturk avatar Mar 17 '23 18:03 matthewturk

Heh, I wish I had had thought up that idea -- it's a good one! But I think you might mean @mdtanker ?

haha sorry! I just edited it. Reminder that I should actually type the handles instead of clicking the popup list (and click the wrong one...).

santisoler avatar Mar 17 '23 18:03 santisoler