Repository navigation
Set up sorting boxes on the CuPy backend - #736
Merged
Merged
Conversation
SortingBoxes could not be created on CuPy: n_cols used xp.sqrt (a 0-d device array that round() rejects) and the pyccel initialize_neighbours was called with a device array. - SortingBoxes._set_boxes: n_cols with math.sqrt; neighbours built on the host with initialize_neighbours and moved to the backend once. - Particles: n_boxes with math.prod; is_domain_boundary and _get_neighbouring_proc (SPH) read a host copy of domain_array. - put_particles_in_boxes / do_sort: the pyccel box-sorting kernels are wrapped in PyccelKernel, an explicit host fallback on CuPy (device -> host -> device per call) until #694 ports them. - Test: Particles6D and ParticlesSPH with boxes_per_dim on CuPy (real or fake CuPy in a serial subprocess on rank 0) match NumPy, including one do_sort. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
spossann
requested changes
Oct 13, 2026
… trip per sort - Box sorting goes through cunumpy Kernel dispatch: four entry kernels in pic/kernels/<name>/ (box_particles, fill_boxes, sort_and_fill_boxes, box_and_sort_particles), declared with Kernel.from_folder and missing_cuda="fallback", so #694 only has to add <name>_cuda.cu. - do_sort (no SPH ghosts) runs assign-box, the per-box count, the sort and the box refill in one kernel: on CuPy the markers cross once in each direction. The count is done on the host (no device bincount, no implicit sync). - The swap lines are scratch buffers, no longer copied back to the device. - boxes is rebuilt after the sort, so it points to the sorted rows. - The box column is taken from SortingBoxes.box_index everywhere. - One host copy of domain_array (_domain_array_host), used for n_cells, the per-rank cell counts, the boundary flags, the neighbouring procs, the neighbour ranks and the box kernels. - Drop the misleading host-bookkeeping comment in SortingBoxes._set_boxes. - Tests: run_fake_cupy_child, mpi_skip for the GPU test, check that boxes matches the markers after do_sort (both sort paths), count the marker transfers, and a multi-rank check on the fake CuPy in test_sorting_device.py. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The CI container keeps the compiled sorting_kernels from devel, and `struphy compile` imports struphy before it rebuilds the kernels. Importing count_particles_in_boxes/find_holes by name then failed with an ImportError in every job. With `import struphy.pic.sorting_kernels as sorting_kernels` (as the SPH and pusher kernels do), the names are only looked up when a kernel runs, which is after the rebuild. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- test_neighbor_ranks: the Particles stubs set _domain_array_host, which _compute_neighbor_ranks and _get_neighbouring_proc now read. - test_kernel_backends: the catalog count includes the 4 box kernels of struphy.pic.kernels. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
spossann
approved these changes
Oct 15, 2026
This was referenced Oct 16, 2026
max-models
added a commit
that referenced
this pull request
Oct 16, 2026
… (#774) **Solves the following issue(s):** Part 1 of #707: the marker buffer is stored as struct-of-arrays (one contiguous block per quantity) on both backends. Part 2 of the issue (tracking the range of marker slots that can hold valid markers, so that launches skip trailing holes) is #775, stacked on this PR. **Core changes:** `Particles.markers` is now a plain C-ordered array of shape `(n_cols, n_markers_max)`: **one column per marker**, indexed `markers[j, ip]` (was `(n_rows, n_cols)`, `markers[ip, j]`). Every quantity `markers[j]` is one contiguous block of `n_markers_max` values. **Naming:** `n_cols` keeps its meaning, the number of quantities per marker. The number of marker slots (holes included) no longer counts rows, so it is renamed after what it counts: `Particles.n_rows` → `n_markers_max`, `padded_n_rows` → `padded_n_markers`, `ROW_ALIGNMENT` → `MARKER_ALIGNMENT`. The quantity indices are still called columns (`first_pusher_idx` etc.). An earlier version of this PR kept the shape and allocated the buffer with `order="F"`. That gives the same bytes in memory, but every array passed to a kernel then had to carry the order (pyccel `float[:, :](order=F)`, conversion helpers at every entry point, checks in the CUDA arguments). The buffer is now transposed and the indexing changed instead, so no array needs an order flag. - **`struphy.pic.marker_layout`:** - `marker_array(n_cols, n_markers_max)`, plus `padded_n_markers`/`MARKER_ALIGNMENT`: `n_markers_max` is padded to a multiple of 32, so every row of the buffer starts on a 256-byte boundary (point 3 of the issue's second comment). - The module docstring documents the convention and its one pitfall. Selecting markers (`markers[:, valid_mks]`) gives a Fortran-ordered copy in NumPy, so convert it with `xp.ascontiguousarray` before passing it to a kernel or to MPI. - **pyccel kernels:** - All kernels index `markers[j, ip]` and declare a plain `float[:,:]` (C order). - The marker count is `shape(markers)[1]`, and `MarkerArguments.n_markers` is `markers.shape[1]`. - The sorting kernels copy whole markers as columns. - **CUDA:** - The kernels index `markers(j, ip)`. `CudaMarkerArguments` takes the C-contiguous `(n_cols, n)` array again. - cunumpy infers the launch size from the first axis of the first array, which is now `n_cols`. So `Pusher` and the accumulators pass `n_threads=args_markers.n_markers`, and the geometry and spline marker kernels pass `markers.shape[1]`. - `prepare_push` (the fused preamble from #708) takes an `Array2D<double>` view (a raw `double*` would be made C-contiguous, i.e. copied, by the launch). It numbers its threads row by row, so neighbouring threads touch neighbouring markers. - **`Particles`:** - The properties follow the layout: `markers_wo_holes(_and_ghost)` are `(n_cols, n)`, `positions` is `(3, n)`, `velocities` is `(vdim, n)`, and `phasespace_coords`, `f_coords` and `f_jacobian_coords` are `(k, n)`. The setters check these shapes. - Holes are `markers[0] == -1`, ghosts are `markers[-1] == -2`, and whole markers are selected with `markers[:, sel]`. - The sorting etas, the bc buffer and `_is_on_domain_of_rank` work on `(3, n)`. - **MPI exchange:** - The send lists, the receive buffers and the SPH ghost-box arrays are `(n_cols, n)`. - The send lists and ghost-box markers are made C-contiguous. A column selection is Fortran-ordered, and MPI would otherwise send it marker by marker into C-ordered receive buffers. - The CuPy exchange receives into one buffer per source rank, because a slice of a common `(n_cols, n_recv)` buffer is not contiguous. - **Evaluation at markers takes `(k, n)` arrays, one column per point** (eta1..3 in rows 0..2), so the buffer and its selections go in without a transpose: - `Domain` (metric coefficients, pull/push/transform, `show`), `SplineFunction`, the GVEC equilibria, the kinetic-background moments and `B0` callables, and the canonical Maxwellian (`coords[0]` is `(5, n)`); - user callbacks that are evaluated at markers (the example params files read `etas[0][i]`). Output shapes are unchanged. `Domain` and `SplineFunction` make the input C-contiguous (no copy for the buffer). - **Gather only the quantities that are used** (point 4 of the issue's second comment): binning, the box count of the sorting boxes, the magnetic energy of `DriftKineticElectrostaticAdiabatic`, and two emptiness checks in the weight initialization. - **Box sorting from `devel` (#736):** `devel` is merged into this branch. Its box kernels (`box_particles`, `fill_boxes`, `sort_and_fill_boxes`, `box_and_sort_particles`) call the sorting kernels, and the two it adds there (`count_particles_in_boxes`, `find_holes`) now read the box row and row 0 of each marker column, like the other sorting kernels. - **HDF5 output and restart use the buffer layout:** - restart data `restart/<species>` is `(nt, n_cols, n_markers_max)`; - saved markers `kinetic/<species>/markers` are `(nt, n_cols, n_to_save)`; - external marker files have a `markers` dataset of shape `(n_cols, N)`. **Restart and external files written before this change do not load.** `_load_restart` copies the common markers when the padding differs and asserts that any extra ones in the file are holes. The post-processed orbits keep their `(n_markers, n_quantities)` layout. - **Fix found along the way:** the `info` output of `VlasovAmpereCoupling` read `particles.bufferindex`, which does not exist, so it always failed. It now reads the velocities saved by the preamble, gathering 6 rows instead of whole markers. **Tests:** - `pic/tests/test_marker_layout.py`, on NumPy and on the fake CuPy (emulated CUDA launches), on 1 and 2 ranks. It checks: - the layout helpers; - that the buffer is C-ordered `(n_cols, n_markers_max)` with `n_markers_max` a multiple of 32, and stays the same object through loading, RK4 and v×B pushes, MPI sorting, boundary conditions, evaluation at markers and binning; - that marker selections are Fortran-ordered and accepted by `MarkerArguments` and a kernel after `ascontiguousarray`; - that evaluation on selections gives the same values as on the buffer. - All tests that build markers or evaluation points by hand use the new layout. The CUDA parity cases pass `n_threads` explicitly. - The box checks added by #736 in `test_sorting.py` and `test_sorting_device.py` read the box row of the markers. - `test_mhd_equils` checked its eta1 case with `kind="markers"` left over from the previous case; fixed. - Run locally before the full suite was left to CI: the marker-layout, sorting, draw-parallel and load-external tests on 2 ranks, the emulated CUDA `prepare_push`/parity cases, and the geometry, feec, equilibrium, model-scalar, post-processing and output tests, plus `test_verif_LinearVlasovAmpereOneSpecies` on 1 and 2 ranks. The MPI tests caught the Fortran-ordered send lists described above. The SPH and pusher tests were stopped partway (no failures until then), and pyccel compilation and the GPU tests run only in CI. Nothing was run locally after the merge of `devel` (including the new box kernels); CI covers it. **Benchmark (CPU):** `pic/tests/bench_marker_layout.py` times the layout-dependent operations on the active backend. The numbers below were measured with the `order="F"` version of this PR. The buffer has the same bytes in memory now, so they are expected to carry over, but they have not been measured again. The setup is 1M markers (1.25M marker slots × 26 quantities), a Cuboid, 8³ cells and cubic splines, with the median of 5 calls on NumPy, 1 rank, no OpenMP, on an M-series MacBook Air. Before: `devel` at 11adb7b. | Operation | Before (row per marker) | After, run 1 | After, run 2 | Speedup | |---|---:|---:|---:|---:| | preamble (row assignments of `Pusher._push`) | 527 ms | 14 ms | 34 ms | 15–37× | | `update_holes` | 53 ms | 1.1 ms | 0.9 ms | 47–61× | | `apply_kinetic_bc` (periodic) | 264 ms | 70 ms | 71 ms | 3.7× | | `mpi_sort_markers` (1 rank) | 1277 ms | 507 ms | 519 ms | 2.5× | | weights of valid markers (`particles.weights`) | 134 ms | 40 ms | 43 ms | 3.1–3.3× | | binning `e1_v1` | 2728 ms | 2057 ms | 2311 ms | 1.2–1.3× | | `push_eta_stage` (RK4 pusher) | 2685 ms | 2021 ms | 1985 ms | 1.3× | | `push_vxb_analytic` (pusher) | 6213 ms | 4977 ms | 4861 ms | 1.25–1.3× | | `charge_density_0form` (accumulation) | 4073 ms | 3087 ms | 3231 ms | 1.25–1.3× | | all quantities of the valid markers (`markers_wo_holes_and_ghost`) | 225 ms | 385 ms | 392 ms | 0.6× | Caveats: - The machine was heavily loaded during all runs (load average 210–265 from other jobs), so the absolute numbers are inflated and noisy. - The operations on single quantities gain an order of magnitude or more, far beyond the noise. - The 1.25–1.35× on the kernel-bound rows (pushers, accumulation) should not be read as a kernel speedup until it is measured on a quiet node. The issue's quiet single-thread microbenchmark found kernel time unchanged within ±3%. - Gathering all quantities of the valid markers is about 1.7× slower, as expected: a marker now touches 26 separate rows. The hot paths above no longer do it. It is still used by `markers_wo_holes_and_ghost`/`markers_wo_holes` and by the saved-markers output. - With the `order="F"` version, the valid markers after all timed calls were bit-for-bit equal to `devel` (`--save-markers`, which still writes one row per marker). - **Not measured yet:** - The GPU, where most of the gain is expected (preamble, `update_holes`, `valid_mks` masks, diagnostics). It waits for the first H100 run (#687): `CUNUMPY_BACKEND=cupy python src/struphy/pic/tests/bench_marker_layout.py`. - CPU with OpenMP on a compute node. - SPH neighbour loops, whose per-marker access gets more expensive with this layout (see the issue). **Model-specific changes:** None beyond indexing: every model uses the new layout through `Particles`. User callbacks evaluated at markers (density, temperature or perturbation functions in params files that accept a single 2D array) now receive `(3, n)`; the example params files are updated. **Documentation changes:** - "Marker array layout" section in the developer docs (`dev-pic.rst`, automodule of `struphy.pic.marker_layout`). - The marker-layout question in `CUDA_STRATEGY.md` is marked as decided, with the new convention. The kernel and argument docstrings describe the layout. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
max-models
added a commit
that referenced
this pull request
Oct 16, 2026
**Stack:** part 2 of 2, based on #774 (merge that first). This PR's diff includes the commits of #774 until it is merged. #774 changed the marker buffer to one column per marker (`(n_cols, n_markers_max)`, `markers[j, ip]`), and this branch is merged with it, so "marker slot" below means a column of the buffer. Following #774, the names count markers, not rows: the bound is `n_markers_used` (was `n_rows_used`), counted by `count_markers_used` and tracked by `_track_markers`. Both branches are also merged with the current `devel`, including the kernel-based box sorting of #736. **Solves the following issue(s):** Closes #707 together with #774. This is part 2 of the issue: launches and per-quantity operations cover only the marker slots that can hold markers, not the trailing holes of the buffer. **Core changes:** The marker buffer has room for `(1 + bufsize) * Np` markers (`n_markers_max` slots), and every kernel and per-quantity operation ran over all of them. `Particles.n_markers_used` is now a bound with the invariant that **every marker slot from it on is a hole**, and the per-marker work stops there: - **Kernels:** - `args_markers.n_markers` holds the bound, so the pyccel kernels loop over it and the CUDA kernels return after it. - The 15 accumulation kernels that looped over the full marker count from the array shape now use `args_markers.n_markers`. - `Pusher` and the accumulators launch `n_markers` threads. - **Python operations on `markers[:, :n_markers_used]`:** - the pusher preamble (`prepare_push` gets the sliced `Array2D` view on the GPU); - the residual check of the iterative pushers; - `update_holes`; - the boundary conditions (`_find_outside_particles` and the snapshot of the positions); - the alpha-weighted sorting positions of `mpi_sort_markers` (the first `n_markers_used` columns of the `(3, n_markers_max)` helper arrays). **Keeping the bound without reading the device:** - New markers always fill the lowest holes (`nonzero` of the hole mask, ascending). So `k` incoming markers end below `(occupied slots) + k`, where `_n_occupied_bound` is an upper bound on the occupied slots. - The sent, received and removed counts are already host integers: `send_inds.size`, the NumPy `recv_info`, and the lost-marker counts of the boundary conditions. So `_track_markers` updates the bound on the host. On the CuPy backend the time loop gets no new device-to-host transfer, which the transfer budget of the GPU end-to-end tests checks. - **Where the bound is updated:** - **counted exactly** with `count_markers_used` (reads row 0; one device read on CuPy): after loading (fresh, external or restart; restart files can have markers in any slot) and after box sorting (`do_sort`, also with the box kernels of #736), which moves holes anywhere. Code that writes markers into holes itself, outside `Particles`, must call it too; it is public for that reason. - **tracked from host counts:** the MPI exchange, removal at the boundary, and the SPH ghost-box exchange (own copy and receives); - **unchanged on purpose:** holes created without a count (removed ghost particles) leave the bound too high, which is safe. **Fix included (also on #774):** `test_verif_LinearVlasovAmpereOneSpecies::test_delta_f_initial_condition` checked the mean of the delta-f weights per rank, assuming every rank holds a random subset of the markers. - The markers are sorted into the domain of their rank, so on four ranks each rank holds a quarter of the `cos(kx)` perturbation: mean/rms = (2/π)/(1/√2) ≈ 0.9, exactly the measured 1.25e-7/1.39e-7. - The sums are now reduced over the clone's communicator. - The test fails on four ranks without any of these changes, but testmon had not selected it on recent PRs (0 of 3184 verification tests selected on `726-sorting-boxes-cupy`). Touching `pic/base.py` in #774 selected it in the `domain cloning in Container` job. **Tests:** - `test_markers_used` (in `test_marker_layout.py`), run on NumPy and on the fake CuPy, on 1 and 2 ranks. It uses removing boundaries in eta1, pushes that move markers out, and MPI sorting. It checks: - after every step, that every marker slot after `n_markers_used` is a hole and that `args_markers.n_markers` matches; - that the final markers (by ID) and the number of lost markers are bit-for-bit those of the same run with every slot counted as used. - `bench_marker_layout.py` (from #774): before the merge with the new layout, the valid markers after all timed operations were bit-for-bit those of `devel`. With 1M markers there are 1,251,008 slots, of which `n_markers_used` = 1,000,000. - Run locally: the new test and the verification test above, and the marker-layout tests after the renaming. The merges with #774 and `devel` were resolved without running the suite locally; CI ran on the merged state, where `test_markers_used` called the wrong device-context helper after the merge (fixed). **Benchmark:** - The work of every restricted operation scales with `n_markers_used / n_markers_max`. That is 0.8 for the default `bufsize = 0.25` right after loading, and about 0.5 for `bufsize = 1` (the 8M-slot buffer for 4M markers in the issue's trace). - I could not measure it here. `bench_marker_layout.py --all-markers-used` compares the same build with and without the bound. I ran it twice, alternating, but the machine had a load average of about 500 from other jobs, and operations this PR does not touch varied by up to 15× between runs. So I don't claim a number. - It needs a quiet node or the GPU (#687): - `bench_marker_layout.py --all-markers-used --json all.json` - `bench_marker_layout.py --json used.json` - `bench_marker_layout.py --compare all.json used.json` **Model-specific changes:** None. **Documentation changes:** `Particles.n_markers_used` and the tracking (`_track_markers`, `count_markers_used`) are documented in their docstrings. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
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.
Closes #726
What changed
Particlessetup with boxes (pic/base.py):n_boxesusesmath.prodinstead ofxp.prod(xp.array(...)). On CuPy the old version gave a 0-d device array, andSortingBoxes._set_boxesfailed withtype ndarray doesn't define __round__ method.domain_arrayhas one host copy,_domain_array_host, made once in__init__. The host bookkeeping uses it:n_cells, the per-rank cell counts,is_domain_boundary(Python bools),_get_neighbouring_proc(the pycceldistanceused to get 0-d device arrays, soParticlesSPHsetup failed on CuPy) and the neighbour-rank check.SortingBoxes._set_boxes(pic/sorting.py):_neighboursis built on the host with the pyccelinitialize_neighboursand moved to the active backend once withxp.to_cunumpy.pic/kernels/<name>/, one folder per kernel as inCUDA_STRATEGY.md):box_particles,fill_boxes,sort_and_fill_boxesandbox_and_sort_particles, declared withKernel.from_folder(..., missing_cuda="fallback"). On NumPy they run the pyccel kernel. On CuPy they are an explicit host fallback (Port the diffusion and SPH kernels to CUDA #694 adds<name>_cuda.cuwithout changing the call sites). The helpers stay insorting_kernels.py.do_sort()without SPH ghosts calls onlybox_and_sort_particles: assign the boxes, count per box, sort, refillboxes. On CuPy the markers cross once in each direction. The per-box count is done on the host (no devicebincount, no implicit sync).put_particles_in_boxescallsbox_particles, then (SPH) the ghost exchange andfill_boxes.boxesis rebuilt after the sort, so it points to the sorted rows (before, it still pointed to the rows before the sort).SortingBoxes.box_indexfor the box column.Time loop of non-SPH models
For a non-SPH model (e.g.
VlasovAmpereOneSpecies), settingboxes_per_dimalone does not sort anything during the time loop. Pushers only callput_particles_in_boxeswhensorting_boxes.communicateis true, which is SPH only. Box sorting runs only:draw_markers(sort=True), whenSortingParameters(do_sort=True);sort_stepsteps insim.py, viado_sort(), whenEnvironmentOptions.sort_step > 0.Both paths now work on CuPy through the host fallback above. The fallback still has to be ported (#694).
Remaining for #694
<name>_cuda.cuin their folders).ParticlesSPHsetup works on CuPy, also on several ranks. Drawing markers with a domain needs CUDA launches, which the fake CuPy cannot run, so nothing after setup was exercised for SPH.Tests
test_sorting.py:check_sorting_boxes_on_cupy(serial): createsParticles6D(boxes_per_dim=(4, 3, 2))on NumPy and on CuPy and comparesneighbours. It then runsdo_sort()on the same markers on both backends and compares the markers and box arrays. It also checks that the markers cross host↔device exactly once in each direction (xp.profiling.count_transfers), and checks theParticlesSPHsetup. It runs astest_sorting_boxes_on_cupy(real CuPy,mpi_skip) andtest_sorting_boxes_on_cupy_fake_cupy(fake CuPy viarun_fake_cupy_child).assert_boxes_match_markers: every row ofboxeslists exactly the marker rows of that box. It is used intest_sorting, in the newtest_boxes_valid_after_do_sort(both sort paths) and in the CuPy checks.test_sorting_device.py:check_sorting_boxes_deviceruns on every rank (fake CuPy in CI on 1–4 ranks;test_sorting_boxes_gpuon a GPU). It does the samedo_sort()comparison and transfer count with a real decomposition, checks that the box kernel used this rank'sdomain_arrayrow, and compares theParticlesSPHneighbouring procs (26 directions), boundary flags and boundary boxes with NumPy.mpirun -n 1..4 env CUNUMPY_FAKE_CUPY=1 pytest --with-mpi pic/tests/test_sorting_device.pyandmpirun -n 2/3 pytest --with-mpi pic/tests/test_sorting.pypass.Merge order
Based on
devel. It does not depend on the unmerged CUDA PRs.🤖 Generated with Claude Code