From 4d38a0d65839f951bf275e465f8fb65a35b60b8d Mon Sep 17 00:00:00 2001 From: =?utf8?q?Szil=C3=A1rd=20P=C3=A1ll?= Date: Sat, 28 Aug 2021 23:32:26 +0200 Subject: [PATCH] Fix incorrect event dependency of GPU update With separate PME ranks, staged communication and GPU update, the final forces are produced on the CPU and re-uploaded to the GPU prior to update. This upload was done on a "All" locality leading to an incorrect dependency on an event corresponding to the GPU reduction. This leads ot a logic error and incorrect synchronization, but not incorrect results as the "All" locality copy is done on the update stream, hence implicit dependency applies. This change moves this special case copy to the same locality used in other force host to device copies to eliminate the bug. Fixes #4130 --- src/gromacs/mdrun/md.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/gromacs/mdrun/md.cpp b/src/gromacs/mdrun/md.cpp index fdb28998e2..57fb86149a 100644 --- a/src/gromacs/mdrun/md.cpp +++ b/src/gromacs/mdrun/md.cpp @@ -1508,8 +1508,9 @@ void gmx::LegacySimulator::do_md() if (simulationWork.useGpuPme && simulationWork.useCpuPmePpCommunication) { - // The PME forces were recieved to the host, so have to be copied - stateGpu->copyForcesToGpu(f.view().force(), AtomLocality::All); + // The PME forces were recieved to the host, and reduced on the CPU with the rest of the + // forces computed on the GPU, so the final forces have to be copied back to the GPU + stateGpu->copyForcesToGpu(f.view().force(), AtomLocality::Local); } else if (!runScheduleWork->stepWork.useGpuFBufferOps) { @@ -1517,7 +1518,6 @@ void gmx::LegacySimulator::do_md() // host and have to be copied stateGpu->copyForcesToGpu(f.view().force(), AtomLocality::Local); } - const bool doTemperatureScaling = (ir->etc != TemperatureCoupling::No && do_per_step(step + ir->nsttcouple - 1, ir->nsttcouple)); -- 2.22.0