fix(swarm): parallel read_timestep restores each particle once (rank-0 routed read) + honest global-add docstring

[lmoresi]

Defect

Closes#324. Follows the investigation recorded on that issue (2026-07 audit,
SWARM findings family; docs/reviews/2026-07/REMEDIATION-WORKLIST.md).

Swarm.read_timestep read the full coordinate dataset on every rank and then
called add_particles_with_global_coordinates(..., migrate=True). Migration is
a scatter with no duplication-prevention code — each rank's copy of every point is routed to
the owning rank — so a parallel restore produced one copy of the saved swarm
per rank.

Reproduction (before the fix, np2, box mesh, fill_param=2):

AssertionError: DUPLICATION: saved 972, restored 1944 (2.0x)

The failure is silent in production: no error, just np-fold particle counts and
correspondingly wrong integration/statistics after every parallel restart.

The root cause of the pattern was a false promise in the
add_particles_with_global_coordinates docstring ("If the same array is passed
on every rank, this produces one copy of each point in the correct partition"),
which is mechanically untrue.

Fix

• Swarm.read_timestep(migrate=True) now reads the coordinate dataset on rank 0
  only, stages empty (0, dim) arrays on the other ranks, and lets
  Swarm.migrate() route each point to its owner — the same rank-0 routed-read
  design commit 9fb198d introduced for SwarmVariable.read_timestep. The
  serial path and the migrate=False escape hatch (full per-rank copy, points
  outside the mesh preserved) behave exactly as before.
• The add_particles_with_global_coordinates docstring now states the real
  contract: a low-level primitive with no locality filtering and no de-duplication;
  correct usage is migrate=False full-copy, or pre-partitioned / rank-0-only
  input followed by migration. add_particles_with_coordinates is
  cross-referenced as the safe rank-identical-input method. Its legitimate
  migrate=False callers (evaluation swarms, mesh transfer, global evaluation)
  are untouched.

Tests

• New tests/parallel/test_0757_swarm_read_timestep_mpi.py (inside CI's
  tests/parallel/test_075* glob): restored global count == saved count,
  gathered/sorted coordinates match to 1e-12, and a SwarmVariable read back
  on top of the restored swarm reproduces its analytic values. Written first
  and shown to fail (2.0x duplication) against the unfixed code.
• tests/test_0003_save_load.py::test_swarm_save_and_load now asserts the
  restored particle count (it previously checked nothing).

Gate results:

• Serial pytest tests/ -m "level_1 and tier_a" -q: identical before and
  after the fix — 328 passed, 10 skipped, 4 xfailed, 1 xpassed both runs.
• Full tests/test_0003_save_load.py: 6 passed.
• New test at np2: 2 passed; at np4: 2 passed.
• Swarm parallel set (test_0755, test_0756, test_0765, test_0766):
  np2 20 passed / np4 21 passed, with ONE pre-existing failure in
  test_0765_internal_boundary_integral_mpi.py — a direct _deform_mesh
  call rejected by the live-mesh guard from PR fix: medium Track-0 misc — parallel BoxInternalBoundary, SL theta restore, viewer crash, projection double-count, units-boundary honesty (BF-10a/12/13/15/18, D9) #326, present on
  development independently of this change (flagged with a TODO(BUG)
  in the test).

Also flagged (TODO(BUG), out of scope here): Swarm.save returns on non-zero
ranks while rank 0 is still appending metadata, so an immediate reopen can hit
HDF5 file locking (BlockingIOError errno 35) — found while reproducing #324.

Underworld development team with AI support from Claude Code

[Copilot]

Pull request overview

This pull request fixes a parallel restart defect in Swarm.read_timestep where each MPI rank previously restored the full saved coordinate set and then migrated, resulting in an np-fold duplication of particles. The fix switches to a rank-0-only read (staging empty arrays on other ranks) and then uses migration to route particles to their owning ranks, and it corrects the misleading contract described in add_particles_with_global_coordinates’s docstring.

Changes:

• Fix Swarm.read_timestep(migrate=True) to read coordinates on rank 0 only and migrate from a single inserted copy, preventing np-fold duplication in parallel restores.
• Update add_particles_with_global_coordinates docstring to accurately describe its non-deduplicating semantics and point users to safer alternatives.
• Add/strengthen regression tests: a new MPI test that would have failed before the fix, and a serial save/load test that now asserts restored particle count.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File	Description
src/underworld3/swarm.py	Implements rank-0 routed read for swarm coordinate restore; updates docstring contract; adds a TODO note about a known save/metadata race.
tests/parallel/test_0757_swarm_read_timestep_mpi.py	New MPI regression tests verifying restored swarm count/coordinates match saved state and that swarm variables read correctly post-restore.
tests/test_0003_save_load.py	Strengthens serial save/load test by asserting restored swarm particle count matches the saved count.
tests/parallel/test_0765_internal_boundary_integral_mpi.py	Adds an explanatory TODO comment about an unrelated pre-existing failure due to _deform_mesh live-mesh guard behavior.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[lmoresi]

Design discussion (maintainer review): the rank-0-routed read fixes the duplication but adopts a filesystem-dependent trade-off that deserves to be explicit rather than inherited.

The primitive was never broken — add_particles_with_global_coordinates is a correct low-level tool; only its docstring (and one caller) were wrong. And rank-0 reading is not inherently better — it was chosen here for consistency with the 9fb198d pattern in SwarmVariable.read_timestep, which was solving an aggregate-memory blow-up (np × N_global simultaneous holds), not an IO problem. As an IO pattern it serializes the read through one process and adds an all-to-one/one-to-all hop: the worst case on a scalable parallel filesystem.

The strategy space:

1. Rank-0 read + migrate scatter (this PR): best on weak/NFS filesystems (single reader avoids contention); memory hotspot on rank 0; serialized IO.
2. Read-everywhere + keep-local (the model this codebase already implements in add_particles_with_coordinates): every rank reads, filters by points_in_domain, keeps its own — zero communication, no migrate, no hotspot; cost is np-fold read amplification, which striped/cached parallel filesystems absorb; transient full-array hold per rank, chunkable if ever needed.
3. Slice + migrate (the extreme-scale endpoint): each rank independently reads a contiguous 1/np slice (disjoint read-only HDF5 access, no mpio required) then one migrate routes ownership — each byte read once, N/np memory.

Which is 'best' depends on the filesystem; none is universally correct. The regression tests added here assert outcomes (count / coordinates / values at np2 and np4), not strategy — so the implementation can be swapped without touching them.

Proposed follow-up per maintainer steer: switch this PR's restore path to strategy 2 (the filtering sibling — smaller diff, no comms), document the trade-off at the call site, and reconsider SwarmVariable.read_timestep's rank-0 pattern in the same terms as its own issue.

Underworld development team with AI support from Claude Code

[lmoresi]

Strategy swap done per the design comment (commit b2da15f): Swarm.read_timestep(migrate=True) now uses the keep-local model — every rank reads the coordinate dataset with plain read-only h5py, and add_particles_with_coordinates keeps only the locally-owned points via points_in_domain. No migrate call, no communication, no rank-0 hotspot; the np-fold read amplification trade-off is documented at the call site, and SwarmVariable.read_timestep's rank-0-routed pattern is left as is pending its own reconsideration.

The migrate=False escape hatch (full per-rank copy via the global method, out-of-mesh points preserved) and the serial path are unchanged.

Verification after the swap (single amr-dev env, site-packages confirmed byte-identical to src):

• Reproduction script: np2 restore saved=972 restored=972.
• tests/parallel/test_0757_swarm_read_timestep_mpi.py UNCHANGED: 2 passed at np2, 2 passed at np4.
• tests/test_0003_save_load.py: 6 passed.
• Serial pytest tests/ -m "level_1 and tier_a" -q: 328 passed, 10 skipped, 4 xfailed, 1 xpassed — identical to the pre-fix baseline.

Underworld development team with AI support from Claude Code