Conversation
…ion fallback In calc_uVpsi_rdivided (nonlocal_potential.f90), the compute loop writes the projected nonlocal-PP integrals into dev_uVpsibox (the OpenACC device array), while the host array uVpsibox stays uninitialized. The MPI_VERSION3 path copies the device data back to the host (uVpsibox = dev_uVpsibox) before use, but that copy-back is inside "#ifdef FORTRAN_COMPILER_HAS_MPI_VERSION3" -- so under the non-MPI3 fallback path (FORTRAN_COMPILER_HAS_MPI_VERSION3=OFF) the subsequent comm_summation(uVpsibox, ...) sums uninitialized host memory, yielding uVpsibox2 = 0 and dropping the nonlocal pseudopotential contribution from the Hamiltonian entirely. This produces a silently wrong total energy whenever the real-space grid is MPI-divided (nproc_rgrid>1, the only code path that calls calc_uVpsi_rdivided) on a GPU/OpenACC build with the non-MPI3 fallback: the missing E_ion_nloc term inflates the total energy by |E_ion_nloc| (~22,325 eV on a 486-atom SiO2 3x3x3x3 system), and the matter current density diverges from the correct trajectory from the first timestep. Mirror the existing MPI_VERSION3 copy-back into the #else branch so the host arrays are populated from device before comm_summation reads them. Verified on RIKYU (GB200, nvhpc/26.3, FORTRAN_COMPILER_HAS_MPI_VERSION3=OFF): the patched nproc_rgrid=2,1,1 run is now bit-for-bit identical to the nproc_rgrid=1,1,1 reference (Eall = -164707.165382338 eV at t=0, matching to 15 significant figures), where the unpatched run gave -142382.23 eV.
|
Can one of the admins verify this patch? |
Contributor
|
Thank you @william-dawson san, I have merged this PR! |
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
3 participants
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.
This is a small fix: when MPI3 is disabled in a GPU build, the
#elsebranch ofcalc_uVpsi_rdividedcomputes pseudo-potential projections on the device (dev_uVpsibox) but never copies them back to the host arrays beforecomm_summation. That silently reduces stale data and gives wrong energies in multi-rank runs.The MPI3 branch (
#ifdef) right above already does this copy-back (uVpsibox = dev_uVpsibox). This PR adds the same#ifdef USE_OPENACCcopy-back to the#elsebranch so both paths behave correctly.Verification
SiO₂ 3×3×3 (486 atoms, 1944 states), Rikyu GB200,
nvhpc/26.3, 4 GPUs / 4 ranks: