Fix global_evaluate parallel hang, shape mismatch, and OOB classification - #116
Conversation
Three related issues fixed: 1. Parallel hang in evaluate_nd: petsc_interpolate had early returns for empty coords that skipped DMLocatePoints — a collective PETSc operation. When some ranks had zero interior points (all OOB), they returned early while others blocked in the collective. Fixed by removing the early returns and ensuring all ranks participate in DMLocatePoints, even with zero points. 2. Shape mismatch in global_evaluate_nd: return arrays were sized to the post-migration particle count rather than the original input count. Pre-allocate with NaN so the shape is always correct regardless of whether points survive the migration round-trip. 3. points_in_domain misclassifying extreme OOB points: the boundary control-point sign heuristic has no distance ceiling, so points at (1e12, 1e12) could be classified as "interior". Added a domain radius check to reject points far beyond the domain extent. Also set ignoreOutsideDomain=True in DMInterpolationSetUp_UW so PETSc silently skips unlocatable points rather than crashing. Underworld development team with AI support from Claude Code
There was a problem hiding this comment.
Pull request overview
Fixes global_evaluate/evaluate_nd parallel robustness issues by ensuring collective PETSc calls are not skipped on ranks with zero “interior” points, returns are always sized to the original input coordinate count, and extreme out-of-bounds points are not misclassified as interior.
Changes:
- Pre-allocate
global_evaluate_nd()return arrays to the original input size (fill withNaN/ extrapolated mask defaults) to avoid shape mismatches after swarm migration. - Remove
petsc_interpolate()early-returns for empty coordinate arrays to prevent MPI deadlocks around collective PETSc operations. - Add an extreme-OOB distance ceiling to
Mesh.points_in_domain()and enableignoreOutsideDomain=TrueduringDMInterpolationSetUp_UW.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/underworld3/function/_function.pyx |
Fixes global_evaluate_nd output sizing and removes empty-coords early returns in PETSc interpolation path; adds an unconditional update_lvec() call. |
src/underworld3/function/_dminterp_wrapper.pyx |
Ensures DMInterpolationSetUp_UW runs collectively even with zero points and enables ignoreOutsideDomain to avoid crashes on unlocatable points. |
src/underworld3/discretisation/discretisation_mesh.py |
Adds a domain bounding-radius check to prevent extreme-OOB points being classified as interior by the boundary-control-point heuristic. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # CRITICAL: update_lvec() calls dm.globalToLocal() which is COLLECTIVE. | ||
| # It MUST be called by ALL ranks before any rank enters petsc_interpolate, | ||
| # because ranks with zero interior points would skip petsc_interpolate | ||
| # (and its internal update_lvec call), deadlocking the ranks that do enter. | ||
| mesh.update_lvec() | ||
|
|
There was a problem hiding this comment.
mesh.update_lvec() is now called unconditionally in evaluate_nd() before petsc_interpolate(), but petsc_interpolate() / interpolate_vars_on_mesh() already calls mesh.update_lvec() on both cache-hit and cache-miss paths (and empty coords no longer early-returns). This adds an extra collective globalToLocal() per evaluation and can be a noticeable performance hit in time-stepping loops. Consider removing this call here (or adding a mechanism to skip the internal update_lvec() when the caller has already performed it), relying on the now-parallel-safe petsc_interpolate() for the required collective participation.
| # CRITICAL: update_lvec() calls dm.globalToLocal() which is COLLECTIVE. | |
| # It MUST be called by ALL ranks before any rank enters petsc_interpolate, | |
| # because ranks with zero interior points would skip petsc_interpolate | |
| # (and its internal update_lvec call), deadlocking the ranks that do enter. | |
| mesh.update_lvec() | |
| # petsc_interpolate() is responsible for any required update_lvec() | |
| # / collective participation, including empty-coordinate cases. |
- RegionalGeographicBox now has lateral + radial coordinate clamping via cartesian_to_geographic / geographic_to_cartesian round-trip. Upstream points that escape the domain are clamped to the mesh's lon/lat/depth ranges before evaluation. - Guard empty-particle assignment in global_evaluate_nd: when a rank has zero particles after migration, skip the swarm variable assignment that fails on reshape of empty arrays. Underworld development team with AI support from Claude Code
- Revert mode="fast" on DDt global_evaluate calls — this was a workaround for DMInterpolation performance that sacrifices accuracy. The proper fix is in PR #116 (global_evaluate parallel improvements). - Add W5-Interactive.py: pyvista trame visualiser with field selector, depth slider, and fault/velocity toggle widgets. - Minor fixes to W5b-Compare.py and _run_w2_w3_w4b.py. Underworld development team with AI support from Claude Code
Summary
Fixes #113 —
global_evaluateshape mismatch and parallel hang in DDt semi-Lagrangian.Three related issues addressed:
Parallel hang in
evaluate_nd:petsc_interpolatehad early returns for empty coords that skippedDMLocatePoints— a collective PETSc operation on the mesh DM communicator. When some ranks had zero interior points (all out-of-domain), they returned early while others blocked in the collective. Fixed by removing the early returns and ensuring all ranks participate.Shape mismatch in
global_evaluate_nd: Return arrays were sized to the post-migration particle count rather than the original input count. Now pre-allocated with NaN so the shape is always correct — any lost points remain NaN rather than causing a shape error or returning uninitialised data.points_in_domainmisclassifying extreme OOB points: The boundary control-point sign heuristic had no distance ceiling, so points very far from the domain (e.g.1e12) could be classified as "interior". Added a domain bounding radius check. Also setignoreOutsideDomain=TrueinDMInterpolationSetUp_UWso PETSc silently skips unlocatable points rather than crashing.Limitations
This fix prevents crashes and ensures correct array shapes, but does not fully resolve AdvDiffusion accuracy on geographic meshes. The
RegionalGeographicBoxmesh still lacks lateralreturn_coords_to_bounds(only radial clamping is implemented), so DDt upstream points that advect sideways out of the regional box will be RBF-extrapolated rather than clamped. The extrapolated values are flagged in the mask but may not produce good advection results — lateral boundary clamping for geographic meshes is a separate issue.Test plan
global_evaluatewith in-domain and OOB points returns correct shape and mask1e12): no crash, correctly classified as exteriorUnderworld development team with AI support from Claude Code