Repository navigation
Conversation
…g boxes (#764) On more than one process, draw_markers MPI-sorted the markers only if sorting boxes were set up. Without boxes, the markers drawn on the whole domain stayed on the wrong process until the first push, and accumulations before it (e.g. the initial Poisson solve of VlasovAmpereOneSpecies) wrote outside the local stencil arrays (segfault on 2 ranks). Now draw_markers always MPI-sorts on several processes; sorting into boxes is unchanged. On one process the behaviour is unchanged. New MPI test test_draw_without_boxes_on_rank_domain (pseudo_random, sobol_standard, sobol_antithetic; sort=True/False): fails on 2 ranks without the fix. Closes #764. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
spossann
added a commit
that referenced
this pull request
Oct 13, 2026
The end-to-end tests run on 2 ranks without sorting boxes and need the markers MPI-sorted after drawing (#764). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
max-models
added a commit
that referenced
this pull request
Oct 13, 2026
#734) **Solves the following issue(s):** Part of #689, tracked in #650. This PR covers the items of #689 that are left for `VlasovAmpereOneSpecies`: running on 1 and 2 ranks, no host/device transfers in the time loop, and the diagnostics it needs. It does not close #689, because these runs use the CPU emulation; the first run on a real GPU is #687. **Stack:** this is **gpu-1** of a new stack. Merge it after #722 (`cuda-namespaces`), #716, #717, #719 and #765 (MPI sort of the markers after drawing, needed for the 2-rank runs). It merges those branches in, so until they are merged, this diff also shows their commits. **gpu-2** (the benchmark script) is based on this branch. **Core changes:** - **New test `models/tests/test_gpu_VlasovAmpereOneSpecies.py`.** A small Landau-damping run on NumPy and on CuPy: 8 cells, 320 Sobol markers, 3 steps, full-f weights, and an `e1_v1` binning plot so that the diagnostics run on the device too. It compares the energies, the `e_field` and `phi` coefficients, and the markers by ID (tolerances explained in `compare_runs`), and checks the transfer budget of the time loop. - **Shared helpers in `models/tests/gpu_e2e.py`.** The transfer budget, the instrumentation, gathering markers across ranks and the device context, moved out of #719's test. `test_gpu_LinearVlasovAmpereOneSpecies.py` now uses them. - **The GPU tests also run without a GPU.** On cunumpy's fake CuPy (`CUNUMPY_FAKE_CUPY=1`), every CUDA launch is emulated on the CPU and `CudaKernel.compile` is skipped. `Simulation.compile_cuda_kernels` still checks that every kernel of the time loop has a CUDA version. Without a GPU and without the fake CuPy, `test_fake_cupy_matches_numpy` runs the comparison in a serial child process. - **Transfer budget of `model.integrate`.** Only scalar downloads are allowed: the residual norm of each Krylov iteration, which feectools copies for its convergence test (feectools#98 keeps everything else of the solve on the device). No arrays, no host-to-device copies, no host kernels. - **Bug fix (every backend): `VlasovAmpereCoupling` updates the ghost regions of the mid-point field before the push.** The pusher kernel reads `_e_sum._data` including its ghost regions, and `_e_sum` is built with `+=` from the solver output, which does not sync them. Stale ghost regions would give a wrong field near process (and periodic) boundaries. - **`Particles`.** Checking whether MPI is CUDA-aware now happens when the marker array is allocated (in the setup), not on the first marker exchange. Before, that check was the only array transfer inside the time loop on 2 ranks. **Results** (fake CuPy, every CUDA launch emulated on the CPU; macOS; **not run on a GPU**): | test | 1 rank | 2 ranks | |---|---|---| | `test_gpu_VlasovAmpereOneSpecies.py` (NumPy reference + CuPy vs NumPy + transfer budget) | passed | passed | | `test_gpu_LinearVlasovAmpereOneSpecies.py::test_cupy_matches_numpy_without_transfers` | passed | not run | Transfers in the time loop of the CuPy run: - `integrate`: one 8-byte residual norm per pcg iteration. - Diagnostics: one 24-byte copy of the three scalars per update. - Output: one copy per saved dataset. **Diagnostics:** no new CUDA kernels are needed for this model. The scalars use `xp` reductions and feectools' device inner products, and binning uses `xp.histogramdd`. **Found along the way (not changed here):** now #766 (Poisson stabilization) and #767 (energy diagnostic with the control variate). - The initial Poisson solve with the default `stab_eps=1e-14` puts (Monte-Carlo net charge) / 1e-14 into the constant mode of `phi`. That makes `phi` about 1e14, so E = -grad(phi) loses about 5% to cancellation on either backend. The verification tests use this default. The new test uses `stab_eps=1e-6`, as #719 does. - With `control_variate=True`, `kinetic_energy` at t=0 is computed with the full-f weights and afterwards with the delta-f weights, so `total_energy` jumps after the first step. With full-f weights the total is conserved from step to step to 1e-9, but the t=0 value still differs by about 2e-4 (not investigated). **Review fixes** (pushed by @spossann, see the review): - the e2e tests run in CI: unit shard-2 (serial and 2 ranks) and on the fake CuPy on 1 and 2 ranks (`test-PR-unit-mpi.yml`); merged #765, without which they segfault on 2 ranks (#764); - `MassMatrixPreconditioner._core_diagonal` returns a diagonal operator again (with the wrapped band entries); - `test_preconditioner_cupy.py` uses cunumpy's `emulated_launches` and `run_fake_cupy_child`, and its GPU test emulates its launches on the fake CuPy; - `FFTSolver.solve` asserts host input. **Documentation changes:** `CUDA_STRATEGY.md`: the end-to-end entry now covers both models, the CPU emulation and the transfer budget. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: Stefan Possanner <stefan.possanner@ipp.mpg.de>
Member
Author
|
Has been solved in another PR of stack #762 |
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
1 participant
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.
Solves the following issue(s):
Closes #764. Needed by #734, whose end-to-end GPU tests run without sorting boxes and crash on 2 ranks without this fix.
Core changes:
Particles.draw_markersalways MPI-sorts the markers on more than one process. Before, it calledmpi_sort_markers()only when sorting boxes were set up. Without boxes, markers drawn on the whole domain (sobol_standard,sobol_antithetic,pseudo_randomwithspatial="disc",external,restart) stayed on the wrong process until the first push. Every setup step before that saw them: the initial Poisson accumulation ofVlasovAmpereOneSpeciesandLinearVlasovAmpereOneSpecieswrote outside the local stencil arrays (segfault on 2 ranks), andParticles5Devaluated the magnetic moment at off-process markers. Sorting into boxes (sort,do_sort) is unchanged, and on one process nothing changes.test_draw_without_boxes_on_rank_domain(pic/tests/test_draw_parallel.py, markedmpi_pic): checks that, without sorting boxes, every marker is on its process right after drawing, forpseudo_random,sobol_standardandsobol_antithetic, withsort=Trueandsort=False. Without the fix it fails on 2 ranks (250 markers off-process per rank).Behaviour change: a strong load imbalance after drawing (e.g.
spatial="disc", or an unevenexternalload) now aborts at draw time (mpi_comm.Abort()in the marker exchange) instead of at the first push.Tests run locally (Linux, Open MPI 4.1.6):
test_draw_parallel.pyandtest_sorting.pypass on 1, 2, 3 and 4 ranks (69 passed each);-m mpi_pic pic/testson 2 ranks: 41 passed;test_sph.pyon 2 ranks: 449 passed;test_tesselation.py,test_sorting.pyandtest_load_external.pyon 2 ranks: 270 passed;VlasovAmpereOneSpeciesandLinearVlasovAmpereOneSpecieson 2 ranks: 3 passed;Documentation changes:
The
draw_markersdocstring describes when the markers are sorted.🤖 Generated with Claude Code