Infrastructure for moving VertexOnlyMeshes by achanbour · Pull Request #5294 · firedrakeproject/firedrake

achanbour · 2026-07-29T10:22:25Z

This PR contains the bulk of the work for supporting mutable VOMs.

Broadly, this introduces 3 new features:

A VertexOnlyMeshMutator that handles two update regimes: one that updates the local (reference-space) state of the VOM and one that does a full (topological) rebuild of the VOM
A conditional caching mechanism wherein FunctionSpace properties get recomputed when the topology version number of their underlying mesh changes
A Function migration mechanism (both lazy and eager) which ensures that Function values refresh accordingly with the VOM's re-ordering.

While I have tried to make 2 and 3 as general as possible, I have only written and tested them to work for DG0-style Function Spaces defined on VOMs.

…on-vom

…o achanbour/mutable-fs-on-vom

…ng mechanism

…tractMeshTopology

…l branch

leo-collins · 2026-07-29T11:51:44Z

        # LRU cache for expressions assembled onto this function
        self._expression_cache = cachetools.LRUCache(maxsize=50)

+        self._mesh_topo = self._function_space.topological.mesh()  # the MeshTopology object


Also need to be careful here with MeshSequences

If self._function_space.topological.mesh() returns a MeshSequence then the versioning mechanism still works as I just added a _topology_version property inside the MeshSequence class:

@property def _topology_version(self): return tuple(mesh._topology_version for mesh in self._meshes)

The migration itself, however, wouldn't work because it currently checks that the Function being updated is one defined on a VOM:

def _rebuild_function(self, current_version): # Check if the mesh on which the function is defined is a VertexOnlyMesh if not isinstance(self._mesh_topology, VertexOnlyMeshTopology): raise TopologyVersionMismatchError( "The mesh topology has changed since this Function was created, \ and migration is currently only supported for Functions defined on VertexOnlyMeshes. \ Please re-create this Function on the updated mesh." )

I think this is fine for now? It would seem wrong to only migrate the VOM component for a MeshSequence and leave everything else stale.

connorjward

This is a big change. Lots of feedback but this is really promising.

connorjward · 2026-07-29T11:40:51Z

+        # and migrate only ones in the intersection
+
+        # NOTE: This doesn't collect reference-cycle garbage identically on all ranks
+        gc.collect()


I would advocate for using this function (just in pyop3 currently but could come in early) instead here. Just temporarily disable the GC for the duration of this function.

But if you don't do things eagerly then this might not be an issue, by definition if you are explicitly asking for a function to be migrated then you must hold another ref to it.

connorjward · 2026-07-29T12:01:36Z

+        ref_coords_func = self.vom.reference_coordinates
+        ref_coords_func.dat.data[:] = new_refcoords
+
+    def rebuild_vom(self, absorbed_vom_indices=None):


I much prefer the pattern where this a method of the VertexOnlyMesh.

Rebuild the VertexOnlyMesh using the state already stored on the VOM.

I think this demonstrates what I mean. The current pattern is:

Partially mutate the VoM

Hand this partially mutated VoM to another class that finishes the mutation

So 1) should be in the VertexOnlyMeshMutator and 2) should be a method of the VertexOnlyMeshTopology?

I don't think a separate class (VertexOnlyMeshMutator) is needed. All of this logic naturally lives on the VoM.

connorjward · 2026-07-29T12:05:03Z

+        # cell_numbering and vertex_numbering are PETSc ISes - translation tables between PETSc numbering of mesh entities and that of Firedrake's
+        # For each plex point, they store two integers: the dof count and an offset (which happens to be the Firedrake's number of that plex point)
+        topology._cell_numbering, _ = topology.create_section(entity_dofs)
+        topology._vertex_numbering = topology._cell_numbering  # holds for VOM only


All this crud has to be here because the __init__ method does a whole lot of magic. I think if we made topology._vertex_numbering into a topology_cached_property then a lot of this could just go away. It is appealing for us to just tweak the swarm, increment the topology count, and then have everything else recompute as needed.

connorjward · 2026-08-03T08:48:36Z

+                        f"Failed to migrate the Function across multiple topology changes: \
+                        the intermediate topology mapping from version {v-1} to {v} could not be found."
+                    )
+                chained_sf = step_sf.compose(chained_sf)  # V -> V-2


I am suggesting chaining the migrations as distinct steps with per-migration SFs, instead of building a composed SF that does it in one shot. Consider the k-2 to k example you showed. I'm basically saying to do

step_sf1 = self._mesh_topology._topology_step_sfs.get(...) self._data = migrate_dg0_dat(self._data, FS_topo, step_sf1) step_sf2 = self._mesh_topology._topology_step_sfs.get(...) self._data = migrate_dg0_dat(self._data, FS_topo, step_sf2)

instead of building the composed thing.

Also you're building the composed SF every time you migrate a function. If you have lots of functions with different topology versions that could get expensive.

And lastly I think it is reasonable to think about cases where the migration is done via a non-SF method. For example if you have some sort of mesh adaptivity you might refine a cell into some smaller cells. To transfer the data you'd have to do an interpolation. We can stack interpolation operations but I don't know how we would compose them into a single 'super interpolation'.

…cofunction

achanbour · 2026-08-05T15:17:15Z

@connorjward regarding your earlier comment about chaining migrations instead of composing the step SFs:
migrate_dg0_dat takes as one of its arguments the topological FS describing the new cell-node layout of the topology we're migrating into i.e., the latest version. But migrating from version k-2 to k requires us knowing the intermediate topology state (at version k-1) which is currently not preserved (all fields such as _cell_numbering, _vertex_numbering entity classes etc. are overwritten to reflect the last version k). All we have left from version k-1 is effectively the step SF k-2 -> k-1 hence why SF composition works nicely.

For transferring data through interpolation, it would make sense to save a snapshot of the topological FS at each intermediate version, in which case stacking the migration/interpolation operations would work. At first impression, it doesn't sound like we're saving much more than simply doing the SF composition. Also, for the DG0 VOM case I'm dealing with now, composing SFs seems the most natural thing to do. What do you think?

connorjward · 2026-08-05T16:32:50Z

But migrating from version k-2 to k requires us knowing the intermediate topology state (at version k-1) which is currently not preserved (all fields such as _cell_numbering, _vertex_numbering entity classes etc. are overwritten to reflect the last version k). All we have left from version k-1 is effectively the step SF k-2 -> k-1 hence why SF composition works nicely.

Ahh yes. Bloody mutable state... OK I see that my previous proposal is not the right way to design the abstraction. Crack on.

connorjward · 2026-08-05T16:35:39Z

+
+    def _migrate_to_current_topology_version(self) -> None:
+        """Migrate this coordinateless function's data to the current topology version."""
+        _migrate_dg0_coefficient(self, self._function_space)


This is the place where we should check that we are actually DG0 and fail appropriately (NotImplementedError)

connorjward · 2026-08-05T16:37:46Z

+        return self._dat
+
+    @dat.setter
+    def dat(self, value):


We should have a comment explaining things here because this is an extremely strange pattern.

connorjward · 2026-08-05T16:38:43Z

+    def dat(self, value):
+        if value is self._dat:
+            return
+        raise AttributeError("A Function's Dat cannot be replaced directly.")


Suggested change

raise AttributeError("A Function's Dat cannot be replaced directly.")

raise AttributeError("The 'dat' of a function cannot be changed")

small thing but I think this is a bit clearer

connorjward · 2026-08-05T16:40:14Z

It is not a requirement for this PR, but you can probably now see that Function and Cofunction share an awful lot of code. It is somewhere on my TODO list to build a parent FunctionSpaceData (when the old version of that class dies) class for the shared functionality. One day...

achanbour added 17 commits July 13, 2026 17:42

transferred all changes related to mutating FS on VOMs

1e1eb33

transferred all changes related to mutating FS on VOMs

886b671

Merge remote-tracking branch 'origin/main' into achanbour/mutable-fs-…

c1c9579

…on-vom

Merge remote-tracking branch 'origin/achanbour/mutable-fs-on-vom' int…

ec59875

…o achanbour/mutable-fs-on-vom

generalise topology version to arbirary mesh topologies + fixed cachi…

3b97dce

…ng mechanism

linting

88add8a

improvements over the caching mechanism

852e255

fixed mesh versioning

bcab3f6

added topology version on ExtrudedMesh which doesn't inherit from Abs…

4c34822

…tractMeshTopology

transferred new functionalities related to mutating VOMs from persona…

723f032

…l branch

merged with main

6cdd99f

review + tidy

f947410

minor fixes + added tests for vom mutation

01b18ae

added fs mutation test + fixed vom mutation test

5150cb5

added more tests and fixes

fa299d0

linting

03defe0

more linting + parametrized parent mesh type in VOM mutation tests

1c104a2

achanbour requested a review from connorjward July 29, 2026 10:23

leo-collins reviewed Jul 29, 2026

View reviewed changes

Comment thread firedrake/functionspaceimpl.py Outdated

connorjward requested changes Jul 29, 2026

View reviewed changes

leo-collins reviewed Jul 29, 2026

View reviewed changes

Comment thread firedrake/vertexonly_mutation.py Outdated

achanbour mentioned this pull request Jul 29, 2026

Mutable FunctionSpaces #5243

Closed

leo-collins reviewed Jul 29, 2026

View reviewed changes

Comment thread firedrake/function.py Outdated

first set of changes

9b5f924

leo-collins reviewed Jul 31, 2026

View reviewed changes

Comment thread firedrake/exceptions.py Outdated

connorjward reviewed Aug 3, 2026

View reviewed changes

achanbour added 2 commits August 3, 2026 10:10

Merge remote-tracking branch 'origin/main' into achanbour/vom-mutation

d8fa4f0

fixes

54ad62c

achanbour and others added 10 commits August 3, 2026 11:47

fixes + changed the swarm array access pattern

f331138

fixed the caching

a6e5742

added a topology version number for MeshSequence

446ced1

fixed attribute name

c61fb93

changed Function migration method to do a single collective broadcast

223ccea

more fixes from the feedback

cf1065b

added setter to dat

380ae71

changes to function registration and eager migration methods

b56fb00

Merge branch 'main' into achanbour/vom-mutation

b340407

fixed migration method

577eb25

achanbour force-pushed the achanbour/vom-mutation branch from d0efc88 to 577eb25 Compare August 4, 2026 12:20

achanbour added 3 commits August 5, 2026 12:32

moved migration logic to coordinateless function and duplicated into …

52ab5c3

…cofunction

split vom rebuild method into several sub-methods

ba6f46c

moved FS mutation tests to separate file

876c420

connorjward requested changes Aug 5, 2026

View reviewed changes

	raise AttributeError("A Function's Dat cannot be replaced directly.")
	raise AttributeError("The 'dat' of a function cannot be changed")

Uh oh!

Conversation

achanbour commented Jul 29, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

connorjward left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

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!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

achanbour commented Aug 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

connorjward commented Aug 5, 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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

achanbour commented Aug 5, 2026 •

edited

Loading