Skip to content

Fix mesh.X.coords stale after mesh deformation - #126

Merged
lmoresi merged 1 commit into
developmentfrom
bugfix/deform-mesh-stale-coords
Apr 22, 2026
Merged

Fix mesh.X.coords stale after mesh deformation#126
lmoresi merged 1 commit into
developmentfrom
bugfix/deform-mesh-stale-coords

Conversation

@lmoresi

@lmoresi lmoresi commented Apr 21, 2026

Copy link
Copy Markdown
Member

Summary

  • mesh.X.coords returned pre-deformation coordinates after _deform_mesh() because self._coords held a numpy view of the old coordinate buffer
  • nuke_coords_and_rebuild (called by _deform_mesh) replaces the DM's coordinate vector via createCoordinateSpace, but self._coords was never updated to view the new buffer
  • Fix: rebuild the NDArray_With_Callback wrapper from the current DM coordinate vector after nuke_coords_and_rebuild, preserving the deformation callback

Reproducer (from @NengLu)

mesh = uw.meshing.StructuredQuadBox(elementRes=(50, 50), ...)
# ... solve for displacement, compute new coords ...
mesh._deform_mesh(mesh.X.coords + displacement)

# Before fix: mesh.X.coords shows original (undeformed) coordinates
# After fix: mesh.X.coords matches mesh.dm.getCoordinatesLocal()

Test plan

  • NengLu's reproducer: mesh.X.coords matches mesh.dm.getCoordinatesLocal() after deformation
  • @NengLu to verify in their workflow

Closes #122

Underworld development team with AI support from Claude Code

_deform_mesh updates the DM coordinate vector and calls
nuke_coords_and_rebuild, which may replace the internal coordinate
buffer (via createCoordinateSpace).  The self._coords NDArray view
still pointed to the old buffer, so mesh.X.coords returned
pre-deformation values.

Fix: rebuild self._coords from the current DM coordinate vector
after nuke_coords_and_rebuild, preserving the update callback.

Closes #122

Underworld development team with AI support from Claude Code (https://claude.com/claude-code)
Copilot AI review requested due to automatic review settings April 21, 2026 07:09

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 fixes a correctness issue in Underworld3 mesh deformation where mesh.X.coords could continue to reference the pre-deformation PETSc coordinate buffer after _deform_mesh() triggers nuke_coords_and_rebuild() (which may internally replace the DM coordinate vector via createCoordinateSpace).

Changes:

  • After nuke_coords_and_rebuild(), rebuild self._coords as a fresh NDArray_With_Callback view of the current dm.getCoordinatesLocal() buffer.
  • Preserve the existing deformation callback(s) by re-attaching prior callbacks to the rebuilt _coords wrapper.

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

Comment on lines +1290 to +1296
old_callbacks = getattr(self._coords, "_callbacks", [])
self._coords = underworld3.utilities.NDArray_With_Callback(
numpy.ndarray.view(
self.dm.getCoordinatesLocal().array.reshape(-1, self.cdim)
),
owner=self,
)

Copilot AI Apr 21, 2026

Copy link

Choose a reason for hiding this comment

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

When rebuilding self._coords, only the callback functions are preserved. Any existing NDArray_With_Callback state (e.g. _callback_enabled from disable_callbacks() or _disable_inplace_operators) is reset to defaults, which can silently change behavior for callers that intentionally disabled callbacks. Consider copying these flags from the old self._coords onto the new instance (and copy the callback list rather than referencing it) so the wrapper rebuild is behavior-preserving beyond just the callback functions.

Suggested change
old_callbacks = getattr(self._coords, "_callbacks", [])
self._coords = underworld3.utilities.NDArray_With_Callback(
numpy.ndarray.view(
self.dm.getCoordinatesLocal().array.reshape(-1, self.cdim)
),
owner=self,
)
old_coords = self._coords
sentinel = object()
old_callbacks = list(getattr(old_coords, "_callbacks", []))
old_callback_enabled = getattr(old_coords, "_callback_enabled", sentinel)
old_disable_inplace_operators = getattr(
old_coords, "_disable_inplace_operators", sentinel
)
self._coords = underworld3.utilities.NDArray_With_Callback(
numpy.ndarray.view(
self.dm.getCoordinatesLocal().array.reshape(-1, self.cdim)
),
owner=self,
)
if old_callback_enabled is not sentinel:
self._coords._callback_enabled = old_callback_enabled
if old_disable_inplace_operators is not sentinel:
self._coords._disable_inplace_operators = (
old_disable_inplace_operators
)

Copilot uses AI. Check for mistakes.
@NengLu

NengLu commented Apr 21, 2026

Copy link
Copy Markdown
Contributor

Test shows the fix works, thanks.

@lmoresi
lmoresi merged commit fc63645 into development Apr 22, 2026
5 checks passed
@lmoresi
lmoresi deleted the bugfix/deform-mesh-stale-coords branch June 13, 2026 00:47
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.

3 participants