fix(swarm): Track-0 stale-cache and migration-semantics fixes (BF-02/05/06/07/08 + #286)#313
Merged
Merged
Conversation
…#286) In nuke_coords_and_rebuild() the point-location kd-tree was rebuilt before self._nav_coords was refreshed from the rebuilt DM, so on an adapted mesh the rebuild indexed an old-sized coordinate array with new-mesh point ranges. When the mesh grew this raised IndexError (index 91 out of bounds for axis 0 with size 80) from swarm migration after mesh.adapt(); when it shrank it silently mislocated points. The navigation-coordinate refresh (both the volume-mesh and manifold branches) now runs before _build_kd_tree_index(). Reproduced by tests/test_0810_amr_swarm_migration_regression.py, which now passes. Underworld development team with AI support from Claude Code
…tion, and populate (BF-02) Swarm.migrate() early-returned when no particle was globally unclaimed without invalidating the canonical .data caches, the particle kd-tree, or the proxy staleness flags (SWARM-01/SWARM-02, 2026-07 audit). In serial or whenever no particle changes rank - the common case - every cached array kept the old particle count and the kd-tree kept a poisoned no-copy view of mutated coordinates. This is the root cause of issue #289: advection ends with a no-move migrate, so material proxies froze at the launch positions and time-stepped models silently used stationary material. The early return now performs the same invalidation as the fall-through path. add_particles_with_global_coordinates() gains an explicit invalidation (mirroring the #216 fix in add_particles_with_coordinates - with migrate=False nothing else invalidates), and populate() invalidates caches created before the swarm had particles (SWARM-17, verified at runtime). Collateral defect exposed by this fix, same #216 stale-cache class: Swarm.apply_snapshot_payload() wrote restored variable data into a detached np.asarray view of the canonical cache, so the DMSwarm field never received the restored values - the restore only worked because the stale canonical copy survived. It now writes through the canonical array so the PETSc pack fires (caught by test_0007_snapshot_inmemory). Regression tests: tests/test_0113_swarm_stale_cache_regression.py (SWARM-01 both migrate= variants, SWARM-17, SWARM-02 kd-tree contract and behavioural mirror probe, and the issue #289 reproducer). Underworld development team with AI support from Claude Code
…f discarding them (BF-05) Variable and coordinate writes made inside migration_disabled() / migration_control() were silently discarded (SWARM-04, 2026-07 audit): both PETSc-sync callbacks early-returned while _migration_disabled was set, nothing ever re-packed the canonical arrays, migrate() then read the stale PETSc coordinates, and its trailing invalidation destroyed the only copy of the writes. The docstrings explicitly recommended writing inside the context, and the parallel test covering the pattern asserted vacuously. Suppressing migration now defers only the PETSc pack, never the data: writes made while the flag is set are recorded per variable and flushed into the DMSwarm by Swarm._flush_pending_petsc_sync() at context exit (and defensively at migrate() entry). Both context-manager modes flush; only the migrate() call itself is deferred (default) or skipped (disable=True). Docstrings updated to state the actual contract. This commit also introduces Swarm._sync_before_assembly() (adjacent to the flush helper): the collective solve-entry synchronisation used by the deferred-migration and stale-proxy fixes. It is wired into the mesh assembly path in the follow-up commit. tests/parallel/test_0755_swarm_global_stats.py's perturbation block now asserts the write actually reaches the DMSwarm (previously vacuous), and tests/parallel/test_0756_swarm_migration_semantics.py (added in the follow-up commit) covers write survival at np2. Underworld development team with AI support from Claude Code
…or zeroing (BF-06) Ranks holding <= 1 particles either hard-crashed or silently corrupted their proxy mesh variables (SWARM-07, 2026-07 audit): rbf_interpolate() returned silent zeros that _rbf_to_meshVar wrote straight into the proxy nodal values, and IndexSwarmVariable's proxy update reached unguarded KDTree construction on an empty coordinate array, raising IndexError inside a collective update - an MPI abort or hang. Starved ranks now leave their proxy nodal values untouched and warn (warnings are suppressed for a swarm that has never been populated, since proxied variables legitimately touch this path at creation time). The guards are written to be collective-safe: MeshVariable reads/writes perform collective ghost synchronisation, so every rank executes the same read-then-write sequence and only the values differ per rank. The IndexSwarmVariable update_type=0 projection is restructured to compute into a local buffer and issue exactly ONE MeshVariable write per level set per rank (serially bit-identical to the old formulation): the previous in-context version issued a data-dependent number of writes whose deferred collective syncs mismatched across ranks - the np4 starved-rank test deadlocked on it. IndexSwarmVariable also gains a real _update_proxy_if_stale() override (its proxies live in _meshLevelSetVars, so the base implementation was a no-op for it) and routes .sym through it; this is also what the solve-entry refresh uses. np4 regression test (all particles seeded on one rank, sentinel survives on starved ranks, no crash) lands with the parallel test file in the follow-up commit. Underworld development team with AI support from Claude Code
…refresh at solve entry (BF-07, BF-08) BF-07 (SWARM-03, 2026-07 audit): the Swarm docstring promised automatic migration, but coordinate writes through the modern interface (swarm.coords setter / swarm._particle_coordinates.data) only packed to PETSc - only the deprecated points path migrated, leaving particles on the wrong rank in parallel. Coordinate writes now mark swarm._needs_migration; the collective migrate() runs DEFERRED at the next collective point - migration-control context exit or solve entry - never per-write, which would deadlock when ranks write unevenly (maintainer-approved fix shape). advection() suspends the deferred migration around its substep loop (a migrate firing from its velocity evaluations would reorder particle rows between captured arrays) and runs its own migrate() at the end, which clears the suspension. BF-08 (LE-03 = SWARM-05, issue #215 Bug 3): proxy refresh fired only via the lazy .sym accessor, but solvers pull the proxy DM directly through mesh.update_lvec(), consuming stale data after a material.data write. Mesh.update_lvec() now runs Swarm._sync_before_assembly() for each registered swarm before its staleness check: a single eager, collective refresh at solve entry (the refresh itself is what sets _stale_lvec). Rank-local flags are combined with MAX reductions so uneven writes are safe; repeated calls are flag-guarded no-ops, preserving the test_0006_memory_leak constraint. The refresh calls inside petsc_interpolate pass swarm_sync=False - only a subset of ranks reaches that function (zero-interior-point ranks skip it), and the hook's reductions must not run on a subset; those sites already rely on the all-ranks update_lvec() in evaluate() for freshness. The resolved TODO(BUG) at the old swarm.py:1075 is removed. Together with the BF-02 invalidation fix this closes issue #289: the issue reproducer (regression test in test_0113) shows the material proxy tracking the advected particles, and a Projection re-solve consumes fresh particle data without touching .sym. Adds tests/parallel/test_0756_swarm_migration_semantics.py: np2 migration-suppressed write survival, np2 deferred migration at context exit and at solve entry, np4 starved-rank proxy behaviour. Underworld development team with AI support from Claude Code
Contributor
There was a problem hiding this comment.
Pull request overview
This PR implements Track-0 remediation for Underworld3’s swarm migration / cache invalidation / proxy freshness semantics, and fixes an AMR point-location regression (#286) by reordering cache rebuilds. It also adds/repairs regression tests to prevent stale-cache, stale-proxy, and starved-rank MPI failure modes from reappearing.
Changes:
- Fix swarm cache invalidation and deferred-migration/proxy-refresh semantics (including solve-entry swarm sync) to prevent stale
.datacaches, stale kd-trees, and stale proxy mesh variables. - Fix AMR mesh rebuild ordering so
_nav_coordsis refreshed before rebuilding the navigation kd-tree index (#286). - Add new serial + MPI regression tests covering the Track-0 defects and repair a previously vacuous parallel test assertion.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_0113_swarm_stale_cache_regression.py | Adds serial regression coverage for stale-cache, kd-tree invalidation, proxy freshness through advection, and solve-entry proxy consumption. |
| tests/parallel/test_0756_swarm_migration_semantics.py | Adds MPI regression coverage for migration-control semantics, deferred migration, and starved-rank proxy safety. |
| tests/parallel/test_0755_swarm_global_stats.py | Strengthens an MPI test to ensure coordinate writes inside migration_disabled() actually reach PETSc (non-vacuous). |
| src/underworld3/swarm.py | Implements deferred PETSc sync flushing, deferred migration flagging, solve-entry proxy refresh, starved-rank guards, cache invalidation fixes, and snapshot restore correctness. |
| src/underworld3/function/_function.pyx | Avoids running swarm-sync logic from petsc_interpolate() on only a subset of ranks by adding swarm_sync=False for those paths. |
| src/underworld3/discretisation/discretisation_mesh.py | Refreshes _nav_coords before rebuilding kd-tree index and adds a swarm_sync control path to update_lvec() to prevent subset-rank deadlocks. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+2361
to
+2369
| # manifold mesh: the nav clone carries its own (ghosted) coords; | ||
| # refresh them from the rebuilt main DM where possible. | ||
| try: | ||
| self._nav_dm.setCoordinatesLocal(self.dm.getCoordinatesLocal()) | ||
| self._nav_coords = numpy.asarray( | ||
| self._nav_dm.getCoordinatesLocal().array | ||
| ).reshape(-1, self.cdim) | ||
| except Exception: | ||
| pass |
Comment on lines
2536
to
+2542
| elif self.update_type == 1: | ||
| # NOTE: this branch performs data-dependent MeshVariable writes | ||
| # outside any access context, which is not parallel-safe | ||
| # independently of the starved-rank issue (pre-existing). | ||
| # The guard here only prevents the empty-rank KDTree crash. | ||
| if starved: | ||
| return |
Comment on lines
+4943
to
+4949
| # Suspend the deferred (solve-entry) migration for the duration of | ||
| # the substep loop: the velocity evaluations below pass through | ||
| # Mesh.update_lvec(), and a migrate() firing there would reorder | ||
| # particle rows between the coordinate array and the velocity array | ||
| # captured from it. advection() performs its own migrate() at the end. | ||
| self._deferred_migration_suspended = True | ||
|
|
…ojection (fossil contract) CI caught tests/test_0726_bare_variable_composition_137.py failing on this branch: its module fixture drops the parent swarm and keeps only the SwarmVariable, then composes with the variable's .sym. On development this silently worked because populate() never marked the proxy stale (the SWARM-17 hole) - .sym returned the proxy without ever dereferencing the dead parent. With the invalidation fixed, the lazy refresh dereferenced the weakly-referenced swarm and raised RuntimeError from swarm.py's lifetime guard. Contract decision (rather than a band-aid): variables continue to hold their parent swarm by WEAK reference - a strong back-reference would cycle with the swarm's own strong _coord_var/_X0 members and defer DMSwarm destruction from refcount-immediate to gc time, re-opening the transient-evaluation-swarm leak that the WeakValueDictionary/__del__ redesign eliminated (guarded by tests/test_0006_memory_leak.py). A variable that outlives its swarm is therefore a symbolic FOSSIL: the lazy refresh (base SwarmVariable and IndexSwarmVariable overrides) now detects the dead parent, warns that the proxy retains its last projection and cannot be refreshed, and returns - .sym stays usable for expression composition. Particle-data paths (.data, rbf_interpolate, ...) still raise through the unchanged .swarm lifetime guard. Adds test_variable_outliving_swarm_is_a_usable_fossil to test_0113 pinning the contract (warn-not-raise on .sym; RuntimeError preserved on data access). Gates: test_0726 5/5; test_0113 7/7; test_0810 1/1; CI-equivalent unmarked batch tests/test_05* + tests/test_07* = 429 passed, 0 failed; np2 swarm parallel set 19 passed. Underworld development team with AI support from Claude Code
Member
Author
|
CI failure in Root cause: the test's module fixture drops the parent swarm and keeps only the SwarmVariable. On development this silently worked because Fix by contract, not band-aid: variables keep holding their swarm weakly — a strong back-reference would cycle with the swarm's own strong Gates re-run after the fix: test_0726 5/5, test_0113 7/7, test_0810 1/1, CI-equivalent unmarked Underworld development team with AI support from Claude Code |
This was referenced Jul 5, 2026
lmoresi
added a commit
that referenced
this pull request
Jul 5, 2026
…ecation, recycle excision, substep/save verification (BF-11/14/16/17, D4/D5/D6) (#323) * fix(swarm): excise broken particle-recycling machinery, delete dead pic_swarm.py (SWARM-08/09, WA-02/03, D4) The recycle_rate > 1 (streak swarm) feature was advertised in the Swarm docstring but broken at two independent points: populate() raised NameError on the undefined 'all_local_cells' (runtime-confirmed) and advection() indexed the undefined 'cellid' whose binding was commented out. The only working copy of the logic lived in swarms/pic_swarm.py, a 1,534-line module that was never installed (no __init__.py, broken relative import) and had already diverged from the live Swarm class. Per the maintainer ruling of 2026-07-06 (decision D4), the recycling machinery is excised rather than repaired: pic_swarm.py is deleted (git history preserves it), the dead recycle_rate > 1 code paths in populate(), advection(), rbf_interpolate() and the particle-add methods are removed, and constructing a Swarm with recycle_rate > 1 now raises NotImplementedError with a clear message instead of crashing later. The recycle_rate and rebuild_on_cycle parameters are retained (inert) so existing no-recycling calls keep working; docstrings now state this honestly. Regression test: test_0110_basic_swarm.py::test_recycle_rate_not_implemented. Underworld development team with AI support from Claude Code * fix(swarm): NodalPointSwarm keyword-explicit super().__init__ + deprecation (SWARM-11, BF-14, D5) NodalPointSwarm.__init__ called super().__init__(mesh, verbose, clip_to_mesh=False) against Swarm.__init__(self, mesh, recycle_rate=0, verbose=False, clip_to_mesh=True): the verbose argument landed positionally in the recycle_rate slot and was silently discarded (runtime-confirmed: verbose=True produced recycle_rate=True, verbose=False). The super call is now keyword-explicit. Per the maintainer ruling of 2026-07-06 (decision D5) the class is also deprecated: it has zero remaining internal instantiation sites (the semi-Lagrangian history managers dropped their nodal-swarm cache) and will be removed next release cycle. Construction now emits a DeprecationWarning while remaining fully functional during the warning period. Regression test: test_0110_basic_swarm.py::test_nodal_point_swarm_deprecated_but_working asserts the warning, that verbose reaches Swarm.__init__, and that the swarm still builds one particle per tracked-variable node. Underworld development team with AI support from Claude Code * fix(swarm): array-view reductions follow the MeshVariable per-component-tuple contract (LE-07, BF-11, D6) SwarmVariable array-view reductions (max/min/mean/sum/std on both SimpleSwarmArrayView and TensorSwarmArrayView) returned a single scalar for multi-component variables, while the MeshVariable array views return a per-component tuple. Reproduced live: a 2-vector swarm variable's .array.max() returned one float. Callers that ported code between mesh and swarm variables silently got whole-array statistics instead of per-component ones. Both view classes now share a _per_component_reduction helper: float for single-component variables, per-component tuple otherwise (components in canonical flat order), matching the MeshVariable contract. Per the maintainer ruling of 2026-07-06 (decision D6) this return-type behaviour change is approved. No internal callers rely on scalar returns from multi-component swarm reductions (checked src/, tests/, docs/ — all existing reduction call sites are on single-component variables, which are unchanged). Tests: the three masking skips in test_0850_comprehensive_reduction_operations.py are removed (the unskip is the regression test); the tensor case gains the required explicit vtype (shared, deliberate design — MeshVariable raises the same ValueError); the stale class-level xfail is narrowed to the one test that genuinely awaits a global MeshVariable.std(). Underworld development team with AI support from Claude Code * fix(swarm): save() sequential fallback writes model-unit coordinates like the parallel branch (SWARM-19, BF-17) Verified real: Swarm.save() wrote different coordinate systems per IO branch. The parallel-HDF5 branch saves _particle_coordinates.data (model units) but the sequential fallback (force_sequential=True, or h5py built without MPI) saved the deprecated self.points property, which multiplies by the model length scale when coordinate scaling is active. With a 1000 km reference depth the two branches produced checkpoints differing by a factor of 1e6 (reproduced: 128/128 coordinates mismatched between branches), and a sequential checkpoint could not round-trip through read_timestep — every particle landed outside the model-unit mesh domain and was deleted (reproduced: 64 particles saved, 0 restored). Both branches now write model-unit coordinates, the convention the parallel branch and read_timestep already used. This also removes the sequential path's DeprecationWarning and its migration side-effect (self.points can trigger a mesh-version migration check on read). Regression tests: test_0114_swarm_save_coordinate_units.py compares both IO branches under active coordinate scaling and round-trips a sequential checkpoint through read_timestep. Underworld development team with AI support from Claude Code * fix(swarm): substepped advection evaluates launch-point velocity globally (SWARM-16, BF-16) Verified real. Swarm.advection() with substeps > 1 evaluated each substep's launch-point velocity with the rank-LOCAL uw.function.evaluate() while no migration happens inside the substep loop (deferred migration is deliberately suspended there so array row order stays stable — see #313). From substep 2 onward a particle that crossed a partition boundary sits outside its rank's domain and the local evaluation silently extrapolated wrong velocities for it. Reproduced with an np2 solid-body-rotation trajectory test: 12/64 final coordinates wrong by up to 3.6e-3 (roundoff-level expected) with no error raised. The launch-point evaluation now uses uw.function.global_evaluate(), exactly like the midpoint evaluation in the same loop (the order-1 path already did this). Row correspondence is preserved: global_evaluate returns values for the local query points in order, and no migration is introduced mid-loop. Also fixes the estimate_dt() shape bug this investigation exposed: evaluate() returns matrix-shaped (n, 1, dim) arrays, so vel[:, 1] indexed the size-1 axis and the swallowed IndexError made estimate_dt() return None for every non-trivial velocity — which silently disabled step_limit substepping entirely (substeps quietly fell back to 1). Velocity samples are now flattened to (n, dim) before the magnitude computation. Regression tests: tests/parallel/test_0766_swarm_substep_advection.py (np2/np4: ring of particles under solid-body rotation, 6 forced substeps, compared per-particle against an exact numpy replication of the midpoint scheme) and test_0110_basic_swarm.py::test_estimate_dt_with_mesh_variable_velocity. Underworld development team with AI support from Claude Code * fix(swarm): remove remaining pic_swarm breadcrumb comments (SWARM-09, D4 follow-through) Comment-only: deletes the commented-out pic_swarm import block that survived the D4 excision commit. No code change. Underworld development team with AI support from Claude Code
This was referenced Jul 6, 2026
Merged
lmoresi
added a commit
that referenced
this pull request
Jul 6, 2026
…tub banner (WE-11) (#335) * docs(design): swarm modernization design doc (FO-01) Campaign dimension-5 deliverable (2026-07 quality campaign): the design blueprint for modernizing the swarm subsystem, building on docs/reviews/2026-07/SWARM-SUBSYSTEM-REVIEW.md and the FO-01 worklist row. Covers, each with current behaviour / problem / target design / migration path + tests, all verified against development @ 3184a40 (code reading plus runtime probes): - self-validating canonical cache (SWARM-10): generation+size token - migration trigger matrix (SWARM-03/18): post-#313 state machine and gaps - shared array-view refactor for both variable families (SWARM-14) - rank-local RBF seam behaviour (SWARM-15) and relation to #314 - _get_map stale-cache trap (SWARM-23): delete the dead trio - KDTree copy-in-__cinit__ (SWARM-20): probe-quantified inconsistency - checkpoint/restore fidelity audit (post-#329 keep-local, #333, #330) - the fossil contract (#313) and how the new cache preserves it Includes a Track-0 finding-status table (fixed findings cited as history, not open problems), non-goals, a phasing plan (FO-02 early items), a test strategy with tier assignments, and ten numbered maintainer questions. Linked into the developer design-documents toctree. Underworld development team with AI support from Claude Code * docs(subsystems): honest banner on the swarm-system stub (WE-11) The 26-line stub claimed the swarm subsystem was 'well documented', 'priority low', citing a nonexistent swarm/ module of 4,484 lines (SWARM-24) - actively misdirecting reviewers away from the subsystem the 2026-07 audit found most in need of attention. Replace the misleading content with a warning banner pointing at the audit (docs/reviews/2026-07/SWARM-SUBSYSTEM-REVIEW.md, with a caveat that many findings are since fixed) and at the modernization design document as the current authorities, until the FO-01 refactor delivers the real subsystem documentation. Underworld development team with AI support from Claude Code
lmoresi
added a commit
that referenced
this pull request
Jul 6, 2026
…patterns (WB-01..06) (#337) * refactor(internal): WB-01 migrate swarm.py off deprecated access() context managers Remove the four remaining live 'with ...access(...)' wrappers in swarm.py (SWARM-13, Wave B of the 2026-07 remediation worklist): - SwarmVariable._rbf_reduce_to_meshVar: read-only block, wrapper dropped - NodalSwarm.__init__ (nX0/nI0 launch-point writes) - NodalSwarm.advection (_X0 / _nR0 writes) Swarm.access() is already a no-op compatibility shim whose only effect is to defer NDArray_With_Callback callbacks to context exit; direct .data writes fire the identical canonical PETSc-pack callback immediately, so the end state is unchanged (the sanctioned raw .data exception, Charter section 7). None of the written variables is the swarm coordinate variable, so no migration semantics are involved. The audit counted 13 sites at development@1d003481; Track 0 (#310/#313), Wave A, and Wave C had already removed all but these four. Underworld development team with AI support from Claude Code * refactor(internal): WB-03 migrate adaptivity.py off deprecated access() context managers Remove the four 'with mesh.access(...)' / 'with swarm.access(...)' wrappers in adaptivity.py (Wave B of the 2026-07 remediation worklist): - create_metric: direct metric.data write - metric_from_field: read-only indicator.data copy - mesh2mesh_swarm: read-only particle-coordinate/variable gather - mesh2mesh_meshVariable: direct tmp_varS.data write Both access() shims are no-op compatibility wrappers that only defer NDArray_With_Callback callbacks to context exit; direct .data access is the sanctioned raw-copy exception (Charter section 7) and produces the identical PETSc pack. Underworld development team with AI support from Claude Code * refactor(internal): WB-05 retire internal mesh.data references (mesh.X.coords) Wave B of the 2026-07 remediation worklist. The audit's ~14 mesh.data sites had already been reduced to zero live coordinate reads by earlier waves; what remained in src/ were docstring examples teaching the deprecated pattern, plus one live 'with self.access(var)' wrapper in Mesh.update_lvec's field-decomposition loop (the wrapper only deferred write callbacks around a pure .vec read, which lazily creates the vector itself — removal is a no-op). - coordinates.py: two geographic-conversion docstring examples -> mesh.X.coords - functions_unit_system.py: evaluate() docstring example -> mesh.X.coords - surfaces.py: transfer_normals docstring text (code already reads the mesh's own vertex coordinates) - nd_array_callback.py: delay_callbacks_global example rewritten with modern .array writes - discretisation_mesh.py: drop the redundant access() wrapper in update_lvec The mesh.data deprecation property itself (and Mesh._legacy_access) are intentionally untouched; the mesh.adapt() docstring example is left for WE-07, which owns it. Underworld development team with AI support from Claude Code * refactor(internal): WB-06 delete Swarm._legacy_access (zero callers) With WB-01 landed there are no remaining internal callers of the old context-managed access machinery on the swarm side. Delete Swarm._legacy_access (the pre-2025 getField/restoreField context manager with its exit_manager migration/proxy-update hooks) and the now-orphaned SwarmVariable._is_accessed init that only it consulted. Verified by grep across src/ and tests/: the only _legacy_access references left are Mesh._legacy_access (out of scope for WB-06) and comments. The deprecated public Swarm.access() compatibility shim is retained unchanged as the documented entry point for legacy user code. Underworld development team with AI support from Claude Code * refactor(internal): WB-04 drop the one live access() wrapper in _function.pyx Wave B pyx verification (2026-07 remediation worklist). The seven deprecated-pattern references the audit indexed in cython/petsc_generic_snes_solvers.pyx are all commented-out fossils (currently lines 3146, 3187, 4212, 4259, 8225, 8315, 8359); per the pyx rule they are left for the Wave A/D pyx sub-wave (WA-07) rather than touched here. Grep did find one LIVE site outside that file: _function.pyx wrapped a single work_var.data write in 'with mesh.access(work_var):'. mesh.access is the no-op deferred-callback shim, so removing the wrapper around one direct .data write is the sanctioned trivially-local migration allowed by the WB-04 rule. Underworld development team with AI support from Claude Code
lmoresi
added a commit
that referenced
this pull request
Jul 6, 2026
…og, value-first call-site sweep (WE-01..03,05,06,08,09,10) (#338) * docs(WE-01): adopt the one-governing-doc-per-topic authority map Repoint CLAUDE.md's Data Access 'Authoritative Reference' from the stale UW3_Style_and_Patterns_Guide.md to subsystems/data-access.md (the guide it crowned teaches patterns the code deprecates at runtime — DOC-04), and record the Style Charter §10 authority table in docs/developer/index.md as the master authority index. The Charter is added to the Getting Started toctree (removes a baseline 'not included in any toctree' warning). Finding: DOC-04 (docs/reviews/2026-07/DOCS-STANDARDS-COHERENCE.md). Underworld development team with AI support from Claude Code * docs(WE-02): de-drift the Style Guide's four stale normative sections Rewrites the sections DOC-01 verified as contradicting the settled standards: - Docstring format: the 'Markdown Docstrings for pdoc/pdoc3' section is replaced by the NumPy/Sphinx RST standard (worked example with :math: and Parameters/Returns/Examples/Notes; conversion tracked in docs/plans/docstring-conversion-plan.md), per Style Charter section 6. - Doc file format: Quarto .qmd prescription (zero .qmd files exist in the repo) replaced by MyST .md/Sphinx guidance matching CLAUDE.md; migration table row updated. - Data access examples: 'Preferred' coordinate examples now use the real, runnable API — mesh.X.coords (read), mesh.deform() (coordinate changes), and the swarm.coords getter/setter for particle positions. The previous 'Preferred' example swarm.data += displacement raises AttributeError (getter-only property — SWARM-13 evidence); mesh.data warns at runtime. The private-attribute migration advice (swarm._particle_coordinates, mesh._deform_mesh presented as the NEW pattern) is deleted. - Front matter: the 21-line Quarto YAML header is replaced by a minimal MyST title block, and the guide now states that the UW3 Style Charter is the normative contract and wins on conflict. All replacement examples verified against current source: Swarm.coords setter (swarm.py), Mesh.deform (discretisation_mesh.py:3133), uw.synchronised_array_update / NDArray_With_Callback.delay_callbacks_global. Findings: DOC-01, SWARM-13 (style-guide part). Underworld development team with AI support from Claude Code * docs(WE-03): regenerate the docstring review queue; add the sweep to the release checklist The queue (last generated 2026-01-13, cdf5bb2) misrepresented the codebase both ways: it flagged now-complete items (solve, SNES_Scalar) as missing and contained zero entries for the June 2026 API (DOC-02). Regenerated over src/underworld3/**/*.py + **/*.pyx at the current tip. Two bugs in scripts/docstring_sweep.py's regex-based Cython parser made the regenerated queue lie about .pyx docstrings and are fixed as part of making the regeneration meaningful: - the indent group '(\s*)' with re.MULTILINE consumed preceding blank lines, shifting the computed definition line so the docstring search started ON the def/class line and always missed; - the docstring search started at the definition line rather than after the (possibly multi-line) signature, so long signatures hid their docstrings; - raw-string docstrings (r""", the norm in the solver .pyx) were not recognised. DOC-02 cross-validation on the regenerated queue now passes: solve / SNES_Scalar in the solver pyx are no longer flagged 'none'; the queue contains the June API (add_nitsche_bc, add_rotated_freeslip_bc, boundary_flux, set_custom_fmg, consistent_jacobian: 13 mentions) and flags the DOC-05 targets (Swarm.advection x2, read_timestep, write_proxy) as undocumented. Also adds the sweep to the quarterly release checklist (guides/release-process.md) so the queue cannot go stale unnoticed again. Findings: DOC-02 (docs/reviews/2026-07/DOCS-STANDARDS-COHERENCE.md). Underworld development team with AI support from Claude Code * docs(WE-05): backfill the changelog for May - early July 2026; add the changelog sweep to the release checklist The changelog (the quarterly CIG/stakeholder record) ended in April 2026 while ~117 first-parent commits landed May through early July (DOC-03). Backfilled at the existing conceptual granularity — 14 grouped entries, grouped by subsystem rather than by PR, matching the established format (### Title (Month Year), bold lead sentence, hyphen bullets, inline PR references): - New '2026 Q3 (July - September)' section: the July 2026 quality campaign (#309-#313, #317, #322-#326, #329, #334 as grouped entries), rotated strong free-slip / boundary traction / dynamic topography (#293, #294, #298, #306), generalized geometric multigrid via custom prolongation (#290, #297), consistent Jacobian tangent (#258), swarm correctness (#216, #313, #323, #329), numpy 2 support (#301, #305). - Extended '2026 Q2' section with the May-June entries: mesh adaptation movers (#190, #209, #213, #228, #259, #264, #266), moving-mesh field transfer / deform() (#246, #249, #251), semi-Lagrangian accuracy controls (#164, #183, #185-#189, #208, #220), snapshot/checkpoint toolkit (#146, #195, #196, #198), Stokes_Constrained (#224, #229, #240, #265), local-h Nitsche + boundary-slip surfaces (#225, #241, #275), units interoperability (#277, #278, #283, #284), memory/evaluation/solver infrastructure (#161, #177-#179, #181, #182, #222, #237, #250, ...). Every entry is backed by a merged commit on development (verified against git log --first-parent aed517f..3184a40). Also adds a quarterly-changelog sweep step beside the docstring sweep in the release checklist (guides/release-process.md) per DOC-03's proposed fix. Findings: DOC-03 (docs/reviews/2026-07/DOCS-STANDARDS-COHERENCE.md). Underworld development team with AI support from Claude Code * docs(WE-06): status headers on the unmarked design docs (per-doc git verification) Adds one-to-three-line Status markers to the 13 design docs that lacked one, following the directory's existing conventions (**Status**: line under the title; status: key inside existing YAML frontmatter for the three frontmatter-only docs), and corrects the stale 'Design Phase' marker on MATHEMATICAL_MIXIN_DESIGN.md (the mixin ships in utilities/mathematical_mixin.py). Every stamp was verified against git history (git log --follow dates) and the current source tree before writing: - Implemented: jacobian-consistent-tangent (PR #258, c63cd70), fmg-checkpoint-hierarchy (3cd73cd), petsc-dmplex-checkpoint-reload-plan (PR #146, write_timestep(petsc_reload=True) in tree), fault-refinement-simplification (smooth_mesh_interior / metric_density_from_gradient / fault_comb_metric all in tree), MATHEMATICAL_MIXIN_DESIGN. - Current reference/contract: mesh-adaptation-formulation, ND_UNITS_BOUNDARY_CONTRACT (PR #278, e0ece9a). - Investigation records (preserved via PR #245, 34a9dd4; production geometric-MG is custom prolongation, PR #290): snesfas-feasibility, snesfas-vanka-feasibility-study. - Design notes / prototypes with honest gaps: in_memory_checkpoint_design (not implemented, per its own trailing Status section), submesh-solver-architecture (extract_region/extract_surface exist; coarsened_companion does not). - Historical: ARCHITECTURE_ANALYSIS (persistence.py layout superseded), COORDINATE_MIGRATION_GUIDE (transition shipped), WHY_UNITS_NOT_DIMENSIONALITY (decision record). The audit's ~16 estimate over-counted: re-derived at this tip, 13 docs were unmarked plus one marked-but-stale (DOC-07). Findings: DOC-07 (docs/reviews/2026-07/DOCS-STANDARDS-COHERENCE.md). Underworld development team with AI support from Claude Code * docs(WE-08): convert units.py public docstrings Google -> NumPy style Docstring-only conversion of the 18 public module-level functions that carried Google-style Args:/Returns:/Raises:/Examples: labels (check_units_consistency, get_dimensionality, get_units, non_dimensionalise, show_nondimensional_form, simplify_units, create_quantity, convert_units, to_base_units, to_reduced_units, to_compact, get_scaling_coefficients, set_scaling_coefficients, validate_expression_units, assert_dimensionality, validate_coordinates_dimensionality, enforce_units_consistency, require_units_if_active, convert_angle_to_degrees) to the NumPy/Sphinx standard (Style Charter section 6). dimensionalise was already NumPy style; one-line docstrings and private helpers are untouched. No code, signature, or behaviour changes (verified: every diff hunk is inside a docstring; ast.parse clean). Finding: API-12 (docs/reviews/2026-07/API-CONSISTENCY-REVIEW.md). Underworld development team with AI support from Claude Code * docs(WE-09): sweep call sites of the newer BC methods to value-first (conds, boundary, ...) order Wave C (#334) made the ORIGINAL value-first order canonical for add_nitsche_bc / add_rotated_freeslip_bc / add_constraint_bc (maintainer decisions D2/D3; Style Charter section 6) with deprecation shims for the legacy boundary-first and g= spellings. This sweep updates every call site of those THREE methods to the canonical order so nothing in the repository exercises the shims — 74 sites total: - tests/: 63 call sites across 12 files (test_1017, test_1018, test_1060, test_1061, test_1062, test_1064, test_1065 x2 serial; parallel test_1017, test_1062, test_1063, test_1064). tests/test_0641_wave_c_api_shims.py is deliberately untouched — its legacy-order calls ARE the deprecation contract. - docs/: 7 sites (curved-boundary-conditions.md x4, CONSTRAINED_FREESLIP_MULTIPLIER.md call + signature line, examples/submesh_investigation/test_region_ds_nitsche.py). - .claude/skills/: 3 sites (adapt-on-top-faults x2, free-surface-convection x1). - CLAUDE.md: 1 signature reference (free-slip BC preference section). The ~1,370 legacy-trio (add_dirichlet_bc/add_natural_bc/add_essential_bc) sites already conform and are untouched per the D2 decision. The audit review documents under docs/reviews/2026-07/ record the pre-decision state as evidence and are not swept. Discovered while verifying the swept tests run warning-free: the Wave C zero-datum guard in add_rotated_freeslip_bc rejects FLOAT zero (sympy.sympify(0.0) != 0 is structurally True), so the canonical add_rotated_freeslip_bc(0.0, boundary) raises NotImplementedError while conds=0 works. Filed as issue #336 with a TODO(BUG) marker at the guard (comment-only src touch); the swept call sites use the working integer form add_rotated_freeslip_bc(0, boundary). No fix applied here (Charter section 9 scope discipline). Findings: API-01/API-02 sweep (WE-09, REMEDIATION-WORKLIST.md). Underworld development team with AI support from Claude Code
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Track-0 remediation (2026-07 quality audit,
docs/reviews/2026-07/REMEDIATION-WORKLIST.md):BF-02 (SWARM-01/02/17), BF-05 (SWARM-04), BF-06 (SWARM-07), BF-07 (SWARM-03),
BF-08 (LE-03 = SWARM-05, issue #215 Bug 3), plus the freshly diagnosed #286.
Closes #289
Closes #286
Defects and fixes
BF-02 — stale caches after no-move migrate / particle addition / populate (SWARM-01/02/17)
Swarm.migrate()early-returned when no particle was globally unclaimed withoutinvalidating the canonical
.datacaches, the particle kd-tree, or the proxystaleness flags. Reproduced (serial): after
add_particles_with_global_coordinatesthe DMSwarm held 974 particles while every cached
.datastill had 972 rows; afteran in-place coordinate mirror +
migrate(),rbf_interpolatereturned garbage fromthe poisoned no-copy kd-tree; after
populate()a pre-existing cache stayed at 0rows. This is also the root cause of issue #289:
swarm.advection()ends with ano-move
migrate()in serial, so_proxy_stalestayedFalseand the materialproxy froze at the launch positions.
Fix: invalidate before the early return; explicit invalidation in
add_particles_with_global_coordinates(mirroring the #216 fix inadd_particles_with_coordinates) and inpopulate()(SWARM-17 verified atruntime, folded in).
Collateral defect exposed by this fix (same #216 stale-cache class):
Swarm.apply_snapshot_payload()wrote restored variable data into a detachednp.asarrayview of the canonical cache — the DMSwarm field never received therestored values, and the restore only "worked" because the SWARM-01 bug preserved
the stale canonical copy. With the invalidation fixed,
test_0007_snapshot_inmemorycontinuation tests caught it (restored material became zeros after the first
advection). Fixed by writing through the canonical array so the PETSc pack fires.
#286 — AMR: kd-tree rebuilt against stale
_nav_coordsIn
nuke_coords_and_rebuild()the point-location kd-tree was rebuilt beforeself._nav_coordswas refreshed, so the rebuild indexed an old-sized coordinatearray with new-mesh point ranges:
IndexError: index 91 is out of bounds for axis 0 with size 80(reproduced viatests/test_0810_amr_swarm_migration_regression.py).Fix: refresh
_nav_coords(volume and manifold branches) before_build_kd_tree_index().BF-05 — writes inside
migration_disabled()silently discarded (SWARM-04)Both PETSc-sync callbacks early-returned while
_migration_disabledwas set andnothing ever re-packed;
migrate()then read the stale PETSc coordinates and itstrailing invalidation destroyed the only copy of the writes. Reproduced (serial):
after writing 42.0 inside the context, the PETSc field still held 0.0.
Fix: separate "suppress migration" from "suppress PETSc sync" — writes are recorded
and flushed to the DMSwarm by
Swarm._flush_pending_petsc_sync()at context exit(and defensively at
migrate()entry); only the migration itself is deferred(default) or skipped (
disable=True). The vacuous assertions intests/parallel/test_0755_swarm_global_stats.py(the perturbation never reached theDMSwarm) now assert that it does.
BF-06 — empty/starved ranks: silent zeros or hard crash (SWARM-07)
rbf_interpolatesilently returned zeros for ranks with <= 1 particles and_rbf_to_meshVarwrote them into the proxy;IndexSwarmVariable's proxy updatereached unguarded
KDTreeconstruction on a 0-particle rank —IndexError: Out of bounds on buffer access(reproduced) inside a collectiveupdate, i.e. an MPI abort/hang.
Fix: starved ranks leave their proxy nodal values untouched and warn (never silent
zeros); guard the KDTree/nearest-neighbour machinery on
local_size <= 1whilekeeping every rank inside the same sequence of collective reads/writes/contexts
(MeshVariable syncs are collective — a naive early-return would deadlock).
IndexSwarmVariablegains a proper_update_proxy_if_stale()override (itsproxies live in
_meshLevelSetVars, so the base implementation was a no-op for it).The
update_type=0level-set projection was also restructured to compute into alocal buffer and issue exactly ONE MeshVariable write per level set per rank
(serially bit-identical): the previous formulation issued a data-dependent number
of writes whose deferred collective syncs mismatched across ranks — the np4
starved-rank test deadlocked on it (each write's ghost sync is collective).
BF-07 — modern coordinate writes never migrate (SWARM-03)
The class docstring promised automatic migration, but
swarm.coords/swarm._particle_coordinates.datawrites only packed to PETSc; only thedeprecated
pointspath migrated. Fix (maintainer-approved shape): coordinatewrites mark
swarm._needs_migration; the collectivemigrate()runs DEFERRED atthe next collective point — migration-control context exit or solve entry — never
per-write (uneven writes would deadlock). The flag is combined across ranks with a
MAX reduction before acting, so uneven writes are safe. Docstrings updated to state
the actual contract.
BF-08 — stale proxy consumed by solvers (LE-03 = SWARM-05, issue #215 Bug 3)
Proxy refresh only fired via the lazy
.symaccessor, but solvers pull the proxyDM directly through
mesh.update_lvec(). Reproduced (serial): writematerial.data, re-solve a capturedProjection— the solve consumed the oldvalues. Fix (maintainer-approved shape): a single eager, collective refresh at
solve entry —
Swarm._sync_before_assembly()invoked fromMesh.update_lvec()before its staleness check (the refresh itself is what sets
_stale_lvec). Italso performs BF-07's deferred migration. Flag-guarded: repeated calls are no-ops,
so nothing is re-interpolated per access (the
test_0006memory-leak constraint).The
TODO(BUG)at the oldswarm.py:1075is resolved and removed.Together with BF-02 this closes #289 (verified with the issue reproducer: the
proxy centroid now tracks the advecting particle centroid).
Tests
tests/test_0113_swarm_stale_cache_regression.py(new, level_1/tier_a):SWARM-01 (both
migrate=variants), SWARM-17, SWARM-02 (kd-tree invalidationcontract + behavioral mirror probe), the swarm.advection() leaves proxy mesh variables stale (frozen material) — auto-update hook orphaned in deprecated access path #289 reproducer as a regression test,
and the [BUG] - Three stale-cache issues after swarm particle addition #215-Bug-3 solve-freshness test.
tests/parallel/test_0756_swarm_migration_semantics.py(new): np2migration-suppressed write survival, np2 deferred migration at context exit and
at solve entry, np4 starved-rank proxy test (all particles on one rank).
tests/parallel/test_0755_swarm_global_stats.py: repaired the vacuousperturbation block.
tests/test_0810_amr_swarm_migration_regression.py(AMR: swarm migration after mesh.adapt() raises IndexError -- k-d tree rebuilt before nav_coords refresh #286) now passes.Gate results
All runs from the fix worktree, env
amr-dev(AMR-enabled PETSc build, neededfor the #286 test), after
./uw build.level_1 and tier_a: BEFORE = 295 passed, 6 failed -- the 6 failuresare exactly the new regression tests, each demonstrating its bug on the pristine
baseline. AFTER = 301 passed, 0 failed (1 skipped, 4 xfailed, 1 xpassed --
unchanged from baseline).
mpirun -np 2,test_0755+test_0756+test_0760+test_0765):19 passed, 1 skipped (the np4-only test).
mpirun -np 4,test_0755+test_0756+test_0765): 17 passed.test_0006_memory_leak: passed (the BF-08 gate -- the solve-entry refresh isflag-guarded, nothing re-interpolates per access).
test_0810_amr_swarm_migration_regression(AMR: swarm migration after mesh.adapt() raises IndexError -- k-d tree rebuilt before nav_coords refresh #286): failed with the reportedIndexError before the fix; passes after.
0.1071 -> 0.1270 (was frozen at 0.1071); also enshrined as a regression test.
Pre-existing np4 defect found while gating (NOT addressed here):
tests/parallel/test_0760_swarm_cache_migration.pyhangs atmpirun -np 4on thepristine baseline (verified by stashing this branch's src changes and
rebuilding): with rank-biased coordinates,
global_evaluateleaves some ranks withzero interior points, and those ranks diverge inside the
DMInterpolationmachinery in
petsc_interpolatewhile the others block in its collectives. Itpasses at np2 (CI runs
--p 2). Per-rank stack traces confirm all of this branch'scollective hooks enter and exit symmetrically before the hang. This should be
tracked as its own issue (parallel-evaluation empty-rank asymmetry, related to the
audit's SWARM-15/16 provisional findings).
Underworld development team with AI support from Claude Code