`_MeshIndexSet` first pass by trexfeathers · Pull Request #7149 · SciTools/iris

trexfeathers · 2026-06-10T10:27:39Z

No description provided.

…ndex-set

trexfeathers · 2026-06-10T11:11:00Z

pre-commit.ci autofix

trexfeathers · 2026-06-10T14:36:38Z

@stephenworsley please can you review the approach I have used with this code, accepting that various TODOs are deferred to #7151. Thanks!

stephenworsley · 2026-06-10T14:59:59Z

+        raise NotImplementedError()
+
+    def is_view_of(self, other: MeshXY) -> bool:
+        """Whether this instance is either itself, or a view of the given :class:`MeshXY`.


Have you considered how this ought to behave when you've sliced or indexed a _MeshIndexSet? If I'm interpreting the code correctly, it looks like you ought to end up with another smaller _MeshIndexSet. In such a case, I could imagine you might want this method to also return True.

I also wonder if it might be worth considering two different methods, one for comparing a _MeshIndexSet to a MeshXY, another for comparing two _MeshIndexSets. I haven't entirely thought through which contexts you might expect to call these methods so I've not come to a conclusion on this myself yet.

This is not intended to be possible, and I've taken some steps to avoid it happening:

iris/lib/iris/mesh/components.py

Lines 3619 to 3630 in ec132b0

case _MeshIndexSet():

# Should not base an index set on another index set - base

# on the original mesh instead.

# TODO: do we need to double-check that self.location

# matches mesh_index_set.location? Any other matching to

# check too?

mesh_index_set = mesh

kwargs = dict(

mesh=mesh_index_set.mesh,

location=mesh_index_set.location,

indices=mesh_index_set.indices[keys],

)

trexfeathers · 2026-06-10T16:01:07Z

+        result = [self.to_MeshCoord(location=location, axis=ax) for ax in self.AXES]
+        return tuple(result)
+
+    def is_view_of(self, other: "MeshXY") -> bool:


Offline conversation with @stephenworsley: is_view_of is not necessary, and any value it might offer is offset by potential confusion. We should remove it.

trexfeathers · 2026-06-10T16:38:33Z

        result_dict = {k: v for k, v in self._members.items() if id(v) in result_ids}
        return result_dict

+    def index(


Offline conversation with @stephenworsley: this concept of mutability only really extends to altering the membership of the Manager, there is no decent way ¹ to protect the coordinates/connectivities themselves from modification, since users are free to assign those to their own variables, and/or to modify the arrays within them.

If we're comfortable with this 'insecurity', then we could consider generating new constituent coordinates/connectivities that explicitly share the same NumPy array as the ones on the original Mesh (as opposed to directly using the slicing API, since that explicitly creates a copy). That would allow the MeshIndexSet to be a true view, with live updating, potentially avoiding the weirdness of re-indexing every time a Manager is requested.

Footnotes

I need to read up on Ensure MeshCoord points and bounds always agree with the Mesh (while keeping Mesh mutable) #4757 ↩

Didn't turn out as expected - indexing arrays using another array of integers NEVER returns a view - but I've got something working.

a6542ee

trexfeathers · 2026-07-06T17:32:57Z

~~Note that currently printing MeshCoords based on _MeshIndexSet is very slow. Computing points and bounds seems OK, which is the bit I was originally worried about.~~

EDIT: fixed by ab688cf

stephenworsley · 2026-07-08T14:55:47Z

+    def cube_dims(self, cube):
+        raise NotImplementedError()
+
+    def as_mesh(self) -> MeshXY:


It's worth considering if we want to realise the data when we call this. This ought to be the most "robust" way of handling this operation. There's maybe a case for keeping this lazy, in order to minimise memory use, but I'm not convinced this is the correct behaviour or even something we can properly implement.

stephenworsley

I've recorded some stuff to follow up later, but this all looks good in principle. A good start for the feature branch, good work!

stephenworsley · 2026-07-08T16:19:23Z

+                if connectivity.location_axis == 1:
+                    new_values = new_values.T
+                if connectivity.start_index == 1:
+                    new_values = new_values + 1


Is there a reason this isn't just new_values += connectivity.start_index?

stephenworsley · 2026-07-09T10:20:07Z

+
+                # Map node indices in "values" to their new zero-based positions
+                #  in "node_indices".
+                order = node_indices.argsort()


It looks like this code is focused on ***_node_connectivity. I suspect this might fail with something like face_edge_connectivity. It may be worth just throwing an error in such cases for the time being. With that said, I don't think this is a blocker to moving forward with work on this feature branch.

stephenworsley · 2026-07-09T10:34:26Z

+            if connectivity is not None:
+                indexing = indices_dict[connectivity.location]
+            if indexing is not None:
+                new_values = connectivity.indices_by_location(


Doesn't need addressing right now, but I think this bit of code could definitely benefit from thinking about making these variable names clearer. In particular, I think this could benefit from being something other than new_values.

stephenworsley · 2026-07-09T10:43:50Z

+                order = node_indices.argsort()
+                old_sorted = da.from_array(node_indices[order])
+                new_ids_sorted = da.arange(len(node_indices))[order]
+                positions = functools.partial(da.searchsorted, old_sorted)


new_ids_sorted and positions should be the same for every loop so we should probably be calculating these outside the for loop. Maybe worth adding a bit of logic to ensure they aren't called unnecessarily.

stephenworsley · 2026-07-09T11:02:06Z

+                    array=new_ids_sorted,
+                    indexing=new_values,
+                    pre_index=positions,
+                )


I think I finally got my head around what this bit of logic does. While there's bits I think can be tidied up, I'm happy with what this does in principle.

trexfeathers added 7 commits June 9, 2026 15:00

Use Monotonicity error in Cube indexing, plus test coverage.

52bcda4

Implement MeshIndexSetMetadata.

848e822

Changes to cube.mesh type hinting.

6b151f5

Create experimental/mesh_coord_indexing.py .

8fc2e79

Implement MeshIndexSet.

40779c4

Render _MeshIndexSet in docs.

c093fd2

Merge remote-tracking branch 'upstream/FEATURE_index_set' into mesh-i…

f1b54da

…ndex-set

trexfeathers added Feature: UGRID Type: Feature Branch Highlight this for a feature branch labels Jun 10, 2026

Easy CI fixes.

e1a220f

pre-commit-ci Bot and others added 6 commits June 10, 2026 11:11

style: pre-commit fixes

8e5722b

Add TODO for later type hinting.

8b7ec5e

Missing sphinx-needs item.

c20d53c

Remove linkable UGRID footnotes to avoid duplication warning.

70cb63c

Fix doctest.

3724c3e

TODO comments.

047c40d

trexfeathers mentioned this pull request Jun 10, 2026

Create MeshIndexSet class #6121

Open

4 tasks

trexfeathers added 2 commits June 10, 2026 15:00

Fix doctest.

dcbb5e0

TODO comment.

36dd627

trexfeathers mentioned this pull request Jun 10, 2026

Finalise _MeshIndexSet work from #7149 #7151

Open

21 tasks

trexfeathers requested a review from stephenworsley June 10, 2026 14:36

Fix doctest.

ec132b0

trexfeathers mentioned this pull request Jun 10, 2026

Index MeshCoord using new MeshXY or _MeshIndexSet #6127

Draft

3 tasks

trexfeathers linked an issue Jun 10, 2026 that may be closed by this pull request

Create MeshIndexSet class #6121

Open

4 tasks

trexfeathers mentioned this pull request Jun 10, 2026

Write tests for UGRID Location Index Set data model #7152

Open

trexfeathers marked this pull request as ready for review June 10, 2026 15:02

stephenworsley reviewed Jun 10, 2026

View reviewed changes

trexfeathers commented Jun 10, 2026

View reviewed changes

scitools-ci Bot added this to 🚴 Peloton Jun 11, 2026

trexfeathers added 2 commits July 1, 2026 16:51

Achieve an accurate view of Mesh via Dask arrays and timestamps.

a6542ee

Remove Mesh.is_view_of.

000c259

trexfeathers commented Jul 1, 2026

View reviewed changes

Comment thread lib/iris/mesh/components.py Outdated

Fixes after rough testing.

2519f2f

Prevent overzealous updates.

ab688cf

stephenworsley reviewed Jul 8, 2026

View reviewed changes

stephenworsley approved these changes Jul 9, 2026

View reviewed changes

	case _MeshIndexSet():
	# Should not base an index set on another index set - base
	# on the original mesh instead.
	# TODO: do we need to double-check that self.location
	# matches mesh_index_set.location? Any other matching to
	# check too?
	mesh_index_set = mesh
	kwargs = dict(
	mesh=mesh_index_set.mesh,
	location=mesh_index_set.location,
	indices=mesh_index_set.indices[keys],
	)

Uh oh!

Conversation

trexfeathers commented Jun 10, 2026

Uh oh!

trexfeathers commented Jun 10, 2026

Uh oh!

trexfeathers commented Jun 10, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

trexfeathers Jun 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Footnotes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

trexfeathers commented Jul 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

stephenworsley left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

trexfeathers Jun 10, 2026 •

edited

Loading

trexfeathers commented Jul 6, 2026 •

edited

Loading