Skip to content

Ugrid save#4303

Closed
pp-mo wants to merge 26 commits into
SciTools:mesh-data-modelfrom
pp-mo:ugrid_save_wip
Closed

Ugrid save#4303
pp-mo wants to merge 26 commits into
SciTools:mesh-data-modelfrom
pp-mo:ugrid_save_wip

Conversation

@pp-mo

@pp-mo pp-mo commented Sep 1, 2021

Copy link
Copy Markdown
Member

🚀 Pull Request

Description

Beginnings of real save code.

Current status :

  • fairly complete initial save code
  • simple tests using UGRID cdl example cases
  • targetted testing almost complete (...tests/unit/fileformats/netcdf/test_Saver__ugrid.py)
    • still TODO:
      • connectivities with missing points (coming next)
      • optional connectivities
      • edge-based meshes (topology-dim = 1)
      • alternate start-index
      • ... and more ?
  • spurious test failures = cartopy coastlines fetch timeout
    • TODO: fix by rebasing, after fixes from 'main' are pulled in to 'mesh-data-model'
    • ?? OR rebase to 'main', if we merge the 'mesh-data-model' feature branch back to main

NOTE :
at present, the first commit is that shown in #4301
Until/unless that itself is merged, it may make more sense to view this relative to the #4301 code


Consult Iris pull request check list

@pp-mo pp-mo changed the title Ugrid save wip Ugrid save Sep 2, 2021
@@ -0,0 +1,75 @@
dimensions:
Mesh2_edge_N_faces = 2 ;

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

N.B. this use of separate dims for the last dim of different connectivity types is by design,
but it could seem wasteful.
we could change this, even create all "small" dimension values only once + re-use them

@pp-mo pp-mo requested a review from bjlittle September 2, 2021 13:42
Comment thread lib/iris/tests/integration/experimental/test_ugrid_save.py Outdated
def test_example_roundtrips(self):
# Check that save-and-loadback leaves Iris data unchanged,
# for data derived from each UGRID example CDL.
# Snapshot the result of saving the example cases.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
# Snapshot the result of saving the example cases.

Copied in error

@bjlittle bjlittle self-assigned this Sep 3, 2021
@pp-mo

pp-mo commented Sep 7, 2021

Copy link
Copy Markdown
Member Author

At the risk of some confusion, I'm going to update this with subsequent commits.
Mostly because I fixed some nasty bugs.
However, this is still very much preliminary :

  1. I have coded some really basic testcases, and it now passes those, but there is much still to consider
  2. the current saver code is now a bit hacky + can probably be tidied somewhat
  3. the way this has evolved may have introduced extra unnecessary changes from the preceding code, so that might also be improved. (though could also conflict with overall complexity, i.e. against point (2))

Comment thread lib/iris/tests/unit/fileformats/netcdf/test_Saver__ugrid.py Outdated
vector_dim_coords = [
coord for coord in dim_coords if id(coord) not in scalar_coord_ids
]
if cube.mesh is None:

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've just done this for now, as it was irritating me + making debugging hard (not to easily see when there is a mesh or not).
This probably belongs in it's own PR.

@trexfeathers trexfeathers deleted the branch SciTools:mesh-data-model September 14, 2021 13:12
@pp-mo pp-mo reopened this Sep 14, 2021
@pp-mo

pp-mo commented Sep 14, 2021

Copy link
Copy Markdown
Member Author

Rebased, should now pass tests.

# metadata comparison
eq = self.metadata == other.metadata
if eq:
eq = self.shape == other.shape

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This fixes it so that connectivities of different shapes can compare correctly (i.e. returns False).
Was failing with error, due to problems of array comparison.

@pp-mo pp-mo closed this Sep 15, 2021
@pp-mo pp-mo mentioned this pull request Sep 15, 2021
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants