Promote monotone interpolation to a universal evaluator flag#208
Conversation
Add an opt-in `monotone` keyword (default False) to uw.function.evaluate
and global_evaluate. When truthy ("clamp"/"pick", with True aliasing
"clamp"), the interpolated result is bounded to the local data range of
the source field: for each query point, the mesh.dim+1 nearest source
DOFs define [min, max]; "clamp" clips into that range, "pick" keeps
in-bounds values and re-evaluates only the out-of-bounds subset via
bounded RBF.
This lifts the kNN clamp/pick limiter that previously lived only in the
semi-Lagrangian trace-back (SemiLagrangian.update_pre_solve) into a
shared evaluator helper (_apply_monotone_limit), so any resampling path
(remesh re-interpolation, projection, restart, the SL DDt itself) can
request the same bounded result from one place. The source field is
recovered from the expression via meshVariable_lookup_by_symbol.
Implementation keeps monotone=False provably bit-identical: the existing
evaluate/global_evaluate bodies are renamed to _evaluate_impl /
_global_evaluate_impl unchanged, and the public functions are thin
wrappers that return the impl result untouched unless monotone is truthy.
ddt.py's inline limiter block is deleted and routed through the new
option (monotone=monotone_mode); a full AdvDiffusionSLCN trajectory is
bit-identical pre/post refactor for None, "clamp" and "pick" (max|Δ|=0).
Scope (MVP): single-MeshVariable expressions only; composites raise
ValueError (the neighbour bound across fields is ill-defined). The
neighbour bound is rank-local and "pick"'s collective re-eval is
serial-only -- both flagged as TODO(parallel) with the nav-clone overlap
machinery noted as the hardening hook. A pre-existing dimensional-vs-ND
bound mismatch in units-active runs is flagged TODO(units), not fixed
(scaling is inactive in the validated baseline).
Tests: new tests/test_0760_evaluate_monotone.py locks the API (default
no-op bit-identical, clamp/pick bounding, composite/unknown-option
refusal); tests/test_1054 gains end-to-end solver-integration coverage.
Underworld development team with AI support from Claude Code
There was a problem hiding this comment.
Pull request overview
This PR promotes the semi-Lagrangian “monotone” limiter (clamp/pick) into a shared, opt-in post-process for uw.function.evaluate() and uw.function.global_evaluate(), then routes the DDt trace-back through that unified evaluator path so other resampling workflows can reuse the same bounded-interpolation behavior.
Changes:
- Adds
monotonekwarg support toevaluate()/global_evaluate()and implements the shared limiter helper (_apply_monotone_limit). - Refactors
SemiLagrangian.update_pre_solve()to useglobal_evaluate(..., monotone=monotone_mode)instead of an inline limiter block. - Adds contract tests for evaluator-level monotone behavior and expands solver integration coverage.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
src/underworld3/function/functions_unit_system.py |
Refactors evaluator implementations and adds the new monotone limiter wrapper/helper. |
src/underworld3/systems/ddt.py |
Switches semi-Lagrangian trace-back limiting to the evaluator-level monotone option. |
tests/test_0760_evaluate_monotone.py |
Adds contract tests for monotone default no-op, clamp/pick behavior, and refusal cases. |
tests/test_1054_advdiff_monotone_mode_kwarg.py |
Adds end-to-end integration tests exercising the refactored limiter path via solver solves. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| out_of_bounds = ((veep_flat < nbr_min) | (veep_flat > nbr_max)) | ||
| oob_mask = out_of_bounds.any( | ||
| axis=tuple(range(1, out_of_bounds.ndim))) | ||
| veep_lim = veep_flat.copy() | ||
| # TODO(parallel): this re-evaluation is gated on a rank-local | ||
| # `oob_mask.any()`, so in parallel one rank may enter the | ||
| # (collective) global_evaluate while another skips it -> deadlock. | ||
| # Lifted verbatim from the validated (serial) SL limiter; pick is | ||
| # serial-only until the rank-local bound is hardened (see the | ||
| # TODO(parallel) above and project_parallel_point_eval_decision). |
| oob_coords = coords[oob_mask] | ||
| value_rbf_oob = global_evaluate(expr, oob_coords, evalf=True) | ||
| vrbf_flat = np.asarray(value_rbf_oob).reshape( | ||
| (-1,) + veep_flat.shape[1:]) |
| # serial-only until the rank-local bound is hardened (see the | ||
| # TODO(parallel) above and project_parallel_point_eval_decision). | ||
| if oob_mask.any(): | ||
| oob_coords = coords[oob_mask] |
| result = _global_evaluate_impl( | ||
| expr, | ||
| coords=coords, | ||
| coord_sys=coord_sys, | ||
| other_arguments=other_arguments, | ||
| simplify=simplify, | ||
| verbose=verbose, | ||
| evalf=evalf, | ||
| mode=mode, | ||
| data_layout=data_layout, | ||
| check_extrapolated=check_extrapolated, | ||
| smoothing=smoothing, | ||
| rbf=rbf, | ||
| force_l2=force_l2, | ||
| ) | ||
|
|
||
| monotone_mode = _normalize_monotone(monotone) | ||
| if monotone_mode is None: | ||
| return result | ||
|
|
||
| if check_extrapolated: | ||
| raise NotImplementedError( | ||
| "monotone interpolation is not supported together with " | ||
| "check_extrapolated in global_evaluate." | ||
| ) |
| result = _evaluate_impl( | ||
| expr, | ||
| coords, | ||
| coord_sys=coord_sys, | ||
| other_arguments=other_arguments, | ||
| simplify=simplify, | ||
| verbose=verbose, | ||
| evalf=evalf, | ||
| mode=mode, | ||
| data_layout=data_layout, | ||
| check_extrapolated=check_extrapolated, | ||
| smoothing=smoothing, | ||
| rbf=rbf, | ||
| force_l2=force_l2, | ||
| ) | ||
|
|
||
| monotone_mode = _normalize_monotone(monotone) | ||
| if monotone_mode is None: | ||
| return result |
…s propagation Responding to the Copilot review on PR #208: - Validate `monotone` up front in both `evaluate` and `global_evaluate` (before the impl call), so an unknown option — or the unsupported `monotone` + `check_extrapolated` combination in `global_evaluate` — fails fast without doing a full, wasted evaluation. - Guard `monotone="pick"` against MPI: its out-of-bounds re-evaluation is a collective `global_evaluate` gated on a rank-local mask, which would deadlock under MPI. Raise a clear NotImplementedError pointing to `monotone="clamp"` (which is rank-local and parallel-safe) instead of hanging. The `TODO(parallel)` for a true global-neighbour bound stays. - Propagate `coord_sys` / `other_arguments` into the `pick` re-evaluation so it stays in the caller's frame. `evalf=True` remains a deliberate override (the FE result is exactly what overshot, so the re-eval uses the bounded RBF path and does not honour `mode`/`rbf`/`force_l2`) — documented in the helper. Not changed (with rationale): - Rank-local neighbour bound: already flagged `TODO(parallel)`; the true global-neighbour bound is deferred per the PR plan. - List/tuple coords in `pick`: the evaluator rejects non-array coords upstream (in `_evaluate_impl`, via `non_dimensionalise`) before monotone runs, so the masking only ever sees ndarray / UnitAwareArray — no coercion needed. All defaults remain bit-identical (full AdvDiffusionSLCN trajectory max|Δ|=0 across None/clamp/pick). tests/test_0760 + test_1054: 14 passed; level_1 and tier_a: 158 passed. Underworld development team with AI support from Claude Code
|
Thanks @copilot — addressed in e171fb2. Point by point:
Defaults stay bit-identical (full |
Summary
Adds an opt-in
monotonekeyword (defaultFalse) touw.function.evaluateandglobal_evaluate. When truthy ("clamp"/"pick", withTruealiasing"clamp"), the interpolated result is bounded to the local data range of the source field: for each query point themesh.dim + 1nearest source DOFs define[min, max];"clamp"clips into that range,"pick"keeps in-bounds values and re-evaluates only the out-of-bounds subset via bounded RBF.This lifts the kNN clamp/pick limiter that previously lived only inside the semi-Lagrangian trace-back (
SemiLagrangian.update_pre_solve) into a shared evaluator helper (_apply_monotone_limit), so any resampling path (remesh re-interpolation, projection, restart, the SL DDt itself) can request the same bounded result from one place. The source field is recovered from the expression viameshVariable_lookup_by_symbol.This is the standalone axis C of the free-surface mesh-motion work — independent of the ALE/mesh-motion axes (A/B), and lands on its own.
Bit-identical guarantee
monotone=Falseis provably bit-identical to today: the existingevaluate/global_evaluatebodies are renamed unchanged to_evaluate_impl/_global_evaluate_impl, and the public functions are thin wrappers that return the impl result untouched unlessmonotoneis truthy.ddt.py's inline limiter block is deleted and routed through the new option (monotone=monotone_mode).A full
AdvDiffusionSLCNtrajectory (steep blob, rotational advection, P3) is bit-identical pre/post refactor —max|Δ| = 0for all three modes (None,"clamp","pick"), not just the default.Scope & flagged follow-ups (MVP)
ValueError(the neighbour bound across fields is ill-defined).TODO(parallel): the neighbour bound is rank-local, and"pick"'s collective re-eval is gated on a rank-local mask → serial-only. Lifted verbatim from the validated (serial) limiter; the nav-clone overlap machinery is noted as the hardening hook.TODO(units): bounds come fromvar.data(always non-dimensional) vs the dimensional value — a pre-existing latent mismatch, harmless while scaling is inactive (as in the validated baseline). Flagged, not fixed.Tests
tests/test_0760_evaluate_monotone.py— locks the API: default no-op bit-identical,clamp/pickbounding against independently recomputed neighbour ranges, composite / unknown-option refusal.tests/test_1054_advdiff_monotone_mode_kwarg.py— gains end-to-end solver-integration coverage.pytest -m "level_1 and tier_a": 158 passed, 5 skipped (MPI), 0 failed.Underworld development team with AI support from Claude Code