Refactored mask regridding for irregular grids (fixes #772)

[zklaus]

Tasks

• Create an issue. This PR partly addresses issue ESMF returns garbage data after CORDEX regridding #772.
• This pull request has a descriptive title that can be used in a changelog
• Add unit tests
• Public functions should have a numpy-style docstring so they appear properly in the API documentation. For all other functions a one line docstring is sufficient.
• If writing a new/modified preprocessor function, please update the documentation. This is only an implementation change.
• Circle/CI tests pass. Status can be seen below your pull request. If the tests are failing, click the link to find out why.
• Codacy code quality checks pass. Status can be seen below your pull request. If there is an error, click the link to find out why. If you suspect Codacy may be wrong, please ask by commenting.
• Please use yamllint to check that your YAML files do not contain mistakes. No yaml files added or changed.
• If you make backward incompatible changes to the recipe format, make a new pull request in the ESMValTool repository and add the link below. No changes to the recipe format.

---

Related to #772

[bouweandela]

Looks good. Did you want to include a unit test that shows that the bug has been solved?

Does this pull request completely solve #772, or are there remaining issues to be tackled?

[valeriupredoi]

cheers @zklaus 🍺 Unfortunately I have gotten rid of my test data set and I think it's prob best if @thomascrocker tested it anyway since he'll pay more attention to the issues he's already seen - @thomascrocker could you give it one more test please mate? I agree with @bouweandela in comment - nice fix!

[valeriupredoi]

approval pending testing and unit test inclusion (if Klaus deems it necessary, I think it'd be good to have one but am not too fussed)

[thomascrocker]

cheers @zklaus 🍺 Unfortunately I have gotten rid of my test data set and I think it's prob best if @thomascrocker tested it anyway since he'll pay more attention to the issues he's already seen - @thomascrocker could you give it one more test please mate? I agree with @bouweandela in comment - nice fix!

Thanks for sorting the fix out. I'm up against a Christmas deadline at the moment, but I'll make sure to test this with my data in the new year.
Cheers

[thomascrocker]

Happy new year all!
The fix solves the regridding, but there are still problems with variable attributes that need to be addressed. See #772 (comment)

[valeriupredoi]

hey fellas, what's the status here: will the work needed to address the attributes be done in a separate PR? I'd advocate we merge this now and do the remainder of the work in a separate PR if it's not inter-dependent with the code changes here 🍺

[thomascrocker]

I was just looking into regridding again with CORDEX and had a go at using this branch. There are a couple of problems..
This #772 (comment) problem with the coord attributes is still outstanding..
And a potentially bigger (dataset) problem...

Some (but not all) of the CORDEX data on ESGF, for example:
http://esg-dn1.nsc.liu.se/thredds/fileServer/esg_dataroot4/cordexdata/cordex/output/EUR-11/SMHI/NCC-NorESM1-M/historical/r1i1p1/SMHI-RCA4/v1/mon/tas/v20180820/tas_EUR-11_NCC-NorESM1-M_historical_r1i1p1_SMHI-RCA4_v1_mon_198101-199012.nc
Does not have any bounds on it's latitude and longitude co-ordinates. The regridding needs bounds on the grid in order to work. Since these are multidimensional coordinates (due to the rotated grid) they can't be easily added to the data as part of automated fixes using the guess_bounds iris function.

[zklaus]

Thanks @thomascrocker. You are right. We are dealing with this precise problem at the moment at our institute. I think the best approach is probably to store known-good latitude-longitude coords including bounds under a hash of the rlat, rlon coords. In principle it should be straight-forward to calculate the bounds from the 1d bounds using iris.analysis.cartography or cartopy.crs.RotatedPole, but storing known good coords might save us some grieve down the line because the CORDEX data tends to have some very small difference in coords in the file on the order of picometers.

Unfortunately, this might have to wait until after the June release.

[thomascrocker]

Thanks @thomascrocker. You are right. We are dealing with this precise problem at the moment at our institute. I think the best approach is probably to store known-good latitude-longitude coords including bounds under a hash of the rlat, rlon coords. In principle it should be straight-forward to calculate the bounds from the 1d bounds using iris.analysis.cartography or cartopy.crs.RotatedPole, but storing known good coords might save us some grieve down the line because the CORDEX data tends to have some very small difference in coords in the file on the order of picometers.

Unfortunately, this might have to work until after the June release.

Thanks, that's a good idea. The MOHC EUR-11 domain files for example do have the correct lat and lon coords, with bounds, so I may see if I can hack together a dataset fix that reads that information and applies it to all CORDEX data on that domain.

[bouweandela]

Is the problem with the bounds somehow overlapping with #184?

[thomascrocker]

Is the problem with the bounds somehow overlapping with #184?

Yes you're right. Good spot, it's the same problem. Although as I've mentioned not all CORDEX data is missing bounds, data from some institutes does have it.

@zklaus is correct though that despite the fact that CORDEX runs over the same domain should all be on the same grid, there are often very small rounding error style differences in the grid specification from one institute to another, so the approach of storing template grid specifications for each domain somewhere and then applying them as a fix to CORDEX data on load is a good idea to save the inevitable errors that would result from doing any multi model statistics or arithmetic.

[bouweandela]

@zklaus Would that solution then replace #184?

[zklaus]

I think this PR and #184 solve different aspects of regridding CORDEX data. This one is really addressing a bug in the regridding, #184 is addressing a common problem in CORDEX data.

[bouweandela]

I did not mean this pull request, I meant: would the solution proposed in #865 (comment) replace #184?

[zklaus]

Well, yes, though it was basically already proposed in #184 (comment) and seems to have been picked up subsequently by @mwjury, but in a slightly unorthodox way by attaching some textfiles containing python code in comments on the ticket.

Can we perhaps move forward like this: Let's finish this PR ignoring the issue of missing bounds on cordex. Let's attack that in #184. For that purpose, could you, @thomascrocker, head over there and sort out with @mwjury how to solve it?

[thomascrocker]

Well, yes, though it was basically already proposed in #184 (comment) and seems to have been picked up subsequently by @mwjury, but in a slightly unorthodox way by attaching some textfiles containing python code in comments on the ticket.

Can we perhaps move forward like this: Let's finish this PR ignoring the issue of missing bounds on cordex. Let's attack that in #184. For that purpose, could you, @thomascrocker, head over there and sort out with @mwjury how to solve it?

Makes sense to keep this PR as fixing the specific regridding issue, and keep #184 to deal with coord bounds. I will put any further work on that problem there.

[codecov]

Codecov Report

Merging #865 (9b3c8ec) into main (5ada605) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main     #865   +/-   ##
=======================================
  Coverage   88.24%   88.24%           
=======================================
  Files         194      194           
  Lines        9831     9838    +7     
=======================================
+ Hits         8675     8682    +7     
  Misses       1156     1156           

Impacted Files	Coverage Δ
esmvalcore/preprocessor/_regrid_esmpy.py	78.40% <100.00%> (+0.89%)	⬆️

---

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 5ada605...9b3c8ec. Read the comment docs.

[valeriupredoi]

this is looking good @zklaus 🍺