Skip to content
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

Notes on implementation; scoping compared to draft conformance rules and ugrid-checks #17

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 61 additions & 2 deletions cc_plugin_ugrid/checker.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ def _check1_topology_dim(self, mesh):
if not self.meshes[mesh]['topology_dimension']:
m = 'Mesh does not contain the required attribute "topology_dimension"'
messages.append(m)
# PP: "R103" [email protected] check.py::L635

try:
assert self.meshes[mesh]['topology_dimension'] in [1, 2, 3]
Expand All @@ -44,6 +45,8 @@ def _check1_topology_dim(self, mesh):
self.meshes[mesh]['topology_dimension'],
type(self.meshes[mesh]['topology_dimension']))
messages.append(m)
# PP: "R104" [email protected] check.py::L646
# ***BUT ALSO*** ugrid-checks allows =0, rejects=3
else:
score += 1

Expand Down Expand Up @@ -82,6 +85,8 @@ def _check2_connectivity_attrs(self, mesh):
m = 'Mesh does not contain the required attribute "topology_dimension",'+\
' therefore any defined connectivity cannot be verified'
messages.append(m)
# PP: "R104" [email protected] check.py::L646
# ***NOTE*** this test is repeated, also occurs above, L038
out_of += 1
return self.make_result(level, score, out_of, desc, messages)

Expand All @@ -93,6 +98,10 @@ def _check2_connectivity_attrs(self, mesh):
if (not self.meshes[mesh][_conn]) and (self.meshes[mesh]['topology_dimension'] == _dim):
out_of += 1 # increment out_of, do not increment score
m = 'dataset is {}D, so must have "{}"'.format(_dim, _conn)
# PP: various, [email protected] ...
# @@ (_dim=1, _conn=edge_node) : "R112" check.py:L675
# @@ (_dim=2, _conn=face_node) : "R113" check.py:L681
# @@ (_dim=3, _conn=volume_node) : ==UNSUPPORTED==
return self.make_result(level, score, out_of, desc, messages)

# now we test individual connectivities -- here we will be incrementing the score
Expand All @@ -106,6 +115,9 @@ def _check2_connectivity_attrs(self, mesh):
# required per the standard and we want to check it exists
if order == 'nonstd':
self.__check_nonstd_order_dims__(mesh, _conn)
# PP: ???the result is discarded???
# it returns a Result object
# (which presumably may be "empty" ???)

if valid:
score += 1
Expand All @@ -115,6 +127,7 @@ def _check2_connectivity_attrs(self, mesh):

# check for optional attributes edge[face][volume]_coordinates
self.__check_edge_face_coords__(mesh, _conn)
# PP: result is ignored !!

return self.make_result(level, score, out_of, desc, messages)

Expand All @@ -135,7 +148,6 @@ def _check3_ncoords_exist(self, mesh):
Additionally, all node coordinates specified in a mesh must be defined
as variables in the dataset.
"""

level = BaseCheck.HIGH
score = 0
out_of = 0
Expand All @@ -146,13 +158,16 @@ def _check3_ncoords_exist(self, mesh):
self.meshes[mesh]['node_coordinates'] = []
if not self.meshes[mesh].get('topology_dimension'):
msg = 'Failed because no topology dimension exists'
# PP: "R104" [email protected] check.py::L646
# ***NOTE*** this test is repeated, also occurs above, L038, L164
messages.append(msg)
out_of += 1
return self.make_result(level, score, out_of, desc, messages)
else:
try:
ncoords = mesh.node_coordinates.split(' ')
assert len(ncoords) == self.meshes[mesh].get('topology_dimension')
# PP: not necessarily required ?? : see below ...
for nc in ncoords:
try:
out_of += 1
Expand All @@ -161,14 +176,27 @@ def _check3_ncoords_exist(self, mesh):
score += 1
except AssertionError:
msg = 'Node coordinate "{}" in mesh but not in variables'.format(nc)
# PP: "R108" [email protected] check.py::L714
# *and* one of "R105" (invalid) / "R106" (absent var)
# -> [email protected] check.py::L164/173
messages.append(msg)
except AttributeError:
msg = 'This mesh has no node coordinate variables'
# PP: "R108" [email protected] check.py::L714
# *and* "R105" [email protected] check.py::L151
out_of += 1
messages.append(msg)
except AssertionError:
msg = 'The size of mesh\'s node coordinates does not match' +\
' the topology dimension ({})'.format(self.meshes[mesh].get('topology_dimension'))
# from above, L168
# len(ncoords) == self.meshes[mesh].get('topology_dimension')
# PP: "A mesh must have node coordinates the same length as the
# value for mesh's topology dimension."
# ***This is not really true***. E.G. can have x,y,z coords
# defining the points of a 1D or 2D mesh.
# Also, in the message "size of mesh\'s node coordinates", the
# term "size" is very confusing
out_of += 1
messages.append(msg)

Expand Down Expand Up @@ -207,7 +235,14 @@ def _check4_edge_face_conn(self, mesh):
try:
dim1, dim2 = self.ds.variables[efc].shape # unpack the tuple
assert dim1 == self.meshes[mesh]['nedges'].size # compare to nedges
# PP: "R304/305/306/307" [email protected] check.py::L442
# PP: ugrid-checks checks the dims of the variable, **not** the
# shape - so it insists that it use the "correct" dim,
# NOT just one of the right size.
# PP: NOTE - the draft conformance, and ugrid-checks, actually
# allow these dims to be swapped -- might be a mistake ...
assert dim2 == 2 # should be equal to 2
# PP: "R308" [email protected] check.py::L485
score += 1
except AssertionError:
messages.append('Incorrect shape ({}, {}) of edge_face_connectivity array'.format(dim1, dim2))
Expand Down Expand Up @@ -320,6 +355,8 @@ def yield_checks(self):


def __check_edge_face_coords__(self, mesh, cty):
# PP: the whole of this is pointless,
# because when called the result is discarded : see above, L129
"""
Check the edge[face] coordinates of a given mesh.

Expand Down Expand Up @@ -349,6 +386,7 @@ def __check_edge_face_coords__(self, mesh, cty):
messages = []
desc = 'Edge coordinates point to aux coordinate variables representing' +\
' locations of edges (usually midpoint)'
# PP: test name "desc" needs generalising to "face or edge"

coordmap = {
'edge_node_connectivity': 'edge_coordinates',
Expand All @@ -359,11 +397,14 @@ def __check_edge_face_coords__(self, mesh, cty):
'edge_coordinates': 'nedges',
'face_coordinates': 'nfaces',
}
# PP: don't get this at all -- seems to require fixed names
# for face/edge dimensions ???

# do(es) the mesh(es) have appropriate connectivity? If not, pass
if (not self.meshes[mesh][cty]):
messages.append('No {}?'.format(cty))
return self.make_result(level, score, out_of, desc, messages)
# PP: in this case, out_of should = 0 ??

# first ensure the _coordinates variable exists
_c = coordmap[cty]
Expand All @@ -372,13 +413,18 @@ def __check_edge_face_coords__(self, mesh, cty):
except AttributeError:
messages.append('Optional attribute, not required')
return self.make_result(level, score, out_of, desc, messages)
# PP: in this case, out_of should = 0 ??

# if it exists, verify its length is equivalent to nedges
try:
for coord in coords.split(' '): # split the string
_coord_len = len(self.ds.variables[coord])
# PP: fails if named vars are missing from dataset ??
_dim_len = len(self.ds.dimensions[varmap[_c]])
# PP: don't get this at all -- seems to require fixed names
# for face/edge dimensions ???
assert _coord_len == _dim_len
# PP: prefer testing var.dimensions over var.shape ??
score += 1
except AssertionError:
m = '{} should have length of {}'.format(_c, varmap[_c])
Expand Down Expand Up @@ -493,10 +539,13 @@ def __check_edge_face_dim__(self, mesh, dim_var):
except AttributeError:
msg = 'Mesh does not contain {}, '.format(dim_var)+\
'required when connectivity in non-standard order.'
# PP: "" [email protected] check.py::L836
return False, msg
except KeyError:
msg = 'Edge dimension defined in mesh, not defined in dataset ' +\
'dimensions'
# PP: message should say "face or edge dimension defined..." ??
# PP: "R115/117" [email protected] check.py::L781
return False, msg


Expand All @@ -522,18 +571,27 @@ def _validate_nc_shape(self, mesh, cty):
assert cty in ['edge_node_connectivity', 'face_node_connectivity', 'volume_node_connectivity']
conn_array_name = mesh.getncattr(cty)
except AssertionError:
# PP: CAN'T happen, given the possible args this is called with
# - see above, L94
return False, None # should never get this, right?
except AttributeError:
return False, None

# use name of array to get that variable from the dataset
_array = self.ds.variables.get(conn_array_name)
# PP: this seems unsafe -- if a connectivity variable specified in
# a mesh does not exist, checker falls over ?!?
# This does NOT happen for coordinates.
# : See cc_plugin_ugrid/tests/checker_test.py:176

_d1name, _d2name = _array.dimensions # tuple of strings
# PP: likewise, unsafe if var doesn't have exactly 2 dims
dim1 = self.ds.dimensions[_d1name] # access the dimension objects
dim2 = self.ds.dimensions[_d2name]

# check against dimensions of dataset
try:
# PP: surely CAN'T fail, as dims came from an actual variable ??
assert dim1.name in self.ds.dimensions.keys()
assert dim2.name in self.ds.dimensions.keys()
except AssertionError:
Expand All @@ -550,6 +608,7 @@ def _validate_nc_shape(self, mesh, cty):
_dim2size = 2
elif cty == 'face_node_connectivity':
_dim1 = 'nfaces'
# PP: does not support quad meshes ???
_dim2size = 3
else:
raise NotImplementedError # haven't dealt with real 3D grids yet
Expand Down
3 changes: 2 additions & 1 deletion cc_plugin_ugrid/tests/checker_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,8 @@ def nc(self, ncpath):
def setUp(self):
"""Method to run before every test"""

dspath = os.path.join(os.getcwd(), 'cc_plugin_ugrid', 'resources', 'ugrid.nc')
# dspath = os.path.join(os.getcwd(), 'cc_plugin_ugrid', 'resources', 'ugrid.nc')
dspath = '/home/h05/itpp/git/ioos_cc-plugin-ugrid/cc_plugin_ugrid/resources/ugrid.nc'
dataset = self.nc(dspath)
self.checker = UgridChecker()
self.checker.setup(dataset) # initializes dict of meshes
Expand Down