New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Implement gwfs function from GEMPAK #911

Merged

dopplershift merged 12 commits into Unidata:master from sgdecker:add_gwf_funs

Aug 6, 2018

Contributor

sgdecker commented Aug 2, 2018

This PR adds the GEMPAK GWFS function by wrapping the SciPy function gaussian_filter. As far as I can tell, there is no way to make gaussian_filter act exactly identical to what GEMPAK does, but the result is awfully close:

This is a work in progress. A few things I haven't figured out:

How to deal with units. This function returns ndarrays no matter what.
How to add this function to the MetPy API part of the docs.

jthielen and others added 3 commits

August 1, 2018 15:41


          Fix non-tuple NumPy indexing in MetPy

77782e7

Since NumPy 1.15 came out, we have been getting pages and pages of
warnings about our use of non-tuple based indexing. This commit converts
all of our previous list-based indexing to tuple-based indexing. This
doesn't yet clear out all the warnings (some of them are upstream in
Pint and scipy), but it does drop the number down from 600-something to
10-something.


          Add GWFS function from GEMPAK

764b722

This commit wraps the gaussian_filter function from SciPy so that it
emulates the GWFS function from GEMPAK as closely as possible. The key
difference is that GEMPAK adds the leftover weights from outside the
averaging window to the center point, but there is no option for
gaussian_filter to do this.

Left unfinished is restoring metadata to the result. For instance, an
xarray.DataArray provided as input becomes a lowly numpy.ndarray on
output.


          Add tests and docs for gwfs()

841c8ea

This commit adds a few basic tests for the metpy.calc.gwfs function,
and also updates the GEMPAK compatibility table.

I'm not sure how to add new documentation to the MetPy API section.

sgdecker requested review from dopplershift and jrleeman as code owners

August 2, 2018 19:18

stickler-ci reviewed

View reviewed changes

metpy/calc/basic.py Outdated

+                  -------
+                  `pint.Quantity`
+                      The filtered 2D scalar grid

stickler-ci Aug 2, 2018

W293 blank line contains whitespace

metpy/calc/basic.py Outdated

+                  where N > 1.  If N <= 1, N = 2 is assumed.  For example, if N = 4,
+                  then the 4 delta x wave length is passed with approximate response
+/e.

stickler-ci Aug 2, 2018

W291 trailing whitespace

metpy/calc/basic.py Outdated

+                  n = int(round(n))
+                  if n < 2:
+                      n = 2
+                  sgma = n / (2*np.pi)

stickler-ci Aug 2, 2018

E226 missing whitespace around arithmetic operator

metpy/calc/basic.py Outdated

+                  if n < 2:
+                      n = 2
+                  sgma = n / (2*np.pi)
+                  res = gaussian_filter(scalar_grid, sgma, truncate=2*np.sqrt(2))

stickler-ci Aug 2, 2018

E226 missing whitespace around arithmetic operator

metpy/calc/basic.py Outdated

		res = gaussian_filter(scalar_grid, sgma, truncate=2*np.sqrt(2))
		return res

stickler-ci Aug 2, 2018

W293 blank line contains whitespace

metpy/calc/tests/test_basic.py Outdated

+.40077472, 41.40077472, 54.40077472, 69.31520047, 82.67898913],
+                                     [ 6.20489127,  7.39627081, 10.40077472, 15.40077472, 22.40077472,
+.40077472, 42.40077472, 55.40077472, 70.31520047, 83.67898913],
+                                     [ 7.20489127,  8.39627081, 11.40077472, 16.40077472, 23.40077472,

stickler-ci Aug 2, 2018

E201 whitespace after '['
E241 multiple spaces after ','

metpy/calc/tests/test_basic.py Outdated

+.40077472, 42.40077472, 55.40077472, 70.31520047, 83.67898913],
+                                     [ 7.20489127,  8.39627081, 11.40077472, 16.40077472, 23.40077472,
+.40077472, 43.40077472, 56.40077472, 71.31520047, 84.67898913],
+                                     [ 8.20038736,  9.3917669,  12.39627081, 17.39627081, 24.39627081,

stickler-ci Aug 2, 2018

E201 whitespace after '['
E241 multiple spaces after ','

metpy/calc/tests/test_basic.py Outdated

+.40077472, 43.40077472, 56.40077472, 71.31520047, 84.67898913],
+                                     [ 8.20038736,  9.3917669,  12.39627081, 17.39627081, 24.39627081,
+.39627081, 44.39627081, 57.39627081, 72.31069656, 85.67448522],
+                                     [ 9.00900782, 10.20038736, 13.20489127, 18.20489127, 25.20489127,

stickler-ci Aug 2, 2018

E201 whitespace after '['

metpy/calc/tests/test_basic.py Outdated

+.20489127, 45.20489127, 58.20489127, 73.11931702, 86.48310568]])
+                  assert_array_almost_equal(s, s_true)
+              def test_gwfs_small_n():

stickler-ci Aug 2, 2018

E302 expected 2 blank lines, found 1

metpy/calc/tests/test_basic.py Outdated

+                            [2.00708990, 3.01417981, 6.01417981, 11.0141798, 17.9503707],
+                            [3.00708990, 4.01417981, 7.01417981, 12.0141798, 18.9503707],
+                            [4.00000000, 5.00708990, 8.00708990, 13.0070899, 19.9432808]]
+                  print(s-s_true)

stickler-ci Aug 2, 2018

E226 missing whitespace around arithmetic operator


          Fix whitespace issues re pep8

b1d7f0e

dopplershift requested changes

View reviewed changes

Member

dopplershift left a comment

Thanks for the contribution, one more step towards GEMPAK!

My only real concern is with the naming of the function, especially in this case where I can't even figure out definitively what gwfs even stands for.

Also, you'll need to update metpy.calc.rst to include this function in the list of calculations in the docs.

metpy/calc/basic.py Outdated

+                  if n < 2:
+                      n = 2
+                  sgma = n / (2 * np.pi)
+                  res = gaussian_filter(scalar_grid, sgma, truncate=2 * np.sqrt(2))

Member

dopplershift Aug 2, 2018

Could just be return gaussian_filter(scalar_grid, sgma, truncate=2 * np.sqrt(2))

metpy/calc/basic.py Outdated

@@ @@ -614,6 +615,90 @@ def sigma_to_pressure(sigma, psfc, ptop): @@
                   return sigma * (psfc - ptop) + ptop
+              @exporter.export
+              @preprocess_xarray
+              def gwfs(scalar_grid, n):

Member

dopplershift Aug 2, 2018

We need a name for this--replicating GEMPAK functionality is a big 👍 . Replicating naming from the 80's when you had to pay per-character (or so it seems) is something I'd like to not replicate. 😁

How about gaussian_smooth?

Also, is n the most sensible argument for this function? Personally, I'd like to be able to specify the kernel width in some kind of physical units as well, but we don't have to do this right now. If this is the most sensible interface to you the user, then I'm happy to just roll with it.

Contributor Author

sgdecker Aug 3, 2018

Yes, gwfs is more of a placeholder name to make it clear what GEMPAK function it corresponds to. Looking ahead to eventual implementations of GEMPAK's sm5s and sm9s, maybe all of these should start with 'smooth', so smooth_gaussian, smooth_5pt, etc.?

Regarding n, it sounds like you are wanting the related GEMPAK function RDFS (Resolution Dependent Filter for Scalar), which takes dx instead of n as the second argument, and then behind the scenes computes an appropriate n based on dx and calls GWFS. But I suppose just because they are two separate GEMPAK functions doesn't mean they have to be separate MetPy functions.

Member

dopplershift Aug 3, 2018

smooth_gaussian sounds good to me.

If both GWFS and RDFS are doing the same calculation, just tweaking the parameter, I’d lean towards putting all of the functionality in smooth_gaussian, and just look at the arguments passed in to determine the proper course of action. That doesn’t have to be implemented in this PR, though—I’d be happy just to merge the GWFS equivalent.

Member

dopplershift commented Aug 2, 2018

@kgoebber, you've written another version of this previously--I'd love to have your input on this and if it would do everything you need it to do.

dopplershift added Type: Feature Area: Calc labels

dopplershift added this to the 0.9 milestone

Member

dopplershift commented Aug 2, 2018

Can you update the function to maintain the units of the input array? Then this PR would close #664 .


          Merge pull request Unidata#910 from jthielen/non-tuple-indexing-cleanup

d531f34

Fix non-tuple NumPy indexing in MetPy

Collaborator

kgoebber commented Aug 2, 2018

I have just used the gaussian_filter - this is a nice warp to that which will present slightly less verbose syntax. gwfs likely stands for gaussian weighted filter scalar in GEMPAK parlance. Eventually we'll want to add the five point and nine point smoother too as they would likely be useful too, but not a top priority.

Member

dopplershift commented Aug 2, 2018

(See #659 for the 5- and 9-point smoothers.)

Contributor Author

sgdecker commented Aug 3, 2018

I do want the function to maintain units, just need to figure it out. I'd also like to make sure it only smooths in the horizontal direction if given an array with more than two dimensions. This would match GEMPAK semantics, where even when you are creating a cross section or time series plot, calling GWFS as part of the function to plot smooths each vertical level or time independently.

Member

dopplershift commented Aug 3, 2018

So to maintain units, what you want to do is:

if hasattr(scalar_grid, ‘units’):
    res = res * scalar_grid.units

Putting that block after the call to gaussian_filter and before the return.

sgdecker added 7 commits

August 3, 2018 15:58


          Add GWFS function from GEMPAK

261188f

This commit wraps the gaussian_filter function from SciPy so that it
emulates the GWFS function from GEMPAK as closely as possible. The key
difference is that GEMPAK adds the leftover weights from outside the
averaging window to the center point, but there is no option for
gaussian_filter to do this.

Left unfinished is restoring metadata to the result. For instance, an
xarray.DataArray provided as input becomes a lowly numpy.ndarray on
output.


          Add tests and docs for gwfs()

10418d7

This commit adds a few basic tests for the metpy.calc.gwfs function,
and also updates the GEMPAK compatibility table.

I'm not sure how to add new documentation to the MetPy API section.


          Fix whitespace issues re pep8

027bb97


          Extend functionality of gwfs and update docs

4f898e2

gwfs is extended to preserve units and ensure that only horizontal
directions are smoothed (assuming last two axes are
horizontal). Documentation is updated so that this function appears.


          Rename gwfs() to smooth_gaussian()

067d33b


          Merge branch 'add_gwf_funs' of https://github.com/sgdecker/metpy into…

582237d

… add_gwf_funs


          Update GEMPAK Conversion Guide

2fdb0d4

One place was overlooked when changing gwfs() to
smooth_gaussian(). This corrects the oversight.

Contributor Author

sgdecker commented Aug 3, 2018

Thanks for the pointer on the units. I'll leave the alternate, dx version for a future issue..

dopplershift approved these changes

View reviewed changes

dopplershift merged commit ef22931 into Unidata:master

Member

dopplershift commented Aug 6, 2018

Thanks for the contribution @sgdecker !

zbruick mentioned this pull request

Normal Smoothing #664

Closed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area: Calc Type: Feature