_deform_mesh: bump _mesh_version + clear runner nav caches#191
Open
lmoresi wants to merge 1 commit into
Open
Conversation
Two cache-invalidation gaps in Mesh._deform_mesh, both exposed by direct _deform_mesh(coords) calls (e.g. every free-surface RK stage in the convection benchmark), which bypass the mesh.X.coords NDArray callback that normally performs this hygiene: 1. _mesh_version was not incremented. PR #182 introduced version-keyed kdtree navigation caches (_BaseMeshVariable._get_kdtree, Mesh._get_domain_kdtree) that gate their rebuild on _mesh_version. The mesh.X.coords callback bumps it; direct _deform_mesh() did not. Result: navigation kdtrees stay frozen on the undeformed mesh, so spatial lookups return pre-deform DOFs after the geometry has moved. PR #188 added _topology_version invalidation here but missed _mesh_version. 2. User-installed coord-identity nav caches were not cleared. A runner's restore_points_to_domain typically caches a kdtree keyed on id(mesh.X.coords). _deform_mesh replaces self._coords with a new object, but CPython reuses freed ids, so a fresh coords array can collide with the old id() and the staleness check false-negatives. Explicitly clearing _restore_kdt / _restore_coords_id defeats the id()-reuse hazard. Verified: _mesh_version now increments on a direct _deform_mesh() call (was frozen at 0 before). Matches the cache hygiene mesh.adapt() and _legacy_access already perform; brings _deform_mesh into line. Independent correctness fix; does not by itself resolve the separate free-surface convection feedback regression under investigation. Underworld development team with AI support from Claude Code
Contributor
There was a problem hiding this comment.
Pull request overview
This PR closes two cache invalidation gaps in Mesh._deform_mesh() that occur when _deform_mesh(coords) is called directly (bypassing the mesh.X.coords NDArray callback), ensuring version-gated navigation caches and runner-installed navigation helpers don’t remain stale after mesh deformation.
Changes:
- Increment
self._mesh_versioninside_deform_mesh()so version-keyed KDTree navigation caches rebuild on geometry updates. - Clear runner/user-installed navigation cache attributes (
_restore_kdt,_restore_coords_id) to avoid stale reuse due to CPythonid()reuse.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+1847
to
+1860
| # Bump the geometry-version counter so version-keyed | ||
| # kdtree navigation caches rebuild against the new DOF | ||
| # positions: _BaseMeshVariable._get_kdtree and | ||
| # Mesh._get_domain_kdtree both gate their rebuild on | ||
| # `_mesh_version`. PR #182 introduced those version-keyed | ||
| # caches; the mesh.X.coords callback path bumps | ||
| # _mesh_version, but direct _deform_mesh() calls (every | ||
| # free-surface RK stage) bypass that callback. Without | ||
| # this bump the navigation kdtrees stay frozen on the | ||
| # undeformed mesh — back-advected SL samples land at the | ||
| # wrong DOFs, corrupting the temperature field. PR #188 | ||
| # added _topology_version invalidation but missed this. | ||
| self._mesh_version += 1 | ||
| # Also nuke any *user-installed* navigation caches that |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two cache-invalidation gaps in
Mesh._deform_mesh, both triggered by direct_deform_mesh(coords)calls (which bypass themesh.X.coordsNDArray callback that normally performs this hygiene — e.g. every free-surface RK stage in the convection benchmark):_mesh_versionnot bumped. PR Consolidate and unify cached spatial indexing (KDTree) #182 introduced version-keyed kdtree navigation caches (_BaseMeshVariable._get_kdtree,Mesh._get_domain_kdtree) gated on_mesh_version. Themesh.X.coordscallback bumps it; direct_deform_mesh()did not — so navigation kdtrees stayed frozen on the undeformed mesh. PR Invalidate evaluate/DMInterp/topology caches on Mesh._deform_mesh #188 added_topology_versioninvalidation here but missed_mesh_version.restore_points_to_domaincaches a kdtree keyed onid(mesh.X.coords)._deform_meshreplacesself._coords, but CPython reuses freed ids → a fresh array can collide with the old id() and the staleness check false-negatives. Explicitly clearing_restore_kdt/_restore_coords_iddefeats the id()-reuse hazard.Brings
_deform_meshinto line with the cache hygienemesh.adapt()and_legacy_accessalready perform.Test plan
_mesh_versionincrements on a direct_deform_mesh()call (was frozen at 0 before the fix)Note: this is an independent correctness fix. It does not by itself resolve the separate free-surface convection feedback regression under investigation (verified — both fixes present in a clean build still reproduce the damped regime).
Underworld development team with AI support from Claude Code