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

diff --git a/src/gromacs/mdrun/md.cpp b/src/gromacs/mdrun/md.cpp
index fdb28998e2bf21f51df314bb30bb9bc705d64207..57fb86149a639299c94c1423d734c1acacab1ce5 100644 (file)
--- 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));