Skip to content

fix(mesh): parallel plex-point→coord-row mapping in kd-tree/face navigation (#360)#363

Merged
lmoresi merged 2 commits into
developmentfrom
bugfix/moving-mesh-kdtree-np
Jul 15, 2026
Merged

fix(mesh): parallel plex-point→coord-row mapping in kd-tree/face navigation (#360)#363
lmoresi merged 2 commits into
developmentfrom
bugfix/moving-mesh-kdtree-np

Conversation

@lmoresi

@lmoresi lmoresi commented Jul 14, 2026

Copy link
Copy Markdown
Member

Root cause

Every moving-mesh / manifold workflow crashed at np>1 the first time navigation rebuilt the cell-search kd-tree (nuke_coords_and_rebuild → _build_kd_tree_index), with IndexError: index N is out of bounds (indices ran both negative and past the end, different per rank). The kd-tree build and the two mark-faces routines mapped vertex plex-points to coordinate rows with the affine shift nav_coords[points - pStart], valid only when vertex numbering is serial-contiguous. On the 1-cell-overlap navigation clone used for manifold (codim-1) meshes the local coordinate array is ordered by the coordinate PetscSection, not by plex_point − pStart, so ghost/halo vertices push the affine index out of bounds. Serial is unaffected (no overlap; the section offset equals the affine shift exactly).

Investigation showed the bug has two parts — the section-offset remap alone is necessary but not sufficient, because distributeOverlap also leaves the ghost coordinates unscattered (the local coordinate vector is short by the ghost rows, so even section offsets overrun it). Both are fixed here.

The fix

Two small private helpers on Mesh, applied method-locally:

  1. _coord_rows_for_points(nav_dm, points) — maps each vertex to its row via the coordinate section offset (getOffset(p) // cdim), the authoritative vertex→row map for the (possibly overlapped) local coordinate array. In serial this equals (plex_point − pStart) * cdim, so rows are bit-identical to the old affine indexing.
  2. _navigation_coords_from_dm(nav_dm) — rebuilds the full section-consistent local coordinate array (including ghost rows) through the coordinate DM's globalToLocal scatter, so every ghost vertex a local cell closure references actually carries coordinates. In serial (no overlap) this equals getCoordinatesLocal().

The four call-site groups (five physical sites)

  • _build_kd_tree_index — the crash-trace site (1)
  • _mark_faces_inside_and_out — cell closure + face closure (2)
  • _mark_local_boundary_faces_inside_and_out — boundary-face closure + the dim==2, cdim==3 cell-normal branch (2)

_nav_coords is now built with helper 2 at construction and, for the deform/adapt refresh in nuke_coords_and_rebuild, via setCoordinates(main-global) + helper 2 so the overlapped clone tracks the moved geometry (the old setCoordinatesLocal(main-local) was size-mismatched on the overlapped clone and left ghost rows unfilled).

Before / after evidence

  • Repro (before): extract_surface on an Annulus at np2/np4 crashed at _build_kd_tree_index with e.g. IndexError: index -20 is out of bounds for axis 0 with size 19 — matching the issue signature (both negative and past-end). Serial clean.
  • After: the new regression test and the previously-red test_0773_surface_smoother_mpi pass at np2 and np4; serial navigation is provably unchanged.

Gates (run serially, single amr-dev env)

Gate Result
Serial level_1 and tier_a 373 passed, 12 skipped, 2 xfailed, 1 xpassed, 0 failed (new test is parallel-only → skipped serially; identical pre/post)
Serial evaluate / RBF / point-location / kdtree families 93 passed, 1 xfailed (pre-existing 1-manifold point-location limitation)
Serial bit-identical probe helper rows == affine rows (max diff 0); evaluate coord error 0.00e+00 (box + annulus)
np2 navigation batch 25 passed, 1 skipped, 1 pre-existing failure (test_0765 _deform_mesh guard — #331/#326, documented TODO(BUG) in the test)
np4 navigation batch 25 passed, 2 pre-existing failures (same test_0765 guard; test_0700 test_nodal_swarm_advection_basic = stale SemiLagrangian.__init__ test API)
New test_0775 FAILS pre-fix (IndexError at _build_kd_tree_index, np2+np4); PASSES post-fix (np2+np4)
Dependencies / lockfile unchanged

The two pre-existing failures are unrelated to this change (one is the coordinate-mutation guard rejecting a direct _deform_mesh call — #331; the other a test using an outdated SemiLagrangian signature).

Coordination

Wave D mesh PR #344 (open) refactors this same file (discretisation_mesh.py). This diff is small and method-local (five one-line call swaps + two new helpers + two _nav_coords assignment sites); #344 will need a trivial rebase over these sites (or vice versa) — please sequence them at merge.

Closes #360

Underworld development team with AI support from Claude Code

…igation (#360)

Every moving-mesh / manifold workflow crashed at np>1 in
_build_kd_tree_index with `IndexError: index N is out of bounds` (both
negative and past-end). The kd-tree and the two mark-faces routines mapped
vertex plex-points to coordinate rows with the affine shift
`nav_coords[points - pStart]`, valid only when vertex numbering is
serial-contiguous. On the 1-cell-overlap navigation clone used for manifold
(codim-1) meshes the local coordinate array is laid out by the coordinate
PetscSection, so the affine shift runs out of bounds across ghost vertices.

Root cause has two parts, both fixed here:

1. Vertex->row mapping. New `_coord_rows_for_points(nav_dm, points)` maps
   each vertex via the coordinate PetscSection offset (`getOffset(p)//cdim`),
   the authoritative map for the overlapped array. In serial the section
   offset equals `(plex_point - pStart)*cdim`, so the rows are bit-identical
   to the old affine indexing (verified: max row diff 0, evaluate error 0)
   and serial navigation is unchanged. Applied at all five sites in
   _build_kd_tree_index, _mark_faces_inside_and_out (x2) and
   _mark_local_boundary_faces_inside_and_out (x2).

2. Incomplete ghost coordinates. distributeOverlap grows the nav clone's
   coordinate section to cover ghost vertices but never scatters their
   coordinates into the local vector, so getCoordinatesLocal() is short by
   the ghost rows and even the section offsets overrun it. New
   `_navigation_coords_from_dm(nav_dm)` rebuilds the full local coordinate
   vector via the coordinate DM global->local scatter. Used at construction
   and, for the deform/adapt refresh, together with setCoordinates(main
   global) so the overlapped clone tracks the moved geometry.

Regression test tests/parallel/test_0775_deform_kdtree_parallel.py: a box
deform navigation check plus a manifold surface navigation check that fails
pre-fix (IndexError at _build_kd_tree_index, np2 and np4) and passes after.
The pre-existing manifold test test_0773 (which failed pre-fix at np2/np4)
also goes green.

Closes #360

Underworld development team with AI support from Claude Code
Copilot AI review requested due to automatic review settings July 14, 2026 23:06

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

Fixes parallel-only crashes in mesh navigation (kd-tree build + face control points) after mesh deformation and on manifold (codim-1) meshes by making vertex plex-point → coordinate-row mapping section-consistent and ensuring ghost/overlap coordinate rows are properly populated on the navigation clone DM. This targets issue #360 in discretisation_mesh.py, which is central to point-location/evaluate and moving-mesh workflows.

Changes:

  • Add Mesh._coord_rows_for_points() to map plex points to coordinate rows via the coordinate PetscSection offset (parallel-safe on overlapped navigation DMs).
  • Add Mesh._navigation_coords_from_dm() to rebuild a full local coordinate array (including ghost rows) via the coordinate DM global→local scatter, and use it when creating/refreshing _nav_coords.
  • Add MPI regression tests covering (a) deform + rebuilt kd-tree navigation and (b) manifold surface extraction/navigation correctness including ghost rows.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
src/underworld3/discretisation/discretisation_mesh.py Replaces affine points - pStart coord indexing with section-offset row mapping; rebuilds navigation coords with a ghost-complete local coordinate scatter for overlapped nav DMs.
tests/parallel/test_0775_deform_kdtree_parallel.py Adds parallel regression coverage for deform-triggered kd-tree rebuilds and manifold surface navigation/ghost-coordinate correctness.

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

Comment thread src/underworld3/discretisation/discretisation_mesh.py
… other DM scatters

INSERT and INSERT_VALUES are aliases in petsc4py (both enum value 1);
this matches the spelling used elsewhere (Copilot review).

Underworld development team with AI support from Claude Code
lmoresi added a commit that referenced this pull request Jul 14, 2026
…360 fix through the D-29/D-34 refactor

Conflict resolution: the Wave D extraction moved the coordinate-array
installation into _install_coordinate_array and the facet-normal dispatch
into _facet_outward_unit_normal; the #360 section-offset row mapping and
ghost-coordinate completion are applied inside the extracted methods.

Underworld development team with AI support from Claude Code
@lmoresi lmoresi merged commit c65cfa0 into development Jul 15, 2026
1 check passed
lmoresi added a commit that referenced this pull request Jul 15, 2026
…ady embeds the #360 fix ported through D-29/D-34

Underworld development team with AI support from Claude Code
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