-
Notifications
You must be signed in to change notification settings - Fork 17
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.
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
Unstructured Scheme - Mesh Fetching #46
Unstructured Scheme - Mesh Fetching #46
Conversation
Codecov Report
@@ Coverage Diff @@
## unstructured_scheme #46 +/- ##
=======================================================
+ Coverage 93.47% 98.07% +4.60%
=======================================================
Files 13 16 +3
Lines 521 624 +103
=======================================================
+ Hits 487 612 +125
+ Misses 34 12 -22
Continue to review full report at Codecov.
|
for more information, see https://pre-commit.ci
…com/stephenworsley/iris-esmf-regrid into unstructured_scheme_mesh_extraction * 'unstructured_scheme_mesh_extraction' of https://github.com/stephenworsley/iris-esmf-regrid: [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
…ion' into unstructured_scheme_mesh_extraction * origin/unstructured_scheme_mesh_extraction: [pre-commit.ci] auto fixes from pre-commit.com hooks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good @stephenworsley 👍 . A few comments inline
grid_x, grid_y = get_xy_coords(target_grid_cube) | ||
grid_x, grid_y = get_xy_dim_coords(target_grid_cube) | ||
mesh = src_mesh_cube.mesh | ||
assert mesh is not None |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this to be properly handled / exception raised in the future? If so maybe a TODO added?
# mesh is probably an iris Mesh object, though it could also be a MeshCoord | ||
mesh, mesh_dim = _get_mesh_and_dim(cube) | ||
mesh = cube.mesh | ||
assert mesh is not None |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TODO here if we plan on adding more error handling
# mesh belongs to (mesh_dim). | ||
mesh, mesh_dim = _get_mesh_and_dim(src_mesh_cube) | ||
# mesh belongs to. | ||
mesh_dim = src_mesh_cube.mesh_dim() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Previous pseudocode version of this (L28) took mesh
as a parameter. Is it not necessary to do this anymore? Is there a possibility that a cube contains more than one mesh?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The previous pseudocode version was written before this functionality was implemented in iris. We've discussed the idea of a cube containing more than one mesh and we don't anticipate this possibility. In fact I don't believe it's possible to define such a thing under the current UGRID conventions.
return mesh | ||
|
||
|
||
def _flat_mesh_cube(): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you document what the object that this returns looks like? i.e. data is all ones?
src = _add_metadata(src) | ||
result = regrid_unstructured_to_rectilinear(src, tgt) | ||
|
||
expected_data = np.ones([5, 6]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not obvious to me why the expected data is [5,6] from the regrid operation, the test is relying on setup from another module in _flat_mesh_cube
. Could you add a comment explaining why this is the expected outcome?
* Unstructured scheme skeleton (#31) * create skeleton of unstructured regridding code This code is primarily based on Iris's current Area weighted regridding scheme, with the required adaptions for accepting unstructred UGRID data. * Unstructured Scheme - Basic GridInfo Handling (#32) Add basic `GridInfo` function and associated tests. It returns an ESMF regridding object for gridded data. * Unstructured scheme iris source (#33) * refresh cirrus-ci and nox * add iris artifact support * deal with special-case cirrus-ci quoting * review actions * Unstructured Scheme - Basic MeshInfo Handling (#36) * provide iris Mesh to MeshInfo conversion * test against iris feature branch * lint fixes and documentation * fix typo * address review comments. * change tests to better reflect UGRID orientation * address review comments. * address review comments * ASCII art fix * Unstructured Scheme - Cube Creation 2D (#39) * support creation of 2D cubes * lint fixes * rewrite test * remove mesh from _create_cube arguments * add TODO * add TODO * Unstructured Scheme - Mesh Fetching (#46) * support mesh fetching * fix docstring * lint fix * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * add docstrings * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * add and restructure tests * fix test * fix test * address review comments * address review comments Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Bill Little <bill.little@metoffice.gov.uk> Co-authored-by: Anna Booton <anna.booton@metoffice.gov.uk> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
* Unstructured scheme skeleton (SciTools#31) * create skeleton of unstructured regridding code This code is primarily based on Iris's current Area weighted regridding scheme, with the required adaptions for accepting unstructred UGRID data. * Unstructured Scheme - Basic GridInfo Handling (SciTools#32) Add basic `GridInfo` function and associated tests. It returns an ESMF regridding object for gridded data. * Unstructured scheme iris source (SciTools#33) * refresh cirrus-ci and nox * add iris artifact support * deal with special-case cirrus-ci quoting * review actions * Unstructured Scheme - Basic MeshInfo Handling (SciTools#36) * provide iris Mesh to MeshInfo conversion * test against iris feature branch * lint fixes and documentation * fix typo * address review comments. * change tests to better reflect UGRID orientation * address review comments. * address review comments * ASCII art fix * Unstructured Scheme - Cube Creation 2D (SciTools#39) * support creation of 2D cubes * lint fixes * rewrite test * remove mesh from _create_cube arguments * add TODO * add TODO * Unstructured Scheme - Mesh Fetching (SciTools#46) * support mesh fetching * fix docstring * lint fix * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * add docstrings * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * add and restructure tests * fix test * fix test * address review comments * address review comments Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Bill Little <bill.little@metoffice.gov.uk> Co-authored-by: Anna Booton <anna.booton@metoffice.gov.uk> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
This PR adds support for fetching meshes and mesh dimensions from iris. Originally we had planned on using a helper function
_get_mesh_and_dim
to do this, however iris has updated to the point where this functionality is given in one line, so for clarities sake I have replaced the helper function. Instead of testing the helper function, I have tested_regrid_unstructured_to_rectilinear__prepare
, where_get_mesh_and_dim
would have been called. I have also added a test forregrid_unstructured_to_rectilinear
to show that the whole regridder now functions as a result. Full test coverage can be added in a further PR.