Extract Particle Communication And Update Infrastructure From PR501#544
Extract Particle Communication And Update Infrastructure From PR501#544aaadelmann wants to merge 22 commits into
Conversation
|
scs-ci run cscs-ci-gh200, cscs-ci-mi300, cscs-ci-openmp |
|
cscs-ci run cscs-ci-gh200, cscs-ci-mi300, cscs-ci-openmp |
|
Looks good to me. But I think CI is still failing. |
Split the communication and particle-update infrastructure from PR501 on top of the PCG split. This brings in reusable communication buffers, page-granular archive allocation, particle attribute serialization hooks, packed particle send IDs, particle sorting buffers, and the rewritten ParticleSpatialLayout update path. Keep this branch independent from the later interpolation, FFT, NUFFT, and PIF splits by dropping those APIs from the extracted ParticleAttrib changes. Add particle update regression coverage and update existing tests for live-view and page-sized buffer semantics. Validated with a Debug Serial Kokkos 5.0.0 build: full 1-rank ctest passes, plus ParticleSendRecv, ParticleUpdate, and ParticleUpdateNonuniform pass under mpiexec -n 2.
Consume CUDA/HIP runtime return values in Archive so HIP nodiscard annotations do not trigger warnings. Update particle benchmark/test callers to store ParticleAttrib::getView() by value now that it returns a live subview instead of a stable lvalue reference.
ParticleSendRecv timed out under CTest with two MPI ranks, but the timeout was only a secondary symptom. One rank threw before the final MPI barrier with: Kokkos::deep_copy extents of views don't match: ParticleAttrib::dview_mirror(64) ParticleAttrib::dview(56) and the peer rank then waited until CTest killed the test. The failing path created a host mirror for expectedRank after bunch->update(), then resized that mirror to expectedRank.size() before copying from expectedRank.getView(). On this branch, particle attributes separate capacity from the live particle count: size() reports the backing capacity, while getView() is trimmed to the live particle range. After migration those values can differ across ranks. Remove the explicit resize to expectedRank.size(). getHostMirror() already returns a mirror compatible with the live view, so the deep_copy now uses matching extents after particle migration. Validated with: cmake --build build --target ParticleSendRecv -j 8 ctest --test-dir build -R ParticleSendRecv --output-on-failure -V The CTest entry runs ParticleSendRecv with mpiexec -n 2 and now passes all 12 typed cases.
benchmarkParticleUpdate failed under the two-rank CTest configuration with a Kokkos deep_copy extent mismatch: ParticleAttrib::dview_mirror(5000) ParticleAttrib::dview(4921) This is the same capacity-vs-live-count issue fixed for ParticleSendRecv. After particle migration, ParticleAttrib::size() reports the backing capacity, while getView() exposes only the live particle range. The benchmark resized the host mirrors for P and R to P->P.size() and P->R.size(), then copied from P->P.getView() / P->R.getView(). When the local particle count differed from the reserved capacity, the destination mirror and source view extents no longer matched. Resize the host mirrors to P->getLocalNum() instead. This matches the live extent used by getView() and keeps the benchmark valid after uneven particle migration across ranks. Validated with: cmake --build build --target benchmarkParticleUpdate -j 8 ctest --test-dir build -R benchmarkParticleUpdate --output-on-failure -V ctest --test-dir build -R 'ParticleSendRecv|benchmarkParticleUpdate' --output-on-failure Both ParticleSendRecv and benchmarkParticleUpdate pass with the configured two-rank MPI launch.
fa749ac to
53f370a
Compare
biddisco
left a comment
There was a problem hiding this comment.
Can you please edit the last couple of commit logs to make them short first line and longer details inside, the current commits break the clean summary view of logs
Reintroduce the `increment_type` helper required by ParticleSpatialOverlapLayout after it was lost during branch split/merge work. This keeps overlap layout code compatible with callers that still use the increment helper during particle layout updates.
Remove the default dimension template parameter that caused faulty type deduction in particle layout/update code. Also update affected ALPINE, cosmology, particle, and test call sites that were left inconsistent after the PR501 split/merge sequence.
Remove invalid `.template` disambiguators from two ParticleContainer calls in
ParticleSpatialLayout::update().
`sendToRank` is a member function template, but the template argument is deduced
from the `ids_sub` argument, so the call should not use an explicit template-id.
`postRecvFromRank` is not a function template at all.
This fixes AppleClang/OpenMP builds that reject the previous syntax with:
a template argument list is expected after a name prefixed by the template keyword
Verification:
- Configured Debug OpenMP build with unit tests enabled
- Built successfully
- Ran unit_tests CTest tree: 37/37 tests passed
GatherScatterTest::ScatterCustomHashTest created a per-rank particle count `n`, then called fillRandomPositions(n). That helper creates local particles and calls bunch->update(), which can migrate particles between ranks in an MPI run. After the update, the actual local particle count is `nLoc`, and the host mirror returned by Q.getHostMirror() is sized for `nLoc` entries. The test still initialized Q_host using the original pre-update count `n`. When a rank ended up with fewer particles than it initially created (`nLoc < n`), this wrote past the end of Q_host and corrupted heap state. The resulting crash appeared later in the test sequence, often inside Kokkos or MPI allocation paths, making the failure look unrelated to the custom-hash scatter test. Initialize charges using the post-update local particle count instead. This matches the size of the host mirror and the particle set that the subsequent hash construction and scatter operation use. Verified on merlin6: - cmake --build build --target GatherScatterTest -j 8 - ctest --test-dir build -R '^GatherScatterTest$' --output-on-failure -V - repeated ctest --test-dir build -R '^GatherScatterTest$' --output-on-failure - git diff --check
Heffte was configured with CMAKE_CUDA_ARCHITECTURES=native, which produced sm_61 CUDA objects inside the A100 build even though Kokkos/IPPL were built for sm_80. Derive CMAKE_CUDA_ARCHITECTURES from the selected Kokkos CUDA architecture before building Heffte from source, so Kokkos_ARCH_AMPERE80 sets CMAKE_CUDA_ARCHITECTURES=80.
The CUDA/HIP archive implementation always used a raw cudaMalloc/hipMalloc buffer whenever GPU support was enabled. That also affected host-space archives, for example Archive<Kokkos::HostSpace> used by OpenMP particle attributes in mixed OpenMP+CUDA builds. As a result, host-side particle serialization could write into CUDA device memory. On A100 this showed up as invalid-permissions/heap corruption failures in multi-rank particle update and gather/scatter tests. Now we use the raw GPU allocation only for true device memory spaces: Kokkos::CudaSpace and Kokkos::HIPSpace. Host-accessible archive memory spaces now use the normal Kokkos view buffer and Kokkos resize/realloc path.
BufferHandler tests assumed that every Archive-backed buffer rounded small allocations to the generic 4 KiB page size. That is no longer true for raw HIP device archives: Archive<Kokkos::HIPSpace> rounds allocations up to the 64 KiB HSA IPC granularity. This caused the HIP BufferHandler test to fail because HIPSpace buffers reported 65536 bytes while the test expected 4096 bytes. Update the test to derive the backend-specific capacity from Archive<memory_space>::getBufferSize() and compare against the maximum of that capacity and BufferHandler's existing 4 KiB page rounding. This avoids hard-coding the HIP granularity in the test while preserving the existing expectations for HostSpace, CUDA, pinned, and managed memory spaces. Also clarify the Archive header comment: raw CUDA/HIP device-memory archives use direct cudaMalloc/hipMalloc storage for MPI IPC compatibility, and HIP device allocations are rounded to the HSA IPC granularity so MI250X/MI300X GPU IPC transfers can be attached by the MPI/HSA stack.
976715a to
01b90a8
Compare
|
Added one liners 01b90a8 Respect backend archive capacity in BufferHandler tests |
Using a regex to scan Kokkos_ARCH_XXX variables makes the implementation more future proof as there is no need to manually add new ARCH family names to the check, we only need the numeric part of the ARCH
|
much better - I added a commit - please check it |
|
cscs-ci run cscs-ci-gh200 |
Extract Particle Communication And Update Infrastructure From PR501
Summary
This PR extracts the particle communication/update infrastructure from the large
PR501 branch. It builds on the PCG allocation split and keeps FFT, NUFFT,
higher-order scatter/gather, and PIF examples out of scope.
The main goals are:
nghostthrough field layout / halo APIs where particle layout needsconsistent ghost-width awareness,
What Changed
Particle migration path
Main files:
src/Particle/ParticleSpatialLayout.hsrc/Particle/ParticleSpatialLayout.hppsrc/Particle/ParticleBase.hsrc/Particle/ParticleBase.hppsrc/Particle/ParticleAttrib.hsrc/Particle/ParticleAttrib.hppsrc/Particle/ParticleAttribBase.hParticleSpatialLayout::update()now has a more explicit multi-stage migrationflow:
The receive tail is timed explicitly:
particleWaitparticleFreeBuffersparticleDeserializeparticleDeserResizeparticleDeserCopyThis makes the previously hidden tail of
updateParticlevisible in profiles.Receive-side pre-reserve fix
The key performance fix is receive-side pre-reserving of particle attribute
capacity.
Root cause observed on LUMI:
ParticleAttrib::deserialize(offset, nrecvs)onceper source rank and attribute.
Kokkos::resizepreserves existing entries, so repeated grows copiedalready-live particle storage many times within a single update step.
particleDeserializeand caused a largeupdateParticleregression.
Fix:
Add
ParticleAttrib::reserve(size_type).Before deferred receive finalizers run, compute final receive capacity once:
Reserve every particle attribute once to that capacity.
Keep receive finalizers focused on archive copy/deserialization.
Relevant code:
Communication archive and buffer handling
Main files:
src/Communicate/Archive.hsrc/Communicate/Archive.hppsrc/Communicate/BufferHandler.hppsrc/Communicate/Buffers.*src/Communicate/Communicator.*src/Communicate/LogEntry.*src/Communicate/LoggingBufferHandler.hThe branch adds/updates:
Archive.Notable HIP detail:
Archiverounds HIP GPU allocations to 64 KiB granularity to satisfy HSA IPCrequirements used by Cray MPICH for large GPU transfers.
hipFree/cudaFreereturn values are intentionally cast tovoidto avoidwarning noise from
nodiscardreturn values in destructors/free paths.Particle sorting infrastructure
Main files:
src/Particle/ParticleSort.hsrc/Particle/SortBuffer.hThe branch introduces reusable sorting buffers and particle sort helpers used by
the new spatial update path. The buffers grow on demand and are reused to avoid
allocation churn.
Field layout / halo
nghostplumbingMain files:
src/FieldLayout/FieldLayout.hsrc/FieldLayout/FieldLayout.hppsrc/FieldLayout/SubFieldLayout.hppsrc/Field/HaloCells.hsrc/Field/HaloCells.hppsrc/Field/BareField.hppThe particle layout changes require consistent ghost-width awareness when field
layouts and halo neighbor regions are computed. This PR threads
nghostthroughthe relevant field layout / halo APIs.
This is small in line count but important for correctness; reviewers should
look at it together with the particle layout changes.
Utility support
Main files:
src/Utility/BufferView.hsrc/Utility/ParallelDispatch.hsrc/Utility/Tuning.hsrc/Utility/TypeUtils.hsrc/Utility/IpplTimings.*src/Utility/Timer.*The utility changes provide reusable support for:
ALPINE Kokkos view lifetime fixes
The ALPINE managers no longer take addresses of temporary Kokkos view handles
returned by
getView().Changed files:
alpine/LandauDampingManager.halpine/BumponTailInstabilityManager.halpine/PenningTrapManager.hBefore:
view_type* R = &(this->pcontainer_m->R.getView()); samplingR.generate(*R, rand_pool64);After:
view_type R = this->pcontainer_m->R.getView(); samplingR.generate(R, rand_pool64);This avoids dangling pointers/references to temporary view handles and fixes
compilers/backends that reject taking the address of a temporary Kokkos view.
Validation And Performance Evidence
LUMI Results (ALPS will follow)
513_10512_10512_10512_10512_10512_10The original symptom was a large
updateParticleregression moving frompr501-pcgto the communication/particle-update split. Initial child timerslooked small because deferred receive finalization/deserialization was hidden in
the tail of
updateParticle.Diagnostics split that tail into:
particleWait,particleFreeBuffers,particleDeserialize.The regression was traced to
particleDeserialize, not MPI wait time.LUMI before/after pre-reserve fix
Recorded in
PR501_SPLIT_MAP.md:updateParticlewall maxparticleDeserializewall maxupdateParticlewall maxparticleDeserializewall maxPost-fix timer split:
particleDeserializewall maxparticleDeserResizewall maxparticleDeserCopywall maxInterpretation:
The repeated preserving resize was the dominant regression.
After pre-reserving, deserialize time is small and almost entirely actual
archive copy.
At 128 ranks,
updateParticlebecame balanced:Local OpenMP check
The split map records a Mac OpenMP comparison between
pr501-pcgandpr501-communication-particle-update.Command shape:
Observed locally:
by the
particleDeserializediagnosis.Test coverage added/updated
New or updated tests include:
unit_tests/Particle/ParticleUpdate.cppunit_tests/Particle/ParticleUpdateNonuniform.cppunit_tests/Particle/ParticleSendRecv.cppunit_tests/Particle/ParticleBase.cppunit_tests/Communicate/BufferHandler.cpptest/particle.ParticleUpdateNonuniform.cppcovers ORB/nonuniform layout scenarios including:Reviewer Notes
Review the particle/communication changes as the new layer on top of PCG.
higher-order scatter/gather, and PIF changes.
nghostfield layout / halo changes should be reviewed with the particlelayout changes; they are part of the same correctness surface.
further GPU performance issues appear, profile
particleDeserCopyandArchive::deserialize(offset).