Fix swarm proxy refresh under an active units model (#426)

[lmoresi]

Fixes #426.

The defect

The swarm kd-tree is built from Swarm._particle_coordinates.data, which is always non-dimensional. The proxy refresh queried it with MeshVariable.coords, which dimensionalises as soon as the model has reference quantities set:

ValueError: KD-tree was built with dimensionless coordinates, but query
coordinates have units 'meter'. Convert to dimensionless first.

So proxied swarm variables did not work at all under an active units model — including IndexSwarmVariable material level sets, which carry their own copy of the same pattern.

Reproduced before the fix:

m.set_reference_quantities(length=uw.quantity(1000.0, "km"),
                           viscosity=uw.quantity(1e21, "Pa*s"))
var = swarm.add_variable(name="rho", size=1, proxy_degree=1, units="kg/m^3")
var.data[:, 0] = 3300.0
var._update_proxy_if_stale()      # -> ValueError

The fix

.coords → .coords_nd at the three live sites:

• SwarmVariable._rbf_to_meshVar
• IndexSwarmVariable._update_proxy_variables, both update_type branches

This is the pattern the rest of the codebase already uses — MeshVariable._get_kdtree builds from coords_nd, SemiLagrangian uses psi_star[i].coords_nd, _apply_monotone_limit non-dimensionalises explicitly. swarm.py was the outlier: grep coords_nd src/underworld3/swarm.py previously returned nothing.

Why it survived

It failed loudly rather than silently corrupting, and no test set reference quantities and then touched a proxy. tests/test_0116_swarm_proxy_units.py now does:

• the plain SwarmVariable proxy path;
• the IndexSwarmVariable level-set path, asserting partition of unity and that every level set stays in [0, 1] — the properties constitutive_models.py relies on when it divides by the level-set sum;
• a dimensionless control, so the fix cannot regress the untouched case.

Written first and shown to fail (2 failed, 1 passed — the control), then fixed (3 passed).

Regression

• level_1 tier_a/tier_b: 469 passed, 0 failed (466 before, plus the 3 new).
• np=2 swarm/index-swarm parallel suite: 7 passed, 4 skipped (those need np=4).

Deliberately not in this PR

A fourth instance of the same pattern is in Swarm._get_map, reachable only from the dead _rbf_reduce_to_meshVar. Both are scheduled for deletion under #428, so touching them here would be churn.

_create_proxy_variable does not forward units= to the proxy MeshVariable, so var.units is kg/m³ while var._meshVar.units is None. That is filed in #426 alongside this, but attaching units there would make var.sym unit-aware and change symbolic behaviour everywhere proxies are composed into expressions. It needs its own design rather than riding along with a coordinate fix, so #426 stays open for that half.

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 (retrospective — this PR was merged without one)

The house rule is an adversarial review posted publicly before a PR is marked ready. That did not happen here: #434 went straight to ready and was merged. Running it now against the merged state, 1e8aeb0e.

What the fix does not cover

Only three of four instances. Swarm._get_map has the identical pattern (var._meshVar.coords against a non-dimensional swarm kd-tree) and was left alone because its only caller, _rbf_reduce_to_meshVar, is dead code slated for deletion under #428. Defensible, but it means a grep for the pattern still hits, and if #428 stalls the fourth instance outlives the fix. It is called out in the commit message rather than marked in the source; a # TODO(BUG) at the site would have been better.

Half of #426 did not ship. The units= forwarding was deliberately deferred, and I said so in the PR body — but the squash commit's (#426) reference auto-closed the issue anyway. A known-unfixed defect sat closed until I noticed while reviewing status. Now split out as #439, with a note on #426. The process lesson is that referencing an issue number in a squash title closes it whether or not the fix is complete; a partial fix should reference the PR, not the issue.

What the test does not prove

test_0116_swarm_proxy_units.py asserts a constant field survives the refresh, and that the level sets sum to one and stay in [0, 1]. Constants are reproduced exactly by any partition-of-unity scheme, so this test confirms the call no longer raises — it does not confirm the transfer is correct under units. A linear field, or a comparison against the same model run dimensionless, would have been stronger. The bug was an exception rather than a wrong answer, so the weaker test does match the defect, but the file reads as broader coverage than it is.

The units fixture sets length and viscosity only; the run reports Not covered: ['temperature']. Nothing in the test needs temperature, so it passes — but a proxy on a temperature-carrying swarm variable under units is untested.

What I did not check

• No parallel test for the units path. The np=2 run covered the existing swarm suite, which is dimensionless. IndexSwarmVariable._update_proxy_variables has per-rank starved-rank branches, and coords_nd changes what every rank feeds its kd-tree; that combination is unexercised under units at np>1.
• No check that the answer matches the dimensionless run. coords_nd is the correct input, but I verified it stops raising, not that a units-active model and its dimensionless twin produce the same proxy.

What holds up

• The regression test was written first and shown to fail (2 failed, 1 passed — the dimensionless control), then fixed. The control is the part that matters: it stops the fix from being validated by a test that would pass either way.
• The change is three one-word substitutions with no behavioural surface beyond the units path; level_1 469 passed / 0 failed, np=2 swarm suite 7 passed.
• Splitting it from Local RBF interpolator with a linear-reproduction guarantee #430 was right: it is cherry-pickable to main and to feature branches without dragging in a new interpolator.