Skip to content

Invalidate evaluate/DMInterp/topology caches on Mesh._deform_mesh#188

Merged
lmoresi merged 2 commits into
developmentfrom
feature/mesh-deform-cache-invalidation
May 14, 2026
Merged

Invalidate evaluate/DMInterp/topology caches on Mesh._deform_mesh#188
lmoresi merged 2 commits into
developmentfrom
feature/mesh-deform-cache-invalidation

Conversation

@lmoresi
Copy link
Copy Markdown
Member

@lmoresi lmoresi commented May 14, 2026

Summary

After a coordinate change in Mesh._deform_mesh, three mesh-level caches were left holding state computed against the pre-deform geometry. mesh.adapt() and _legacy_access (with writeable vars) already invalidate these — this PR brings _deform_mesh into line with the same hygiene.

Caches invalidated:

  • self._evaluation_hash and self._evaluation_interpolated_results (the legacy uw.function.evaluate fast-path cache)
  • self._dminterpolation_cache (stores DMInterpolation structures keyed by (coord_hash, dofcount) — encode cell residency for each query coord plus the reference-to-physical mapping)
  • self._topology_version (bumped; downstream caches read this counter as an invalidation signal)

Empirical impact

Tested against a free-surface convection case where _deform_mesh is called many times per step. Bit-identical trajectory before and after this patch — the cache was rarely returning stale values on that specific problem, because the trace-back path uses fresh coord arrays each step (cache key changes, mostly misses).

So this PR is principled hygiene rather than a curative fix for a known bug. It protects against future failure modes where user code re-uses a coord array across mesh deformations (cache key matches, stale entry returned).

What it doesn't do

No new API surface. No behaviour change for code that doesn't hit the caches. mesh.adapt() and _legacy_access already do this; the inconsistency was that _deform_mesh didn't.

Test plan

  • Full CI suite (expect bit-identical results — these caches are rarely hit on stale entries)
  • No performance regression (invalidation is O(1) for the hash nulls; invalidate_all on the DMInterpolation cache is also cheap)

Files changed

  • src/underworld3/discretisation/discretisation_mesh.py (+13 lines)

Underworld development team with AI support from Claude Code

After a coordinate change, three mesh-level caches need to be marked
stale so subsequent lookups recompute against the new geometry:

  - self._evaluation_hash + self._evaluation_interpolated_results
    (the legacy uw.function.evaluate fast-path cache)
  - self._dminterpolation_cache (DMInterpolation structures keyed on
    coord-hash; stores cell residency + reference-to-physical maps)
  - self._topology_version (a counter that downstream caches consult)

mesh.adapt() (line 3792, 3850-3853) and _legacy_access (line 1848-1853)
already invalidate these. _deform_mesh did not, leaving a gap where
the same uw.function.evaluate query against the same coord array
could hit a stale cache built on the pre-deform geometry.

Empirically the gap was not load-bearing for the convection problem
that motivated this audit (the bit-identical trajectory before and
after this patch confirms the cache was rarely hit on stale entries
in that specific path — trace-back queries use fresh coord arrays
each step, so the (coord_hash, dofcount) key changes and the cache
mostly misses). Land as a hygiene improvement: it brings _deform_mesh
into line with the other mesh-modifying operations and protects
against future failure modes where _deform_mesh is followed by an
evaluate at the same coord array (e.g. user code that holds onto a
coord array across deformations).

No behaviour change for code that doesn't hit the caches. No new API
surface.

Underworld development team with AI support from Claude Code
Copilot AI review requested due to automatic review settings May 14, 2026 11:02
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR aligns Mesh._deform_mesh() cache invalidation with other mesh-update paths so geometry-dependent evaluation and interpolation state is cleared after coordinate changes.

Changes:

  • Clears legacy evaluation cache state after mesh deformation.
  • Invalidates the mesh DMInterpolation cache.
  • Bumps _topology_version as an invalidation signal.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +1819 to +1824
self._evaluation_hash = None
self._evaluation_interpolated_results = None
if hasattr(self, '_dminterpolation_cache'):
self._dminterpolation_cache.invalidate_all(
reason="mesh deformed")
self._topology_version += 1
- Resolve conflict with the kdtree-refactor PR landed on development.
  That PR wrapped _deform_mesh's body in `with self._mesh_update_lock:`;
  the cache-invalidation block needed re-indenting into the lock
  context (semantically equivalent — cache invalidation runs under the
  same lock as the coord update it protects).

- Add tests/test_0825_deform_mesh_cache_invalidation.py with three
  regression checks:
    * _topology_version increments on _deform_mesh
    * _evaluation_hash and _evaluation_interpolated_results are nulled
    * _dminterpolation_cache is emptied after the deform

  Each test warms the relevant cache via uw.function.evaluate before
  calling _deform_mesh, so the assertions exercise the invalidation
  path (not just the initial-state property).

Underworld development team with AI support from Claude Code
@lmoresi lmoresi merged commit ae4b447 into development May 14, 2026
1 check passed
@lmoresi lmoresi deleted the feature/mesh-deform-cache-invalidation branch May 14, 2026 12:00
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.

2 participants