Open LL-Geo opened 1 year ago
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.
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.
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.
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.
Heh, I wish I had had thought up that idea -- it's a good one! But I think you might mean @mdtanker ?
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...).
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