Support building regridders with masks

[stephenworsley]

Addresses #218 by having ESMF ignore masked points.

TODO

• Handle the creation of "contiguous bounds" in a way which prioritises unmasked cells.
• Consider a way to derive masks directly from cube masks.
• Write a benchmark to demonstrate that ignoring discontiguities offers significant performance improvements.
• extend support to unstructured regridders.
• extend support to RefinedGridInfo.
• Extend support to regridder load/save
• Consider a way to derive masks directly from discontiguities in coordinates.
• Consider a way to properly derive a mask from masked data (e.g. only using masks which are constant over all non-horizontal dimensions)

[codecov]

Codecov Report

Merging #219 (b309b4a) into main (38f2b67) will increase coverage by 0.02%.
The diff coverage is 98.79%.

❗ Current head b309b4a differs from pull request most recent head e06c20e. Consider uploading reports for the commit e06c20e to get more accurate results

@@            Coverage Diff             @@
##             main     #219      +/-   ##
==========================================
+ Coverage   99.19%   99.22%   +0.02%     
==========================================
  Files          28       28              
  Lines        2871     3095     +224     
==========================================
+ Hits         2848     3071     +223     
- Misses         23       24       +1     

Impacted Files	Coverage Δ
esmf_regrid/experimental/unstructured_regrid.py	97.36% <ø> (ø)
esmf_regrid/_esmf_sdo.py	96.51% <95.23%> (+1.02%)	⬆️
esmf_regrid/schemes.py	97.43% <97.26%> (-0.35%)	⬇️
esmf_regrid/esmf_regridder.py	95.45% <100.00%> (+0.06%)	⬆️
esmf_regrid/experimental/io.py	100.00% <100.00%> (ø)
esmf_regrid/experimental/unstructured_scheme.py	98.56% <100.00%> (+0.05%)	⬆️
.../tests/unit/experimental/io/test_round_tripping.py	100.00% <100.00%> (ø)
...nstructured_scheme/test_GridToMeshESMFRegridder.py	100.00% <100.00%> (ø)
...nstructured_scheme/test_MeshToGridESMFRegridder.py	100.00% <100.00%> (ø)
...sts/unit/schemes/test_ESMFAreaWeightedRegridder.py	100.00% <100.00%> (ø)
... and 1 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[github-actions]

@SciTools-incubator/esmf-regrid-devs This pull-request is stale due to a lack of activity in the last 90 days. Remove stale label or comment, otherwise this pull-request will close automatically in 7 days time.

[github-actions]

@Scit@SciTools-incubator/esmf-regrid-devs This stale pull-request has been automatically closed due to no community activity

[stephenworsley]

The benchmarks run against a curvilinear grid with a row of degenerate cells which would cause the regridder to run slower. masking. Benchmarks are run with and without the mask being included. When the mask is included there are significant performance improvements (this may be even better when the cells are particularly bad). Note that this benchmarks with masks fails on main.

[trexfeathers]

I'm not quite done reviewing yet, but I wanted to give you some things to work on so you're not waiting on me.

Great idea!

[trexfeathers]

Here's the rest of my review.

Is there merit in replacing NumPy operations on the masks with Dask Array operations? Some Cube masks can be very large.

[trexfeathers]

Apologies, I know I can get a bit evangelical about D.R.Y. code, but do I feel this PR makes a very strong case for more effort in this space.

I believe I have the second-most experience with this codebase, but all the layers and parallel changes made me get quite lost during review. I'm also going to find it difficult to confirm which of my suggestions have been actioned, as the action will need to be mirrored in an uncertain number of places. Also, if I wanted to make an enhancement like this, I doubt I would be successful in remembering all the different places a change needed to be made.

D.R.Y. takes longer to get working, and I appreciate that's work that could otherwise be spent on new features, but it makes future work quicker, easier and less error-prone. This is all doubly true when it comes to developers who are not the original author. I'm basically describing Technical Debt. I know there are already some ideas in the pipeline (#198); take this as my vote for expediting that as well as going further; let's pay off that debt sooner before we pay much more Interest 👍.

Cc @pp-mo as the buddy for iris-esmf-regrid.

[valeriupredoi]

just plopping myself here to say hi and good work on this, guys! Looking forward to see this and the other features, including the handling of the new esmpy in an upcoming release 🍻

[trexfeathers]

Looking good... here are my new comments.

[trexfeathers]

This should have a CHANGELOG entry:

https://github.com/SciTools-incubator/iris-esmf-regrid/blob/38f2b67af67de721a16e4d3d96ff4ccfc477aa98/CHANGELOG.md#L8-L12

[stephenworsley]

This should have a CHANGELOG entry:

I've decided to add an entry for #241 while I was there.