Share the nnn=1 guard between both interpolation entry points; delete the dead RBF surface

[lmoresi]

Fixes #443, #428.

#443 — the two paths drifted exactly where I said they could not

interpolation_matrix(nnn=1, order=1) silently returned a nearest-neighbour operator where the value path raised:

kdt.rbf_interpolator_local(tgt, f, 1, 2, False, order=1)   # ValueError
kdt.interpolation_matrix(tgt, nnn=1, order=1)              # no error, nnz/row = 1

The caller asked for a linear-exact operator and got a constants-only one, with nothing recording the downgrade.

Cause: _local_stencil early-returns for nnn == 1 before any order handling, and the guard sat in rbf_interpolator_local_from_kdtree alone. Sharing a core is not the same as sharing its guards. #430's body claimed the two paths "cannot drift apart"; this is how they did.

The guard moves into _local_stencil, so both entry points inherit it. Verified on both, with nnn=1, order=0 still legal on both.

Two smaller things at the same site:

• The stencil-too-large error read Error in rbf_interpolator_local_from_kdtree - a nearest neighbour wasn't found, naming a function the caller never invoked. It now reports the requested stencil size and the tree size: Cannot build a 20-point stencil: the kd-tree holds 5 point(s)...
• The degenerate-fallback warning used stacklevel=3, tuned for the value path's depth. The two entry points sit at different depths below _local_stencil, so it is now 2 — the deepest frame correct for both.

#428 — dead RBF surface removed

MeshVariable.rbf_interpolate(meth=, rubbish=). Neither was ever read. meth was documented as "reserved, currently unused", which Charter §5 rules out.

Removing meth also fixes a latent test bug: tests/test_0505_rbf_swarm_mesh.py calls var.rbf_interpolate(rx, nnn, p), so its nnn landed in meth and was discarded — every parametrised case actually ran at the default stencil, including the (2, 1, 2) case that believed it was testing the raw nearest-neighbour branch. With meth gone the positional argument means what the test always intended, and the file passes unchanged.

Checked: no other caller passes a second positional argument.

_rbf_reduce_to_meshVar, Swarm._get_map, Swarm._nnmapdict, _nn_proxy. Zero callers. Deleting the trio is the disposition already recorded for SWARM-23 in design/SWARM_MODERNIZATION_DESIGN_2026-07.md, which notes _get_map caches particle indices against a stale key and is a live trap if anything revives it. _nn_proxy existed only to gate a branch inside the dead method and was plumbed out to Swarm.add_variable.

Regression

• level_1 tier_a/tier_b: 551 passed, 0 failed
• level_2 tier_a/tier_b: 319 passed, 0 failed
• np=2 parallel proxy/index-swarm: 4 passed
• Deprecated-pattern scan: clean

105 lines deleted, 65 added.

Underworld development team with AI support from Claude Code

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[lmoresi]

Adversarial review

The deletions are broader than "dead code" implies

_nn_proxy was a public-facing keyword on Swarm.add_variable, underscore-prefixed but reachable. Anything passing _nn_proxy= now gets a TypeError rather than a deprecation warning. Justified — it gated a branch inside a method with zero callers, so it could not have done anything — but it is an API removal without a shim, and the PR title says "dead surface", which undersells it. Nothing in the repo or docs/examples passes it; user scripts outside the repo are not checked.

Same for meth/rubbish on MeshVariable.rbf_interpolate. Removing meth changes what the second positional argument means: a caller writing rbf_interpolate(coords, 4) previously set the dead meth and got the default nnn; now it sets nnn=4. That is the fix, but for an out-of-repo caller it is a silent behaviour change, not an error. The only in-repo instance was the test, which is what the change repairs.

The #443 fix is narrower than the class of bug

Moving one guard into _local_stencil fixes the instance. It does not establish that the two entry points agree everywhere else — I checked nnn=1, nnn > n, empty targets and order validation, but there is no systematic property test asserting the paths are equivalent across the parameter space. test_interpolation_matrix_agrees_with_the_value_path parametrises order and dim; it still does not sweep nnn, p, or monotone. A caller could find the next divergence the same way this one was found.

Worth stating plainly: interpolation_matrix ignores monotone entirely — correctly, since limiting is data-dependent and an operator has no data — but the signature simply omits it, so interpolation_matrix(..., monotone=True) is a TypeError while a reader might expect a no-op. Not changed here; noted.

What the tests do and do not cover

The new guard test asserts both entry points raise, and that order=0 stays legal on both. It does not assert that the operator and the value path produce the same result at nnn=1, order=0 — the one combination where the shapes differ most (data[closest_n.reshape(-1)] versus a CSR with one non-zero per row). They should agree; it is untested.

The error-message test pins a string ("20-point stencil.*holds 5 point"). String-matching tests rot when messages are reworded. It is checking the thing that actually regressed, but a future editor will have to update it.

What I did not do

• No parallel run of the deleted paths. _get_map/_rbf_reduce_to_meshVar were dead, so there is nothing to exercise, but _nn_proxy removal touches SwarmVariable.__init__ and Swarm.add_variable signatures, which every swarm test constructs. Covered by np=2 only, not np=4.
• No check that xxhash is still needed. _get_map was the only user of the import xxhash inside it; the import was local to the deleted function, so it went with it, but I did not check whether xxhash remains a required dependency elsewhere or is now removable from the environment spec.

What holds up

• The defect was found by a retrospective adversarial review of already-merged code, reproduced, filed with a reproducer, and fixed at the layer that makes it structurally impossible rather than at the call site.
• Deleting the _get_map trio matches a disposition already recorded and argued in SWARM_MODERNIZATION_DESIGN_2026-07.md (SWARM-23) rather than being an opportunistic cleanup.
• 105 lines removed against 65 added, with level_1 551 / level_2 319 / np=2 4 passing and the style gate clean.

[lmoresi]

Checked the one open question from the review: xxhash is still used by cython/petsc_generic_snea_snes_solvers.pyx (three sites), so deleting _get_map does not make it a removable dependency.

[lmoresi]

Correction to the comment above: the file is src/underworld3/cython/petsc_generic_snes_solvers.pyx (I mistyped it). Three sites, so xxhash stays a required dependency.