Draft: OpenACC tuning + nvhpc/26.5 fixes (device-side loop tripcounts, MPI_Init CUDA-context hang) - #1276
Draft
william-dawson wants to merge 13 commits into
Conversation
|
Can one of the admins verify this patch? |
william-dawson
marked this pull request as draft
July 28, 2026 16:35
william-dawson
force-pushed
the
feature/stencil-current-combined
branch
2 times, most recently
from
July 29, 2026 05:21
a78bd4f to
7c6fc7e
Compare
Three independent kernel-level optimizations for the GPU (OpenACC) build, measured on B200 (Rikyu): - pseudo-pt (nonlocal_potential.f90): rewrite the nonlocal-potential application as a batched cuBLAS GEMM instead of a naive per-atom point loop, gated on USE_GEMM. - current-density (density_matrix.f90): inline stencil_current's body directly into calc_current's OpenACC region, removing a device- routine call boundary that cost register-spill overhead. - stencil (zstencil_core/gpu/seq.f90): restructure the hpsi stencil application for better register/cache behavior. Headline (see subwg2-benchmarks' salmon-gpu-optimization-ideas skill for full methodology and numbers): rt iterations improves 1.55x on B200 1-GPU, 1.59x on B200 4-GPU, 1.39x on GH200, correctness bit-exact everywhere. Originally landed as SALMON-TDDFT#1276 (feature/stencil-current-combined, commit 7c6fc7e).
New platform preset for configure.py wiring up the batched cuBLAS-GEMM pseudo-pt path from the previous commit: OpenACC on, USE_CUDA deliberately OFF (this is a distinct, newer optimization path from SALMON's older hand-written CUDA kernels in zpseudo.cu/ stencil_current.cu -- those are a known net loss at this problem size, see subwg2-benchmarks' salmon-build skill, and stencil_current.cu's host wrapper has a separate real out-of-bounds device-buffer bug under real-space decomposition, see the salmon-gpu-optimization-ideas skill's Open item 5), -DUSE_GEMM added to the Fortran/C flags, cusolver linked in alongside cublas.
Under nvfortran 26.5 the OpenACC accumulator/reduction loops in this file
silently return the reduction identity instead of the computed value,
producing a wrong (but plausible, deterministic) total energy -- for an
18-atom SiO2 test, -6112.12922835 eV instead of -6099.94333354 eV, a fixed
12.19 eV offset located entirely in E_ion_ion. nvhpc 26.3 is unaffected.
Three loops in this file are hit:
1. init_ewald, pair-counting loop, reduction(max:npair_bk_max)
-> returns 0, so ewald%bk is allocated with ZERO pair slots and every
later access to it is out of bounds.
2. init_ewald, book-keeping fill loop
-> the per-atom counter ends at 1 instead of ~74.
3. calc_Total_Energy_periodic, real-space Ewald sum, reduction(+:E_tmp)
-> returns 0, dropping the real-space ion-ion term entirely.
The root cause is in the compiler, not in SALMON. This was verified by
rewriting loops 1 and 3 as hand-written CUDA Fortran kernels and running
them on the same GPU, in the same binary, over the same data: CUDA computes
74 and 0.4478233939199485 (both correct, matching 26.3) while the OpenACC
versions return 0. Also ruled out, each by direct test: corrupted input
data, private-array handling, the sum(rab(:)**2) array temporary,
-gpu=tripcount:host, kernels-vs-parallel-loop, explicit data clauses,
source position, and extraction into a separate subroutine.
Fix: gate these three regions on a new SALMON_EWALD_ACC macro, defined only
when the compiler is not affected. On unaffected compilers nothing changes
and the GPU path is kept; on 26.5+ the three loops take the OpenMP host
path that already existed in the #else branch. All three are one-time or
per-ion-move setup work, so there is no hot-path cost.
Separately, the fill loop declared copyin(ewald) -- host->device only --
while writing ewald%bk / ewald%npair_bk inside the region, so those device
writes were semantically discarded. That wrong clause is removed here. It
is an independent latent bug; fixing it alone does not cure 26.5.
Verified on DGX Spark (GB10, Blackwell), same source both ways:
nvhpc/26.5 -> bookkeeping=111, -6099.94333354 eV (host path taken)
nvhpc/26.3 -> bookkeeping=111, -6099.94333354 eV (GPU path retained)
UCC's MPI_COMM_WORLD team bootstrap happens inside MPI_Init_thread, and does CUDA-aware UCX rendezvous-protocol probing (cuCtxGetDevice) as part of that. SALMON calls MPI_Init as essentially its first action -- no CUDA context exists yet, so the probe fails with CUDA_ERROR_INVALID_CONTEXT. UCX appears to swallow that failure silently and cache a broken/incomplete rendezvous-protocol config for the team, which only surfaces later as a hang on the first real collective that needs it: "UCX ERROR cannot find remote protocol for: UCC_UCP_CONTEXT inter-node cfg#N | tag_send from cuda-managed/GPU0", then an infinite spin in ucc_tl_ucp_bcast_knomial_progress with zero further output -- reproduced 100% of the time at 2+ nodes on native nvhpc/26.5 (HPC-X 2.50/UCC 1.8.0/UCX 1.21.0), never at 1 node. Root-caused via a live gdb backtrace on the hung process (pinned it to comm_bcast_array3d_double -> MPI_Bcast -> mca_coll_ucc_bcast, broadcasting system%rocc -- plain host memory, confirming the cuda-managed reference in the error is UCC's own internal staging buffer, not user data) and compute-sanitizer on the real binary, which caught the actual CUDA_ERROR_INVALID_CONTEXT happening inside MPI_Init's UCC team-creation path (ucc_team_create_test_single -> ucc_service_allgather -> ucp_wireup_init_lanes -> ucp_proto_rndv_ctrl_probe -> cuCtxGetDevice). Fix: call acc_init(acc_device_nvidia) before MPI_Init_thread, gated on USE_OPENACC (no-op otherwise). acc_init respects CUDA_VISIBLE_DEVICES, which is already set correctly per-rank before this point (wrapper.sh sets it from OMPI_COMM_WORLD_LOCAL_RANK before exec'ing the binary), so this binds the right GPU. Verified on real hardware: 8-GPU/2-node orbital decomposition, previously hanging 100% of the time, now runs cleanly to completion -- end SALMON, exit 0, rt iterations 57.07s, energy -164703.595116277 eV bit-exact vs the single-rank reference. No downgrade to nvhpc/26.3 or HPC-X 2.20 needed; this fixes the actual bug on the toolchain BenchKit was already trying to use.
william-dawson
force-pushed
the
feature/stencil-current-combined
branch
from
August 18, 2026 02:17
7c6fc7e to
4815076
Compare
william-dawson
added a commit
to william-dawson/benchkit
that referenced
this pull request
Aug 18, 2026
FugakuNEXT-v1 (also open upstream as SALMON-TDDFT/SALMON2#1276) is the single consolidated branch replacing the ad-hoc chain this pin has gone through (v.2.2.2-rikyu-optimized-265 -> the nvhpc/26.3+MPI3-off workaround -> this): develop-2.0.0@9b93a8c4 plus four commits, one per concern -- the OpenACC tuning, the cmake wiring for it, the nvhpc/26.5 Ewald-reduction-bug guard, and (new) a real fix for the 2+ node UCC hang that previously forced the 26.3+MPI3-off downgrade. That fix (acc_init(acc_device_nvidia) before MPI_Init_thread, so UCC's CUDA-aware protocol probe during MPI_Init's team bootstrap doesn't run contextless and cache a broken config) means the downgrade is no longer needed: back to nvhpc/26.5, native MPI3 ON, no FORTRAN_COMPILER_HAS_MPI_VERSION3 override. This is the actually-tested combination now -- orbital decomposition 1-8 nodes and domain decomposition 1-2 nodes, all bit-exact, all previously-hanging configurations included.
…ce buffer allreduce
Collapse the verbose GEMM-path and phase-1.5 comment blocks, restore the load-bearing notes lost when the GEMM path was hoisted (zekr_uV's A(t) time dependence, the 3D-shape nvfortran ICE workaround, the cuBLAS stream ordering requirement), and remove the comm_summation import left behind when phase 1.5 moved to a direct MPI_Allreduce. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Cut the multi-paragraph explanations down to the load-bearing facts, and drop a reference to an internal repository from communication.f90. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
MPI_Allreduce performs the reduction arithmetic on the host even when handed a device pointer, so the pseudo-pt reduce leaves NVLink and runs at a few GB/s. NCCL reduces on the GPU and keeps it there. Guarded by USE_NCCL; the MPI_Allreduce path is unchanged when it is off. The Fortran ncclAllReduce interface does not accept complex(8), so the buffer is aliased to real(8) and summed as 2N doubles. 3x3x3 SiO2, nt=300, Rikyu GB200, energies bit-exact: layout GPUs pseudo-pt comm rt iterations domain 1,2,1 2 14.92 -> 1.59 162.52 -> 148.37 domain 2,2,1 4 18.20 -> 1.70 99.14 -> 80.46 mixed Po4xPg4 16 4.42 -> 0.55 35.40 -> 30.98 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
ncclCommInitRank was checked but ncclAllReduce and the stream sync were not, so a failed reduction would have silently corrupted the result. Check both. Also remove a speedup multiplier from the comment that was written before the measurements and did not match any of them; the load-bearing part is that MPI reduces on the host. Leaves the per-call c_f_pointer alone: d_reduce is reallocated whenever Nlma*norb changes, so hoisting it to one-time init would dangle. Verified on Rikyu, 4 GPU domain 2,2,1: -164703.595116277 eV, bit-exact. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
nsys attributed 82% of calculating-curr to calc_current_nonlocal (88.6 ms per RT step, against 19.6 for the grid stencil). It is an !$acc routine vector doing four projections per (ilma,io) -- conjg(zekr_uV).psi and the same weighted by x, y and z -- which is zpseudo's phase-1 contraction four times over. Stack zekr_uV and its three coordinate-weighted copies into one packed (nps, 4*max_nproj, natom) matrix so a single cublasZgemmStridedBatched returns all four, and drop the device-routine call boundary with it. 3x3x3 SiO2, nt=300, Rikyu GB200, energies bit-exact at -164703.595116277: layout GPUs calculating curr rt iterations orbital 1 49.48 -> 22.49 234.04 -> 208.35 orbital 4 32.88 -> 7.94 81.76 -> 57.77 orbital 8 29.73 -> 4.34 56.23 -> 30.92 orbital 16 29.47 -> 3.63 44.60 -> 18.72 mixed Po4xPg4 16 10.88 -> 4.15 30.98 -> 24.23 mixed Po8xPg4 32 10.03 -> 2.77 23.18 -> 16.56 Current density was the one kernel that did not scale under orbital decomposition, and the only thing real-space decomposition improved. With it 8x faster, orbital now beats mixed at every count measured (16 GPUs 18.72 vs 24.23), so the halo cost of splitting the grid no longer buys anything on this problem. Guarded by USE_GEMM; the routine-vector path is unchanged when it is off. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
-DSALMON_NO_EWALD_WORKAROUND puts the three reductions back on the GPU, so re-testing whether a given nvfortran still miscompiles them is a build flag rather than a source edit. Default behaviour is unchanged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The inlined stencil in calc_current's OpenACC branch ganged over (ik,io) only -- nk*numo gangs, ~60 at Po=32 against ~148 SMs, so occupancy binds hard at scale and the GPU with more SMs (B200) is more starved, not less. Collapse (ik,io,iz,iy,ix) into one gang reduction so the gang count is nk*numo*ngrid. The per-orbital reductions (rtmp/xtmp/ytmp/ztmp combined after) become a direct reduction into jx/jy/jz, valid because the kAc/BT/rocc/wtk weighting is linear in the grid sum. This is the grid-parallel version that was written and measured once, gained 7% at Po=16, and was reverted because the nonlocal half of current density still dominated. The GEMM rewrite has since removed that bottleneck, so the stencil half is now the larger share and this change should help -- proportionally more on B200 than H100, since the occupancy starvation is what flipped B200 behind H100 on the curr kernel. Not bit-exact (FP order changes); correctness is verified against the reference energy, per the repo's standard bar.
william-dawson
added a commit
to william-dawson/benchkit
that referenced
this pull request
Aug 26, 2026
FugakuNEXT-v1 (also open upstream as SALMON-TDDFT/SALMON2#1276) is the single consolidated branch replacing the ad-hoc chain this pin has gone through (v.2.2.2-rikyu-optimized-265 -> the nvhpc/26.3+MPI3-off workaround -> this): develop-2.0.0@9b93a8c4 plus four commits, one per concern -- the OpenACC tuning, the cmake wiring for it, the nvhpc/26.5 Ewald-reduction-bug guard, and (new) a real fix for the 2+ node UCC hang that previously forced the 26.3+MPI3-off downgrade. That fix (acc_init(acc_device_nvidia) before MPI_Init_thread, so UCC's CUDA-aware protocol probe during MPI_Init's team bootstrap doesn't run contextless and cache a broken config) means the downgrade is no longer needed: back to nvhpc/26.5, native MPI3 ON, no FORTRAN_COMPILER_HAS_MPI_VERSION3 override. This is the actually-tested combination now -- orbital decomposition 1-8 nodes and domain decomposition 1-2 nodes, all bit-exact, all previously-hanging configurations included. Signed-off-by: William Dawson <william.dawson@riken.jp>
nvfortran >= 26.5 evaluates an OpenACC loop's tripcount on the device rather than on the host, which is the spec-correct behaviour. Three loops in total_energy.f90 take their bound from info%nion_mg, a scalar member of a dummy-argument derived type that is not device-resident under -gpu=managed, so the device read a zero tripcount: the loops ran zero times and their reductions returned the identity. The visible symptom was a wrong but plausible Total Energy with no crash and no warning -- the ion-ion pair count came out 0 instead of 111 on an 18-atom test, and the real-space Ewald term was dropped entirely. Hoist the bound into a local scalar at each of the three loops. They stay on the GPU and nvhpc 26.5 now reproduces 26.3 bit-for-bit. This replaces the previous workaround, which gated the loops on __NVCOMPILER_MAJOR__/MINOR and ran them on the host for >= 26.5. That was based on a misdiagnosis: the reductions were never miscompiled, the loops simply did not execute. The version gate, the SALMON_EWALD_ACC macro and the SALMON_NO_EWALD_WORKAROUND switch are all removed, restoring upstream's plain USE_OPENACC gating. Also keeps the separate fix to init_ewald's fill loop, which declared copyin(ewald) while writing ewald%bk / ewald%npair_bk and so discarded those device writes. Verified on Rikyu (GB200, nvhpc/26.5, -acc=strict -gpu=...,managed): 18-atom SiO2, pair count 111 and Total Energy 11798.50640962 / 2510.46134449 at 1 rank, matching nvhpc/26.3 exactly; identical results at 2 and 4 ranks, where info%nion_mg differs per rank.
william-dawson
added a commit
to william-dawson/benchkit
that referenced
this pull request
Aug 26, 2026
FugakuNEXT-v1 (also open upstream as SALMON-TDDFT/SALMON2#1276) is the single consolidated branch replacing the ad-hoc chain this pin has gone through (v.2.2.2-rikyu-optimized-265 -> the nvhpc/26.3+MPI3-off workaround -> this): develop-2.0.0@9b93a8c4 plus four commits, one per concern -- the OpenACC tuning, the cmake wiring for it, the nvhpc/26.5 Ewald-reduction-bug guard, and (new) a real fix for the 2+ node UCC hang that previously forced the 26.3+MPI3-off downgrade. That fix (acc_init(acc_device_nvidia) before MPI_Init_thread, so UCC's CUDA-aware protocol probe during MPI_Init's team bootstrap doesn't run contextless and cache a broken config) means the downgrade is no longer needed: back to nvhpc/26.5, native MPI3 ON, no FORTRAN_COMPILER_HAS_MPI_VERSION3 override. This is the actually-tested combination now -- orbital decomposition 1-8 nodes and domain decomposition 1-2 nodes, all bit-exact, all previously-hanging configurations included. Signed-off-by: William Dawson <william.dawson@riken.jp>
yoshifuminakamura
pushed a commit
to RIKEN-RCCS/benchkit
that referenced
this pull request
Aug 27, 2026
FugakuNEXT-v1 (also open upstream as SALMON-TDDFT/SALMON2#1276) is the single consolidated branch replacing the ad-hoc chain this pin has gone through (v.2.2.2-rikyu-optimized-265 -> the nvhpc/26.3+MPI3-off workaround -> this): develop-2.0.0@9b93a8c4 plus four commits, one per concern -- the OpenACC tuning, the cmake wiring for it, the nvhpc/26.5 Ewald-reduction-bug guard, and (new) a real fix for the 2+ node UCC hang that previously forced the 26.3+MPI3-off downgrade. That fix (acc_init(acc_device_nvidia) before MPI_Init_thread, so UCC's CUDA-aware protocol probe during MPI_Init's team bootstrap doesn't run contextless and cache a broken config) means the downgrade is no longer needed: back to nvhpc/26.5, native MPI3 ON, no FORTRAN_COMPILER_HAS_MPI_VERSION3 override. This is the actually-tested combination now -- orbital decomposition 1-8 nodes and domain decomposition 1-2 nodes, all bit-exact, all previously-hanging configurations included. Signed-off-by: William Dawson <william.dawson@riken.jp>
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
Six independent, individually-reviewable changes for the OpenACC (GPU) build, each landed as its own commit:
USE_GEMM), current-density inlined intocalc_current's OpenACC region (removes a device-routine call boundary), and a restructured hpsi stencil kernel. Measured on B200 (Rikyu):rt iterationsimproves 1.55x on 1-GPU, 1.59x on 4-GPU; 1.39x on GH200. Correctness bit-exact everywhere. (Originally the entire content of this PR before the additions below.)nvhpc-openacc-gemm.cmake— newconfigure.pyplatform preset wiring up (1): OpenACC on,USE_CUDAdeliberately off (SALMON's older hand-written CUDA kernels inzpseudo.cu/stencil_current.cuare a separate, older optimization path -- a net performance loss at the problem sizes tested so far, andstencil_current.cu's host wrapper has a real out-of-bounds device-buffer bug under real-space decomposition),-DUSE_GEMM,cusolverlinked alongsidecublas.nvfortran26.5 evaluates a loop's tripcount on the device rather than on the host, which is the spec-correct behaviour. Three loops intotal_energy.f90took their bound frominfo%nion_mg, a scalar member of a dummy-argument derived type that is not device-resident under-gpu=managed, so the device read a zero tripcount: the loops ran zero times and the reductions in them returned the identity, giving a wrong but plausible total energy with no crash and no warning. Fix: hoist the bound into a local scalar at each loop, so they stay on the GPU and 26.5 reproduces 26.3 bit-for-bit (verified on B200 at 1, 2 and 4 ranks, whereinfo%nion_mgdiffers per rank). Diagnosis of the tripcount default change is due to Seth Camp (@scamp-nvidia, NVIDIA); an earlier revision of this PR misattributed the symptom to a compiler codegen defect and worked around it with a version gate and a host fallback, both now removed. Also drops acopyinclause on one of those loops, which writes to the structure it declared host-to-device only, so those device writes were being discarded.MPI_Init_thread— fixes a real, 100%-reproducible hang at 2+ nodes on nativenvhpc/26.5(HPC-X 2.50/UCC 1.8.0/UCX 1.21.0). UCC'sMPI_COMM_WORLDteam bootstrap happens insideMPI_Init, and does CUDA-aware UCX rendezvous-protocol probing (cuCtxGetDevice) as part of that -- but SALMON callsMPI_Initbefore ever touching CUDA, so the probe runs with no context and fails withCUDA_ERROR_INVALID_CONTEXT. UCX appears to swallow that silently and cache a broken protocol config for the team, which only surfaces later as an infinite spin on the first real collective that needs it (cannot find remote protocol for: UCC_UCP_CONTEXT inter-node cfg#N | tag_send from cuda-managed/GPU0). Root-caused via a livegdbbacktrace on the hung process andcompute-sanitizeron the real binary (caught the actualCUDA_ERROR_INVALID_CONTEXTinsideMPI_Init's UCC team-creation path). Fix:acc_init(acc_device_nvidia)beforeMPI_Init_thread, gated onUSE_OPENACC(no-op otherwise).!$acc update host/deviceare no-ops under-gpu=managed). Packing into a CUDA Fortrandevicebuffer cut pseudo-pt communication 3.9-4.3x.-DUSE_NCCLoptionally routes the same reduce throughncclAllReduce.(ik, io, grid)into one gang reduction incalc_current's inlined stencil, which was occupancy-starved at high orbital parallelism (~60 gangs against ~148 SMs atPo=32). Bit-exact;rt iterimproves 4.5% at 4 GPUs rising to 23% at 32.Earlier revisions of this PR carried a version-gated host fallback for (3), on the assumption that the reductions were miscompiled. They were not, and it has been removed.
Scaling matrix
Same binary throughout (
USE_GEMM+USE_NCCL),nvhpc-hpcx-cuda13/26.5,--mca fcoll individual,--exclusive. Every run bit-exact at-164703.595116277 eV. 3x3x3 SiO2: 486 atoms, 2592 electrons, 1944 states, grid 87x147x93, Gamma-only,nt=300from a pre-staged folded restart.Times are seconds for the whole
rt iterationsblock (300 steps).Po=nproc_ob,Pg= product ofnproc_rgrid; mixed layouts keepPginside a node viaprocess_allocation='grid_sequential'.Po4Pg4Po8Po2xPg4Po16Po4xPg4Po32Po8xPg4Pure orbital decomposition is fastest at every count, by 1.29-1.39x over the best mixed layout. Real-space decomposition's only advantage is on current density, and that kernel is now small enough (2.8-4.3 s at 8+ GPUs) that it does not pay for the halo exchange splitting the grid costs. Domain decomposition stays useful as the fallback when orbital parallelism runs out, since
Pohas to divide sensibly intonstate.Two environment notes for reproducing this:
nvhpc-hpcx-cuda13/26.5. On a single node26.3 + --mca coll_ucc_enable 1gives a faster allreduce, but UCC is not usable across nodes on this stack, so it is not used anywhere above.--mca fcoll individualis required.MPI_File_read_allreads the restart directly intospsi%zwf, which is managed memory, and OMPIO's two-phase collective I/O then redistributes it throughcuMemcpyAsync. On 26.3 that is merely slow (restart read 66.9s -> 41.7s with the flag); on 26.5 it deadlocks whenever thenproc_rgridproduct exceeds 2.Miyabi-G: three-way build comparison at 4 GPUs
3x3x3 SiO2, 486 atoms,
nt=300, orbitalnproc_ob=4. Miyabi-G is 1 GPU/node (H100, IB NDR), so 4 GPUs = 4 nodes. Built with nvhpc/26.3 (cc90). Rikyu row from the matrix above (B200, 1 node, NVLink, nvhpc/26.5).USE_OPENACCUSE_CUDA