fix: swarm empty-rank save deadlock (MPIO dtype) and estimate_dt reshape crash - #680
Merged
Merged
Conversation
On an MPI rank holding zero particles (e.g. crust-only tracers confined to a subset of ranks), the velocity array entering the estimate_dt() max-speed reduction is empty; reshape(n, -1) then raises 'ValueError: cannot reshape array of size 0 into shape (0,newaxis)' because NumPy cannot infer the implied dimension from zero elements. Guard the empty case explicitly so the rank contributes zero to the global max and estimate_dt() returns cleanly. The empty-rank collective point-location deadlock in advection's global_evaluate is fixed upstream (issue #611 / PR #656) and covered by tests/parallel/test_1076_global_evaluate_empty_rank.py, so no local workaround is needed here. Regression: test_0796 exercises the DEFAULT (non-evalf) advection path on empty ranks, which also runs estimate_dt via order=2. test_0795 covers evaluate + write_timestep on empty ranks. Underworld development team with AI support from Claude Code
An int swarm variable (e.g. add_variable('uid', size=1, dtype=int)) holds
particles on some ranks but not others. On an empty rank (local_size == 0),
SwarmVariable.unpack_raw_data_from_petsc() returned np.zeros((0, n)) which is
float64, so an empty rank's data array had dtype float64 while a non-empty
rank's was the field's PETSc type (int32). In parallel HDF5 every rank must
create an identical dataset, but SwarmVariable.save() passed the per-rank
local_data.dtype to the collective create_dataset(); the divergent dtype left
the collective close(s) unsynchronised and the save deadlocked — the same
silent-hang class as issue #151.
Fix: store the field's PETSc dtype (_petsc_dtype) and use it for every
zero-length fallback array, so empty-rank data agrees with the field's true
type across all ranks. Regression covered by
tests/parallel/test_0795_swarm_empty_rank_evaluate_save.py::test_passive_swarm_save_empty_ranks,
which hung at np=4 before this change and now passes.
Underworld development team with AI support from Claude Code
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The new advection regression test can be flaky/incorrect due to add_particles_with_coordinates() migrating particles across ranks, and the new tests introduce disallowed data-access patterns (mesh.access/.data) per the style charter.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Fixes two empty-MPI-rank failure modes in swarm workflows (advection timestep estimation and parallel-HDF5 saving), and adds MPI regression tests to prevent deadlocks/crashes when some ranks hold zero particles.
Changes:
- Preserve PETSc field dtype on empty ranks for
SwarmVariableunpack/initialisation, preventing dtype divergence in collective parallel-HDF5 writes. - Guard
Swarm.estimate_dt()against zero-length velocity arrays on empty ranks. - Add MPI regression tests covering evaluate/save and advection with empty ranks.
File summaries
| File | Description |
|---|---|
src/underworld3/swarm.py |
Stores PETSc dtype on swarm variables for consistent empty-rank fallbacks; adds empty-rank handling in estimate_dt(). |
tests/parallel/test_0795_swarm_empty_rank_evaluate_save.py |
New MPI regression coverage for evaluate() + write_timestep() under empty-rank distributions. |
tests/parallel/test_0796_swarm_advection_empty_rank.py |
New MPI regression coverage for swarm.advection(..., order=2) exercising global_evaluate and estimate_dt() on empty ranks. |
Review details
Suppressed comments (2)
tests/parallel/test_0796_swarm_advection_empty_rank.py:47
- New code should not use
with mesh.access(...)(style charter forbids it); use the variable's.arrayproperty directly for data assignment.
v = uw.discretisation.MeshVariable("v", mesh, mesh.dim, degree=1)
with mesh.access(v):
v.data[:, 0] = mesh.X.coords[:, 1]
v.data[:, 1] = 0.0
tests/parallel/test_0795_swarm_empty_rank_evaluate_save.py:348
- New code should not use
with mesh.access(...)or the flat.datacompatibility layer (see the style charter’s Data Access rules). Use the MeshVariable.arrayproperty directly for assignment, and read coordinates frommesh.X.coords.
with mesh.access(T_mesh):
T_mesh.data[:, 0] = 300.0 + 100.0 * T_mesh.coords[:, 0] * T_mesh.coords[:, 1]
- Files reviewed: 3/3 changed files
- Comments generated: 4
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
5092
to
+5101
| if vel.ndim == 3: | ||
| vel = vel.reshape(vel.shape[0], -1) | ||
| # Guard against empty ranks: an array of size 0 cannot be | ||
| # reshaped with a `-1` axis (NumPy cannot infer the implied | ||
| # dimension from zero elements) — e.g. (0, 1, dim) -> (0, -1) | ||
| # raises ValueError. A zero-particle rank legitimately has no | ||
| # velocities and contributes 0 to the global max below. | ||
| if vel.size == 0: | ||
| vel = np.zeros((0, vel.shape[2]) if vel.ndim >= 3 else (0,)) | ||
| else: | ||
| vel = vel.reshape(vel.shape[0], -1) |
Comment on lines
+51
to
+63
| # Particles only on rank 0 (collective call) | ||
| if uw.mpi.rank == 0: | ||
| coords = (np.random.rand(100, mesh.dim) * 0.8 + 0.1) | ||
| else: | ||
| coords = np.empty((0, mesh.dim)) | ||
| swarm.add_particles_with_coordinates(coords) | ||
|
|
||
| uw.mpi.comm.barrier() | ||
|
|
||
| # Sanity: the empty-rank precondition must actually hold (all 100 points | ||
| # are added on rank 0 and no migration happens here, so the other ranks | ||
| # hold zero particles). | ||
| sizes = uw.mpi.comm.allgather(swarm.local_size) |
| import os | ||
| import numpy as np | ||
| import pytest | ||
| import sympy as sp |
| ] | ||
|
|
||
|
|
||
| def test_advection_empty_rank_default(tmp_path_factory): |
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.
Summary
Two fixes making passive-swarm operations safe when one or more MPI ranks hold zero particles.
estimate_dt()empty-rank guard — zero-particle velocity data is reshaped to an explicit(0, dim)array instead of crashing onreshape(n, -1)withValueError: cannot reshape array of size 0 into shape (0,newaxis).Empty-rank field-dtype preservation — fixes a parallel-HDF5 (MPIO) collective-close deadlock in
SwarmVariable.save/write_timestep. Anintswarm variable (e.g.add_variable("uid", dtype=int)) heldint32on a non-empty rank butfloat64on an empty rank (the empty-rankunpack_raw_data_from_petscfallback returned a bare float64np.zeros((0, n))).save()passed the per-rank dtype to the collectivecreate_dataset, so HDF5 metadata diverged and the collectiveclosedeadlocked. The field PETSc dtype is now stored (_petsc_dtype) and used for every zero-length fallback so dtypes agree across ranks.Intended outcome
write_timestep/ swarmsaveno longer hang with empty ranks (was reproducible 6/6 at np=4, now clean 6/6).estimate_dt()no longer crashes on empty ranks.The
evaluate/global_evaluate/advectionempty-rank deadlock was already fixed upstream by #611 / PR #656; coverage here guards that against regression.Tests
New regression tests (both run with
--with-mpi):tests/parallel/test_0795_swarm_empty_rank_evaluate_save.py— evaluate +write_timestepon empty ranks.test_passive_swarm_save_empty_ranksreproduces the user's crust-only-tracer pattern and hung at np=4 before this change, now passes.tests/parallel/test_0796_swarm_advection_empty_rank.py— advection on the default non-evalfpath with empty ranks.Result: 5 passed at np=2 and 5 passed at np=4 (4 from test_0795 + 1 from test_0796).
Underworld development team with AI support from Claude Code