Repository navigation
SCC26: Use nonblocking MPI for halo exchange - #2090
RedTimesZero wants to merge 2 commits into
Conversation
|
Claude Code Review Head SHA: fb5c8d9 Files changed:
Findings: |
Post both sides of a direction with MPI_Isend/MPI_Irecv and wait once with MPI_Waitall instead of two back-to-back MPI_SENDRECV calls. halo_size now uses integer arithmetic and is asserted against the per-direction buffer count. Fixes MFlowCode#2089 Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
8b30b03 to
fb5c8d9
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2090 +/- ##
==========================================
+ Coverage 61.90% 61.92% +0.02%
==========================================
Files 86 86
Lines 22885 22913 +28
Branches 3369 3372 +3
==========================================
+ Hits 14166 14190 +24
- Misses 6220 6222 +2
- Partials 2499 2501 +2 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
@RedTimesZero A maintainer will mark your pull request as ready for review when it is suitable to do so |
|
Thanks for the note, and sorry for marking it ready myself. I'll leave it as a draft and won't change its state in future PRs either. The Phoenix (NVHPC) gpu-omp job shows MPI-related test failures in this PR. I'm looking into it and will push a fix before asking for review. |
NVHPC 26.3 computed a zero-length section for buff_send(off:off + buffer_count - 1) when the bounds were local variables of s_mpi_exchange_halo_buffers, so the per-side target update moved nothing and the 2-rank halo data was stale. Pass the offset and count as dummy arguments of small helpers instead.
Lines of Code
|
Description
This PR is from the NTHU team ( Team NTHU SCC ) participating in the SC26 Student Cluster Competition.
Replace the two blocking
MPI_SENDRECVcalls per direction in the halo exchange with oneMPI_Isend/MPI_Irecv+MPI_Waitall, so a rank no longer waits for the first neighbour before starting the second side.Fixes #2089
Changes
s_mpi_sendrecv_variables_buffersis split intos_mpi_pack_halo_buffer,s_mpi_exchange_halo_buffersands_mpi_unpack_halo_buffer, around a smallhalo_sidedescriptor (peer, tags, pack/unpack offsets, buffer offset).buff_send/buff_recvare split into two halves (side 2 starts athalo_size + 1).halo_sizenow uses integer arithmetic so the halves cannot overlap, and an@:ASSERTchecksbuffer_count <= halo_size + 1.GPU_UPDATEstaging are unchanged (now per half).GPU_UPDATEis done in two small helpers (s_mpi_halo_send_to_host,s_mpi_halo_recv_to_device) that take the offset and count as arguments. With the bounds as local variables ofs_mpi_exchange_halo_buffers, NVHPC 26.3 computed a zero-length section under OpenMP offload (the runtime reportedsize=0, nothing moved) and the 2-rank tests failed with--gpu mp.m_boundary_common.fpp: comment only, documenting the call order (-1first, then+1, same arguments).Documentation: the "How to Extend MPI Halo Exchange" section of
docs/documentation/contributing.mdstill describes theMPI_SENDRECVstructure. I left it untouched in this PR; if the approach is accepted I am happy to update it, with whatever wording you prefer.Testing
./mfc.sh test(full,--gpu acc, 1 H200, 2 ranks)./mfc.sh test --gpu mp(NVHPC 26.3),2 MPI Ranks./mfc.sh test --gpu mp,MPI Consistency--gpu accand--gpu mp--gpu accand--gpu mp./mfc.sh format--gpu mpNote: the test suite only exercises 2 ranks, so the pairwise path (two distinct neighbours in one direction) is covered by the shock-droplet comparisons above, not by
./mfc.sh test.Performance
Grind time (ns/gp/eq/rhs), master -> this PR. Unmodified examples, one rank per H200 (one rank per node),
--case-optimization,--mpi, no other options.Notes:
halo_sizecomputation.GPU_UPDATEsection bounds are passed to small helper routines as arguments; with local variables NVHPC 26.3 computed a zero-length section under--gpu mp. Before this fix the multi-rank OpenMP runs produced NaN.Contribution Policy
All contributions are expected to demonstrate:
If these expectations are not met, we would prefer to implement the changes ourselves rather than spend time reviewing low-effort submissions.