Add arbitrary window smoother and refactor existing smoothers #1223
Conversation
|
I've finally gotten the chance to go back through and update this. Tests and an example of the smoothers are now included. I also rebuilt the array type wrapping helper to be a decorator that should easily generalize to most calculations (any calculation which has an output that matches the dimensionality of one of its inputs). I do want to leave the WIP label on for right now though, since the wrapping helper is a mess of conditional statements and could use a refactor. But, it is much closer to being ready, and I wanted to get this out there now to hopefully help with what is needed for xarray compatibility for v1.0. Between this helper and implementing #893 as a follow-up to #1260, I think most of |
jthielen
changed the title
|
I was able to go back and refactor the output wrapping decorator. I realized that a cleaner implementation would be to make two assumptions I think are quite reasonable:
So, this should be ready for a full review now! |
|
Also, one other note, since the wrapper added here is meant to generalize to most other functions to help bring greater xarray compatibility, here are the steps I would see remaining after this PR:
|
dopplershift
added
Area: Calc
src/metpy/calc/basic.py
Outdated
| for _ in range(passes): | ||
| # Set values corresponding to smoothing weights by summing over each weight and | ||
| # applying offsets in needed dimensions | ||
| data[_trailing_dims(_offset(_pad(n), 0) for n in weights.shape)] = sum( |
There was a problem hiding this comment.
Readability of this isn't great - maybe be a bit more explicit/verbose. Shouldn't be a performance hit, correct?
There was a problem hiding this comment.
Since I'm iteratively modifying the array that is used in the calculation, replacing the sum with a loop would require another temporary array that would be cleared each pass, which I think would be a hit on memory? I could assign some of the indexers to temporary variables or internal functions to try making it more readable though.
|
|
||
| """ | ||
| # Generate the circle | ||
| size = 2 * radius + 1 |
There was a problem hiding this comment.
Plus one so it's inclusive in the radius? Maybe note that in the docs.
There was a problem hiding this comment.
Plus one since the window has to have odd shape in all dimensions. I'll definitely add a note to the docs.
| + r * (smooth_grid[2:, 2:] + smooth_grid[2:, :-2] + | ||
| + smooth_grid[:-2, 2:] + smooth_grid[:-2, :-2])) | ||
| return smooth_grid | ||
| return smooth_window(scalar_grid, window=weights, passes=passes, normalize_weights=False) |
…ntities and DataArrays
|
Updates addressing the feedback have been added, and all tests are green except the codecov tests coverage, which I have no idea why it isn't 100%. |
Description Of Changes
I've had a good part of this sitting in my repo for a while, and I've neglected to do anything with it since I don't have tests yet. While I still don't have tests yet, #1221 gave me the idea of going back to it and put it up as a PR to get any early feedback on it, since it does strip and reattach units in the smoother. Plus, this may motivate me to finally put together the tests.This PR adds an arbitrary window smoother that utilizes a weighted sum over offset array subsets. Convenience methods for rectangular and circular windows are also added, along with refactoring the n-point smoother to use the new window smoother (which allows more flexible dimensionality). In an attempt to be compatible with base ndarrays, pint Quantities, and xarray DataArrays, there is also a array wrapping decorator added, which may be able to be generalized to other functions for makeshift xarray support.
Checklist
smooth_windowsmooth_circularsmooth_rectangularwrap_data_like