Callback-array parallel safety: remaining exposures after #376 (swarm.points callback, delayed-callback replay, central guard)

[lmoresi]

Follow-up to #376 (fixed in #378). The root cause there — NDArray_With_Callback.__array_finalize__ hands the sync callbacks to every array derived by indexing, so callbacks written for the canonical storage fire on views and temporary copies — is more general than the two canonical callbacks that were fixed. The historical disable_inplace_operators=True blocking on swarm.points ("disabled for parallel safety", af308d8, Sept 2025) was a symptom-level mitigation of the same disease. Four remaining items, none blocking #378:

1. swarm.points / swarm_update_callback still has the disease, plus a worse one

• The callback calls collective migrate() from inside a per-write callback — the exact pattern the swarm variable canonical callback was rewritten to avoid (SWARM-03 deferred migration) — and raises RuntimeError rank-locally on a size mismatch. Ranks that write unevenly diverge on collectives.
• The in-place blocking only covers the i-operators; __setitem__ on a derived fancy-indexed copy still fires the callback.
• The documented example in migration_control()'s own docstring raises today (verified):

with swarm.migration_control():
    swarm.points[mask1] += delta1
# RuntimeError: In-place addition (+=) is disabled for parallel safety.

The disable_inplace_operators flag propagates to the fancy copy via __array_finalize__, so the advertised pattern is simply broken. .points is already deprecated in favour of swarm variables — either port it to the guarded-callback + deferred-migration pattern (after which the blocking flag can be dropped and the docstring becomes true), or accelerate its retirement.

2. Delayed-callback replay is rank-asymmetric by construction

delay_callback / uw.delay_callbacks_global queue (callback, array, context) per rank and replay them between blanket barriers at context exit. Queue lengths and contents are rank-dependent (each rank queues what it locally triggered) while the callbacks contain collectives — the barriers around the batch cannot fix mismatched collectives inside it. Per-item exceptions are swallowed with logger.warning (the same swallow that hid #376 for ten months). The robust shape already exists in the codebase: the swarm canonical callback's _pending_petsc_sync marks variables dirty by NAME and flushes collectively at context exit. The delay machinery should do the same — flush dirty variables canonically, not replay rank-local event queues.

3. The #376 guard lives in two closures, not the class

Any future add_callback user re-inherits the trap. Add e.g. add_canonical_callback() on NDArray_With_Callback that applies the base-chain identity guard (view → sync canonical; copy → skip, the parent __setitem__ does the real sync) once, centrally, so the safe pattern is the default. Migrate the three existing registration sites (mesh variable, swarm variable, swarm.points) onto it.

4. Adjacent, lower severity

• Silent-bypass writes: np.add(x, 1, out=x), .fill(), np.copyto mutate without any callback → values land (the canonical wraps the vec memory) but ghost sync and the state increment don't happen — stale ghosts/caches rather than hangs.
• Every in-place op copies the entire array as old_value for callbacks that never read it — pure memory/bandwidth waste on large variables.

Context and verification details: PR #378 review discussion; the guard semantics (identity in numpy's base chain, NOT np.may_share_memory, which is False for any zero-size array and re-creates the rank asymmetry on empty ranks) are documented in the fixed callbacks in discretisation_mesh_variables.py and swarm.py.

Underworld development team with AI support from Claude Code

[lmoresi]

Additional finding from the second-pass review of the #378 fix commit, for the dirty-flag flush design (item 2):

The base-chain identity guard is rank-symmetric for indexing statements, but NOT for reshape/ravel of a non-contiguous derived view: var.data[:, ::2].ravel() is a copy on non-empty ranks yet a view on a rank whose local array is zero-size (numpy can always view-reshape an empty array), so an in-place write on that result classifies asymmetrically and can still mismatch the collective sync. The pattern is exotic and its write semantics are already silently broken on the non-empty ranks (the write lands in a dead copy), so it was judged non-blocking for #378 — the comments there now scope the claim and point here. This corner is unresolvable at the callback level (the two zero-size cases are locally indistinguishable); it falls out naturally once syncs are flushed from per-variable dirty flags instead of per-trigger callbacks.

Underworld development team with AI support from Claude Code

[lmoresi]

Implementation series is up, stacked and in draft until #378 merges (the two canonical callbacks touched here are the closures #378 patches inline — this series absorbs that conflict by deleting the inline guard in favour of the central one):

• Item 3 (central guard) + item 4b (old_value copies) → Callback-array parallel safety: central view/copy guard, honest exceptions, no dead snapshots (#379 items 3+4b) #381 bugfix/canonical-callback-guard (base: development). Adds add_canonical_callback() with the base-chain identity guard, migrates both canonical callbacks, deletes the dead unguarded factories, removes the exception swallow, drops the never-read old_value snapshots (12 in-place operators, not 11 as stated above).
• Item 1 (swarm.points) → Retire the swarm.points writable stack: read-only snapshot, coords is the write path (#379 item 1) #382 (stacked on Callback-array parallel safety: central view/copy guard, honest exceptions, no dead snapshots (#379 items 3+4b) #381). Decision: the mesh.points treatment rather than a port — deprecated read becomes a detached read-only snapshot (also removing a migration-on-READ side effect not listed above), setter refuses loudly pointing at swarm.coords / _particle_coordinates.data, migration_control() docstring example replaced with the interface that actually works and pinned by a test.
• Item 2 (delayed replay) → synchronised_array_update: collective dirty-variable flush replaces rank-local event replay (#379 item 2) #383 (stacked on Callback-array parallel safety: central view/copy guard, honest exceptions, no dead snapshots (#379 items 3+4b) #381). Dirty-marking + creation-order registration ids + unconditional allgather union at exit; swarm packs stay rank-local. Two adjacent traps had to fall with it: the .array views packed collectively per write (bypassing the context entirely), and lazy first-touch canonical creation was collective (now eager at construction, per the VE Stokes hangs on first solve at specific (np, mesh) combinations #130 reasoning). No-flush-on-exception is a deliberate behaviour change, argued in the PR.
• Item 4a (silent bypasses) → Intercept out= ufunc writes: close the silent callback bypass (#379 item 4a) #385 (stacked on synchronised_array_update: collective dirty-variable flush replaces rank-local event replay (#379 item 2) #383). __array_ufunc__ intercepts out= writes; np.copyto and ufunc.at remain documented bypasses.

Follow-up filed along the way: #384 (rank-asymmetric _temp_mag_{id(self)} temp-variable names, discovered because it rules out name-based flush keys).

Underworld development team with AI support from Claude Code