Fix swarm cache invalidation after parallel migration - #65
Conversation
…eletion SwarmVariable._canonical_data caches were only invalidated inside the delete_lost_points branch of Swarm.migrate(). When particles moved between ranks without deletion (e.g. balanced swap with same count), caches became stale — storing the wrong particle count and values. This caused shape mismatches in global_evaluate when used by NavierStokesSLCN/DDt solvers in parallel. Two fixes: - swarm.py: move cache invalidation out of delete_lost_points block so it runs unconditionally after every migration - _function.pyx: add cache invalidation after the bare-bones dm.migrate() in global_evaluate_nd (which bypasses Swarm.migrate entirely) Adds parallel regression test (test_0760_swarm_cache_migration.py). Fixes #64 Underworld development team with AI support from Claude Code
There was a problem hiding this comment.
Pull request overview
Fixes stale SwarmVariable._canonical_data (and coordinate) caches after particle migration in parallel runs, which could otherwise lead to shape mismatches during global_evaluate / SemiLagrangian-style workflows (Issue #64).
Changes:
- Invalidate swarm coordinate + variable caches after
Swarm.migrate()regardless of whether lost points are deleted. - Invalidate the same caches in
global_evaluate_ndafter calling baredm.migrate()(which bypassesSwarm.migrate()). - Add a new parallel regression test module intended to cover direct swarm migration and
global_evaluate-driven migration paths.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
src/underworld3/swarm.py |
Moves cache invalidation to run after migration generally (not only in the delete-lost-points branch). |
src/underworld3/function/_function.pyx |
Clears swarm/variable caches after bare dm.migrate() in global_evaluate_nd. |
tests/parallel/test_0760_swarm_cache_migration.py |
Adds regression tests for cache invalidation across migration paths (needs adjustments for coverage + CI execution). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| import pytest | ||
| import numpy as np | ||
| import underworld3 as uw | ||
| from mpi4py import MPI |
There was a problem hiding this comment.
from mpi4py import MPI is unused in this test module. Consider removing it to keep the test file minimal (other parallel tests import MPI only when they need MPI.COMM_WORLD etc.).
| from mpi4py import MPI |
| pre_count = swarm.dm.getLocalSize() | ||
|
|
There was a problem hiding this comment.
pre_count is assigned but never used. If it’s not needed for an assertion/debug message, remove it to avoid dead code in the regression test.
| pre_count = swarm.dm.getLocalSize() |
| # Clamp to domain | ||
| mid_pt_coords = np.clip(mid_pt_coords, 0.0, 1.0) | ||
|
|
||
| result = uw.function.evaluate(v.sym, mid_pt_coords) |
There was a problem hiding this comment.
This test claims to exercise the “DDt/SemiLagrangian” displaced-node global evaluation path, but it calls uw.function.evaluate(...) (local evaluation) rather than uw.function.global_evaluate(...). To cover the regression described in #64 (which goes through global_evaluate_nd and swarm migration), this should call global_evaluate (or update the test name/docstring if local evaluate is intentional).
| result = uw.function.evaluate(v.sym, mid_pt_coords) | |
| result = uw.function.global_evaluate(v.sym, mid_pt_coords) |
| Run with: | ||
| mpirun -n 2 python -m pytest --with-mpi tests/parallel/test_0760_swarm_cache_migration.py | ||
| """ |
There was a problem hiding this comment.
This test module won’t be picked up by the repo’s current parallel test runner: scripts/test.sh only executes tests/parallel/test_075*py, so test_0760_* won’t run in CI by default. Consider either updating the parallel test glob in scripts/test.sh to include this file (or test_076*), or renaming the test file to match the existing test_075* pattern so the regression is continuously exercised.
| # Add particles at random positions — distribution will be uneven across ranks | ||
| np.random.seed(42 + uw.mpi.rank) | ||
| coords = np.random.random((200, mesh.dim)) | ||
| swarm.add_particles_with_global_coordinates(coords, migrate=False) | ||
| var.data[...] = uw.mpi.rank | ||
|
|
||
| pre_count = swarm.dm.getLocalSize() | ||
|
|
||
| # Migrate — particles move to owning rank | ||
| swarm.migrate(remove_sent_points=True, delete_lost_points=False) | ||
|
|
||
| post_count = swarm.dm.getLocalSize() | ||
| coords_cached = swarm._particle_coordinates.data.shape[0] | ||
| var_cached = var.data.shape[0] |
There was a problem hiding this comment.
coords_cached = swarm._particle_coordinates.data.shape[0] doesn’t currently validate cache invalidation, because _particle_coordinates.data is never accessed before swarm.migrate(). To actually regress the stale-cache bug, force creation of the coordinate cache before migration (e.g., access _particle_coordinates.data once), then assert the post-migration shape matches dm.getLocalSize().
Summary
_canonical_datacaches were only invalidated inside thedelete_lost_pointsbranch ofSwarm.migrate(), causing stale caches when particles moved between ranks without deletionglobal_evaluate_ndwhich uses bare-bonesdm.migrate()bypassingSwarm.migrate()entirelytest_0760_swarm_cache_migration.py) with 3 tests covering direct swarm migration,global_evaluatewith biased coordinates, and displaced-node evaluation (the DDt/SemiLagrangian path)Test plan
global_evaluatestress test passes with biased coordinatesmpirun -n 2 pytest --with-mpi tests/parallel/test_0760_swarm_cache_migration.pyFixes #64
Underworld development team with AI support from Claude Code