From 50f3838b06b916b323d24d8ee7e525c7c2224f14 Mon Sep 17 00:00:00 2001 From: lmoresi Date: Thu, 27 Aug 2026 14:47:24 +1000 Subject: [PATCH] Forward the location policy on the empty-rank path (#611) `global_evaluate` deadlocked whenever a query set left some rank with no points. At np=4 the migration test hung indefinitely -- 900 s with no progress on a 300-point query -- while passing at np=2. Native stacks during the hang name both halves: 3 ranks DMSwarmMigrate -> DMSwarmDataExCreate -> MPI_Comm_dup 1 rank DMLocatePoints -> DMGetBoundingBox -> MPI_Allreduce A mismatched collective, not slowness and not a location failure: every rank was inside MPI, none in locator code. The cell-location policy (`mesh._hint_is_authoritative`) decides whether the barycentric hint may bypass PETSc's `DMLocatePoints`, which is COLLECTIVE on the mesh DM communicator. The policy is a mesh capability, so all ranks agree on it -- instrumenting confirmed `auth=True` on all four. It was then discarded in transit. `create_structure` had no hint array to pass when a rank held zero points, and passed `hintAuthoritative = 0` HARDCODED beside the NULL: ierr = DMInterpolationSetUp_UW(self._ipInfo, dm, 0, 1, NULL, 0) so that rank alone took the DMLocatePoints branch. Forwarding the caller's policy is the fix; `petsc_tools.c` additionally has to accept a NULL hint when there are no points to hint at, since the bypass is trivially correct with nothing to locate. Why it hid at np=2: the test biases every point into x > 0.5, and at np=4 one rank owns solely x < 0.5 and so receives nothing, where at np=2 the coarser partition leaves both ranks straddling the split. Measured ownership at np=4: rank 1 gets 0 of the 300 points, and rank 1 is the rank the watchdog roll call singled out. Two wrong turns are worth recording so they are not retried. A pre-touch of `mesh.dm.getBoundingBox()` does nothing -- PETSc does not cache it, so the later call inside the locator reduces again. And forcing the DMInterpolation cache decision to be unanimous does nothing here either: instrumenting showed all four ranks MISS the cache, so it was never the divergence. Both were reverted rather than shipped. The docstring on `_location_capability` states the assumption that made this invisible: "deliberately NOT reduced across ranks: the evaluator runs on COMM_SELF [...] petsc_interpolate is reached only by ranks holding points". Both halves are false -- `DMLocatePoints(dm, ...)` uses the mesh DM's communicator, and a trace shows all ranks entering `petsc_interpolate`, including one with zero points. The policy does not need reducing (it already agrees); it needs to survive the call. Regression covers a query biased so some rank is empty, plus an unbiased control that passed before the fix -- so a failure there means the ordinary path broke rather than the empty-rank path being repaired. Green at np=1, 2, 4, as is the whole of test_0760 which previously hung at np=4. Underworld development team with AI support from Claude Code --- .../function/_dminterp_wrapper.pyx | 13 +++- src/underworld3/function/petsc_tools.c | 12 +++- .../test_1076_global_evaluate_empty_rank.py | 62 +++++++++++++++++++ 3 files changed, 85 insertions(+), 2 deletions(-) create mode 100644 tests/parallel/test_1076_global_evaluate_empty_rank.py diff --git a/src/underworld3/function/_dminterp_wrapper.pyx b/src/underworld3/function/_dminterp_wrapper.pyx index 6c10c7ae..966dc0e0 100644 --- a/src/underworld3/function/_dminterp_wrapper.pyx +++ b/src/underworld3/function/_dminterp_wrapper.pyx @@ -178,7 +178,18 @@ cdef class CachedDMInterpolationInfo: &cells_view[0], 1 if hint_authoritative else 0) else: - ierr = DMInterpolationSetUp_UW(self._ipInfo, dm, 0, 1, NULL, 0) + # No hint array to pass (no points, or no cells) — but the POLICY + # still has to be forwarded. Hardcoding 0 here made a rank with zero + # local points disagree with its peers about whether the hint is + # authoritative, and petsc_tools.c takes the DMLocatePoints branch + # when it is not. DMLocatePoints is COLLECTIVE on the mesh DM, so + # that rank blocked inside DMGetBoundingBox -> MPI_Allreduce while + # the others bypassed and ran on to DMSwarmMigrate -> MPI_Comm_dup: + # a deadlock whenever a query set leaves some rank empty (#611). + # The policy is a mesh capability and already agrees across ranks; + # it just has to survive the trip. + ierr = DMInterpolationSetUp_UW(self._ipInfo, dm, 0, 1, NULL, + 1 if hint_authoritative else 0) if ierr != 0: DMInterpolationDestroy(&self._ipInfo) raise RuntimeError(f"DMInterpolationSetUp_UW failed with error {ierr}") diff --git a/src/underworld3/function/petsc_tools.c b/src/underworld3/function/petsc_tools.c index 96936d38..7bec52e7 100644 --- a/src/underworld3/function/petsc_tools.c +++ b/src/underworld3/function/petsc_tools.c @@ -69,7 +69,17 @@ PetscErrorCode DMInterpolationSetUp_UW(DMInterpolationInfo ctx, DM dm, PetscBool PetscCall(PetscMalloc2(N, &foundProcs, N, &globalProcs)); for (p = 0; p < N; ++p) foundProcs[p] = size; cellSF = NULL; - if (owning_cell && hintAuthoritative) { + /* N == 0 is included deliberately. An empty hint array reaches C as a NULL + `owning_cell`, which flipped this test and sent a rank with no local points + down the DMLocatePoints branch ALONE -- and DMLocatePoints is collective on + the mesh DM communicator (the comment below is right that the + Allreduce(foundProcs) is a COMM_SELF no-op, but DMLocatePoints itself is + not). Three ranks bypassed while the empty one blocked inside + DMGetBoundingBox -> MPI_Allreduce, deadlocking the job (#611). With no + points there is nothing to locate, so bypassing is trivially correct and + the branch now depends only on `hintAuthoritative`, which is a mesh + capability and agrees across ranks. */ + if ((owning_cell || N == 0) && hintAuthoritative) { /* Bypass DMLocatePoints when the caller supplies an AUTHORITATIVE hint (ported from feature/dminterp-bypass-element-check, 17a5a8d). diff --git a/tests/parallel/test_1076_global_evaluate_empty_rank.py b/tests/parallel/test_1076_global_evaluate_empty_rank.py new file mode 100644 index 00000000..e8ec8a41 --- /dev/null +++ b/tests/parallel/test_1076_global_evaluate_empty_rank.py @@ -0,0 +1,62 @@ +#!/usr/bin/env python3 +"""global_evaluate must not deadlock when a rank receives no query points (#611). + +The cell-location policy (`mesh._hint_is_authoritative`) decides whether the +barycentric hint may bypass PETSc's `DMLocatePoints`. `DMLocatePoints` is +COLLECTIVE on the mesh DM communicator, so every rank has to reach the same +verdict — and the policy is a mesh capability, so they do. + +It was then thrown away in transit. `CachedDMInterpolationInfo.create_structure` +had no hint array to pass when a rank held zero points, and passed +`hintAuthoritative = 0` hardcoded along with the NULL. That rank alone took the +`DMLocatePoints` branch and blocked inside `DMGetBoundingBox -> MPI_Allreduce`, +while its peers bypassed and ran on to `DMSwarmMigrate -> MPI_Comm_dup`. + +Measured before the fix, at np=4: three ranks in `MPI_Comm_dup`, one in +`MPI_Allreduce`, no progress in 900 s on a 300-point query. + +The trigger is a query set that leaves some rank empty. Biasing every point into +x > 0.5 does it at np=4 — one rank owns only x < 0.5 — but not at np=2, where +the coarser partition leaves both ranks straddling the split. That is exactly +why this hid at np=2 and appeared at np=4. +""" +import numpy as np +import pytest + +import underworld3 as uw + +pytestmark = [pytest.mark.level_1, pytest.mark.tier_a] + + +def _mesh_and_field(tag): + mesh = uw.meshing.UnstructuredSimplexBox( + minCoords=(0.0, 0.0), maxCoords=(1.0, 1.0)) + field = uw.discretisation.MeshVariable(tag, mesh, mesh.dim, degree=2) + return mesh, field + + +def test_global_evaluate_with_points_only_on_some_ranks(): + """The regression. Every point in x > 0.5, so some rank owns none of them.""" + _mesh, field = _mesh_and_field("u1076a") + rng = np.random.default_rng(42) + coords = rng.random((300, 2)) + coords[:, 0] = 0.5 + 0.5 * coords[:, 0] + + result = uw.function.global_evaluate(field.sym, coords) + assert result.shape[0] == 300, ( + f"rank {uw.mpi.rank}: expected 300 results, got {result.shape[0]}") + + +def test_global_evaluate_with_points_everywhere_still_works(): + """Negative control. + + The unbiased query already passed before the fix, so if this ever fails the + change has broken the ordinary path rather than repaired the empty-rank one. + """ + _mesh, field = _mesh_and_field("u1076b") + rng = np.random.default_rng(42) + coords = rng.random((300, 2)) + + result = uw.function.global_evaluate(field.sym, coords) + assert result.shape[0] == 300, ( + f"rank {uw.mpi.rank}: expected 300 results, got {result.shape[0]}")