Skip to content

Recover non-finite values in parallel global evaluation - #602

Open
gthyagi wants to merge 1 commit into
underworldcode:developmentfrom
gthyagi:bugfix/parallel-global-evaluate-nonfinite-fallback
Open

Recover non-finite values in parallel global evaluation#602
gthyagi wants to merge 1 commit into
underworldcode:developmentfrom
gthyagi:bugfix/parallel-global-evaluate-nonfinite-fallback

Conversation

@gthyagi

@gthyagi gthyagi commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

Summary

  • include non-finite interpolation results in the existing parallel best-claim fallback
  • recover those points from the globally nearest rank even when the point-location mask reports them as located
  • add a focused regression test covering both masked and located-NaN values

Root cause

global_evaluate_nd() previously entered its parallel recovery path only for points marked extrapolated by return_mask. A failed interpolation can occasionally return NaN while retaining a located flag, so the value bypassed recovery. In Semi-Lagrangian Crank-Nicolson midpoint tracing, one such value makes the departure coordinate and transported history non-finite.

Implementation

The new _global_fallback_indices() helper selects the union of extrapolated points and points with any non-finite result component. These points use the existing globally nearest-rank RBF fallback. The collective structure and finite-value behavior are unchanged.

Validation

  • clean ./uw build
  • direct helper assertion: passed
  • mpirun -n 2 python -m pytest --with-mpi tests/parallel/test_0760_swarm_cache_migration.py -q: 4 passed per rank in 3.69 s

This focused change was extracted from the mantle-convection benchmark branch before that branch is reset to current upstream development.

Include located-but-nonfinite interpolation results in global_evaluate's parallel best-claim fallback. This prevents finite SLCN midpoint coordinates from receiving NaN velocities when a rank-local interpolation reports a false located status.\n\nAdd a focused regression for fallback index selection. Validate with the two-rank migration suite and an eight-rank Zhong A1 cellsize=1/16 step that previously diverged with DIVERGED_FNORM_NAN.
@gthyagi

gthyagi commented Aug 18, 2026

Copy link
Copy Markdown
Contributor Author

@lmoresi Could you please review the parallel global_evaluate fallback in this PR? This is particularly important for evaluations near physical boundaries and MPI partition seams.

We observed cases where interpolation returned a non-finite value even though the point-location mask reported the point as located. The existing fallback therefore did not run, and a single NaN could propagate through SLCN midpoint tracing into the departure coordinates and transported history.

This PR sends those non-finite located results through the existing globally nearest-rank RBF recovery path. Could you please check whether:

  • treating every non-finite located result as a fallback candidate is the correct boundary behavior;
  • the nearest-rank recovery preserves the intended FE evaluation semantics near curved boundaries and partition seams; and
  • the collective path remains safe for the SLCN use case?

The focused two-rank MPI regression file passes (4 passed per rank), but a review of the numerical semantics near boundaries would be valuable before merging.

@gthyagi
gthyagi marked this pull request as ready for review August 18, 2026 03:48
@gthyagi
gthyagi requested a review from lmoresi as a code owner August 18, 2026 03:48
@lmoresi

lmoresi commented Aug 18, 2026

Copy link
Copy Markdown
Member

Adversarial review

Reviewed at the PR head against development at 112c277. Four findings and
two things checked and cleared.

1. The recovery substitutes an RBF extrapolation for a failed interpolation,
and nothing says it happened.
For a point the mask reports as located, this
rank's nearest-centroid distance is effectively zero, so it wins the best-claim
(my_claim/win_rank, lines 618-625) and contributes its own value. That value
is not the one that produced the NaN — ext_vals comes from a fresh
evaluate_nd(..., rbf=True) — so the caller receives an RBF fit where they asked
for the interpolant. That is the right thing to do given a NaN, and it is a
different quantity, delivered silently: global_evaluate's default path returns
return_value alone, and only a caller passing check_extrapolated=True sees
return_mask flip. A count of recovered points, warned once per call, would make
the substitution visible without changing the contract.

2. The root cause is not diagnosed, and recovering from it removes the
symptom.
"A failed interpolation can occasionally return NaN while retaining a
located flag" describes a defect in location or interpolation, not a property of
the problem. After this lands, that defect produces a finite RBF value instead of
a NaN and there is nothing left to notice. Worth an issue naming the condition —
which expression, which mesh, whether the point sits on a facet or a rank
boundary — so the underlying fault is still findable.

3. The test pins the index selection, not the recovery. test_global_fallback_includes_nonfinite_located_values
calls _global_fallback_indices with hand-built arrays. It never enters
global_evaluate_nd, never constructs a located NaN, and would pass unchanged if
the recovery path were deleted. There is also no negative control: nothing here
demonstrates the failure the PR exists to fix, so a future refactor that
reintroduces it stays green. The PR's own validation ran an existing file, which
tells us this change breaks nothing — not that it does anything.

4. One stray NaN now costs the whole ladder, on every rank. n_ext_total > 0
short-circuits the fallback, and previously a run with nothing extrapolated
skipped it. Now a single non-finite component anywhere makes every rank
allgather the coordinate set, run a full local RBF evaluation of the global
extrapolation set, build a kd-tree index, query it, and execute four Allreduces.
Correct, and a large constant for one bad point in a semi-Lagrangian step that
runs every timestep. Worth knowing before it is used at scale.

Checked and cleared.

  • np.isfinite(return_value).all(axis=(1, 2)) hard-codes three dimensions. That
    is safe: an expression without .shape is wrapped as sympy.Matrix(((expr,),))
    at line 480, so expr_shape is always a 2-tuple and return_value always
    3-dim. Verified for a scalar .sym, a bare x+y, a vector and an outer
    product.
  • The collective structure is genuinely unchanged. Entry to the ladder is decided
    by n_ext_total, an allgathered sum, so all ranks agree whether to run it —
    which matters because this is the path we measured during adapt/relax follow-ups from the #488 re-review: empty-rank eval_metric skip (3 sites), reconnect invariant guard, vacuous partition test #512, where
    global_evaluate was found not to block on its peers for the shapes tried.
    Widening ext_idx does not change who enters.

Underworld development team with AI support from Claude Code

@lmoresi

lmoresi commented Aug 18, 2026

Copy link
Copy Markdown
Member

Finding 2 is now #604, so the condition survives this PR landing rather than being closed out with it.

It also records the clue the fix contains: the replacement value comes from evaluate_nd(expr, all_ext, rbf=True, ...), a different interpolation path, and for a located point this rank wins the best-claim on distance and supplies that value itself. The RBF route succeeding where the located route returned NaN says the point is not unreachable — one of the two ways of reaching it fails — which is the most specific thing anyone has on this so far.

The suggested first step there is the one that would also answer finding 1: have the recovery count what it repaired and report it once per call. That keeps this PR's behaviour exactly and turns a silent substitution into something observable.

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