Adding ESMF.LocStream capabilities to xESMF #81

raphaeldussin · 2020-02-19T18:23:31Z

@JiaweiZhuang this PR allows xESMF to use LocStream objects (sequence of geographical points).
This is particularly useful to interpolate onto ocean sections, creating open boundary conditions,...

I have made minimal inserts, that do not change any default behavior. Also added some testing.
Let me know if there's more to be done for the PR.

Best,
Raf

…to locstream

codecov-io · 2020-02-19T18:28:45Z

Codecov Report

Merging #81 into master will increase coverage by 0.78%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master      #81      +/-   ##
==========================================
+ Coverage   95.76%   96.55%   +0.78%     
==========================================
  Files           6        6              
  Lines         260      319      +59     
==========================================
+ Hits          249      308      +59     
  Misses         11       11

Impacted Files	Coverage Δ
xesmf/frontend.py	`95.58% <100%> (+1.59%)`	⬆️
xesmf/backend.py	`96.51% <100%> (+0.51%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7479110...b84f2d7. Read the comment docs.

JiaweiZhuang · 2020-02-19T20:30:33Z

xesmf/frontend.py

 class Regridder(object):
 def __init__(self, ds_in, ds_out, method, periodic=False,
- filename=None, reuse_weights=False, ignore_degenerate=None):
+ filename=None, reuse_weights=False, ignore_degenerate=None,
+ locstream_out=False):


locstream might also be input I think? (regridding locstream -> grid)

JiaweiZhuang · 2020-02-19T20:52:50Z

xesmf/frontend.py

+ if len(lon.shape) > 2:
+ raise ValueError("lon can only be 1d or 2d")
+ if len(lat.shape) > 2:
+ raise ValueError("lat can only be 1d or 2d")
+
+ if len(lon.shape) == 2:
+ lon = lon.flatten()
+ if len(lat.shape) == 2:
+ lat = lat.flatten()


Better require strict 1-D shape which is the definition of locstream.

I'm fine with it! maybe the flexibility is not a good thing here actually.

JiaweiZhuang · 2020-02-19T20:56:15Z

Thanks very much for the PR! This is something that I've been wanting to add but never got time to actually do it. Also thanks for the unit test. What would be even more useful is an example notebook under doc/notebooks to show a use case.

I am mostly thinking about how to have a consistent API to handle various cases:

grid -> grid (current only option)
grid -> locstream (your PR)
locstream -> grid (this may have more demand, as the above one can also be achieved by xarray Advanced Interpolation)
locstream -> locstream (also possible but rarely used)

Options are:

Add locstream_out and locstream_in kwarg to Regridder, following your PR
Have a new subclass LocStreamRegridder to handle locstream input/output, and avoid touching the original Regridder API. It might feel like different classes under sklearn linear regressors
Mark the locstream attribute in the grid object ds_in / ds_out (detect coordinate name like locations, but it won't work for dict), and avoid additional kwargs or classes.

I was leaning towards no. 2, but your approach also makes sense.

JiaweiZhuang · 2020-02-19T20:59:46Z

Another complication from LocStream is that this class can only be used for:

Destination for bilinear and patch (cannot be source)
Source and destination for nearest neighbor

It cannot be used for conservative method, for either source or destination.

Ref: ESMF Regridding Update

JiaweiZhuang · 2020-02-19T21:02:01Z

xesmf/tests/test_frontend.py

-def test_build_regridder(method):
- regridder = xe.Regridder(ds_in, ds_out, method)
+@pytest.mark.parametrize('locstream', [True, False])
+def test_build_regridder(method, locstream):


I don't think locstream can be used with conservative method. Does it actually work here?

no I hacked it on line 57 :)

I would use more explicit expression and avoid overwriting method internally:

@pytest.mark.parametrize( "locstream,method", [ (False, method_list), (True, ['bilinear', 'nearest_s2d', 'nearest_d2s']) # conservative method does not work for LocStream ])

raphaeldussin · 2020-02-19T21:57:59Z

About the more general comments:

as an oceanographer, I wouldn't use locstream -> grid when there's more powerful tools such as DIVA https://modb.oce.ulg.ac.be/mediawiki/index.php/DIVA
locstream to locstream also looks very exotic

I went for option 1 because that's in my opinion, the one that added the less code. Once the ESMF Fields are created, ESMF Regridder works the same (not allowing all types of regridding of course).
I think adding a locstream_in a way that's symetric to locstream_out should not be a big issues if you
think that might be useful (see previous point). Even with LocStream in, I think 95+% of users will be coming for the grid-> grid interpolation. What do you think?

I'm gonna put some more tests, the codecov for my PR is not very good. An example notebook with follow once we ironed out the API details.

JiaweiZhuang · 2020-02-19T23:06:32Z

I think adding a locstream_in a way that's symetric to locstream_out should not be a big issues if you think that might be useful

Yeah I think it is good to have both locstream_out and locstream_in as options. The latter is useful for aligning observation data to model grid (similar to supersampling).

For the example notebook, it can be with synthetic data replicating advanced interpolation or with real data from xr.tutorial. Can revert the resulting locstream back to model grid via locstream_in.

raphaeldussin · 2020-02-21T22:36:49Z

@JiaweiZhuang sorry for the repeated commits, it's ready to be reviewed/merged.

raphaeldussin · 2020-03-06T16:45:04Z

@JiaweiZhuang I realized that I have missed one of your comments. Fixed now and I also expanded the test_build_regridder to include locstream_in.

Let me know if there's more that needs to be done. I'd really like to move fwd with it.

xesmf/tests/test_frontend.py

JiaweiZhuang · 2020-03-06T18:08:59Z

@raphaeldussin Sorry I had a long backlog. A few more comments on the test code, and it should be good to go.

JiaweiZhuang · 2020-03-06T18:09:49Z

xesmf/tests/test_frontend.py

+
+ # -----------------------------------------
+ # testing locstream out
+ regridder = xe.Regridder(ds_in, ds_locs, 'bilinear', locstream_out=True)
+ ds_result = regridder(ds_in)
+ # clean-up
+ regridder.clean_weight_file()
+
+ # -----------------------------------------
+ # testing locstream in
+ regridder = xe.Regridder(ds_locs, ds_in, 'nearest_s2d', locstream_in=True)
+
+ outdata = regridder(ds_locs) 
+
+ # clean-up
+ regridder.clean_weight_file()


I would separate out the tests for locstreams into different functions

JiaweiZhuang · 2020-03-06T18:10:05Z

xesmf/tests/test_frontend.py

+
+ outdata = regridder(ds_locs['lat']) 
+
+ # clean-up


I would separate out the tests for locstreams into different functions

JiaweiZhuang · 2020-03-06T18:10:12Z

xesmf/tests/test_frontend.py

+ # -----------------------------------------
+ # testing locstream out
+ regridder = xe.Regridder(ds_in, ds_locs, 'bilinear', locstream_out=True)
+
+ indata = ds_in_chunked['data4D'].data
+ outdata = regridder(indata)
+
+ # clean-up
+ regridder.clean_weight_file()
+
+ # -----------------------------------------
+ # testing locstream in
+ regridder = xe.Regridder(ds_locs, ds_in, 'nearest_s2d', locstream_in=True)
+
+ outdata = regridder(ds_locs['lat'].data) 
+
+ # clean-up
+ regridder.clean_weight_file()


I would separate out the tests for locstreams into different functions

JiaweiZhuang · 2020-03-06T18:10:26Z

xesmf/tests/test_frontend.py

+ # -----------------------------------------
+ # testing locstream in
+ regridder = xe.Regridder(ds_locs, ds_in, 'nearest_s2d', locstream_in=True)
+
+ outdata = regridder(ds_locs['lat'].values) # pure numpy array
+ dr_out = regridder(ds_locs['lat']) # xarray DataArray
+
+ # DataArray and numpy array should lead to the same result
+ assert_equal(outdata, dr_out.values)
+
+ # clean-up
+ regridder.clean_weight_file()


I would separate out the tests for locstreams into different functions

raphaeldussin · 2020-03-06T18:33:52Z

move the locstream tests into their own functions.

JiaweiZhuang · 2020-03-06T20:07:16Z

Thanks @raphaeldussin! Added to 0.3.0 release.

locstream dim name consistent with ds_out

raphaeldussin added 6 commits February 18, 2020 21:42

start adding locstream

2751184

start adding locstream

096e3d3

Merge branch 'locstream' of https://github.com/raphaeldussin/xESMF in…

c23e867

…to locstream

support dataarray

72d6e51

datasets work

6daf198

add testing

1299d8d

JiaweiZhuang reviewed Feb 19, 2020

View reviewed changes

raphaeldussin added 4 commits February 21, 2020 15:11

add locstream input

4501b09

more extensive testing

272a729

fix issues locstream_in

0e6b0ed

add documentation

049485e

improve test regridder

ba49b48

JiaweiZhuang reviewed Mar 6, 2020

View reviewed changes

xesmf/tests/test_frontend.py Show resolved Hide resolved

JiaweiZhuang reviewed Mar 6, 2020

View reviewed changes

xesmf/tests/test_frontend.py Outdated Show resolved Hide resolved

JiaweiZhuang reviewed Mar 6, 2020

View reviewed changes

xesmf/tests/test_frontend.py Outdated Show resolved Hide resolved

JiaweiZhuang reviewed Mar 6, 2020

View reviewed changes

split tests

f795297

fix wrong name fct

b84f2d7

JiaweiZhuang merged commit 60cd934 into JiaweiZhuang:master Mar 6, 2020

raphaeldussin deleted the locstream branch July 25, 2020 16:08

aulemahal pushed a commit to Ouranosinc/xESMF that referenced this pull request May 18, 2021

Merge pull request JiaweiZhuang#81 from malmans2/locstream_dim_name

4fd0291

locstream dim name consistent with ds_out

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding ESMF.LocStream capabilities to xESMF #81

Adding ESMF.LocStream capabilities to xESMF #81

raphaeldussin commented Feb 19, 2020

codecov-io commented Feb 19, 2020 •

edited

Loading

JiaweiZhuang Feb 19, 2020

JiaweiZhuang Feb 19, 2020

raphaeldussin Feb 19, 2020

JiaweiZhuang commented Feb 19, 2020

JiaweiZhuang commented Feb 19, 2020 •

edited

Loading

JiaweiZhuang Feb 19, 2020

raphaeldussin Feb 19, 2020

JiaweiZhuang Feb 19, 2020

raphaeldussin commented Feb 19, 2020

JiaweiZhuang commented Feb 19, 2020

raphaeldussin commented Feb 21, 2020

raphaeldussin commented Mar 6, 2020

JiaweiZhuang commented Mar 6, 2020

JiaweiZhuang Mar 6, 2020

JiaweiZhuang Mar 6, 2020

JiaweiZhuang Mar 6, 2020

JiaweiZhuang Mar 6, 2020

raphaeldussin commented Mar 6, 2020

JiaweiZhuang commented Mar 6, 2020

Adding ESMF.LocStream capabilities to xESMF #81

Adding ESMF.LocStream capabilities to xESMF #81

Conversation

raphaeldussin commented Feb 19, 2020

codecov-io commented Feb 19, 2020 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JiaweiZhuang commented Feb 19, 2020

JiaweiZhuang commented Feb 19, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

raphaeldussin commented Feb 19, 2020

JiaweiZhuang commented Feb 19, 2020

raphaeldussin commented Feb 21, 2020

raphaeldussin commented Mar 6, 2020

JiaweiZhuang commented Mar 6, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

raphaeldussin commented Mar 6, 2020

JiaweiZhuang commented Mar 6, 2020

codecov-io commented Feb 19, 2020 •

edited

Loading

JiaweiZhuang commented Feb 19, 2020 •

edited

Loading