From f0e5f8899e5f88df10bf624da458207a19eaa22a Mon Sep 17 00:00:00 2001
From: Mark Abraham <mark.j.abraham@gmail.com>
Date: Wed, 27 Nov 2019 09:56:37 +0100
Subject: [PATCH] Disable GPU PME-PP communication with mdrun -pmefft cpu

The MdrunMpiTest with two ranks defaults to using a separate PME rank,
and that attempts to run all flavors of -pmefft.  However only -pmefft
gpu is targeted for any of the new GPU development, so the development
feature flags need to account for that.

SimulationWorkload is also improved to be aware of this fact.

Change-Id: I57c8e8ce81fdb7aaf65de89feebb369d3f2f10eb
---
 src/gromacs/mdrun/runner.cpp                  | 25 +++++++++++++------
 .../decidesimulationworkload.cpp              | 12 ++++-----
 2 files changed, 24 insertions(+), 13 deletions(-)

diff --git a/src/gromacs/mdrun/runner.cpp b/src/gromacs/mdrun/runner.cpp
index 7e3176561f..d68a5fec54 100644
--- a/src/gromacs/mdrun/runner.cpp
+++ b/src/gromacs/mdrun/runner.cpp
@@ -196,12 +196,12 @@ struct DevelopmentFeatureFlags
  *
  * \param[in]  mdlog                Logger object.
  * \param[in]  useGpuForNonbonded   True if the nonbonded task is offloaded in this run.
- * \param[in]  useGpuForPme         True if the PME task is offloaded in this run.
+ * \param[in]  pmeRunMode           The PME run mode for this run
  * \returns                         The object populated with development feature flags.
  */
 static DevelopmentFeatureFlags manageDevelopmentFeatures(const gmx::MDLogger& mdlog,
                                                          const bool           useGpuForNonbonded,
-                                                         const bool           useGpuForPme)
+                                                         const PmeRunMode     pmeRunMode)
 {
     DevelopmentFeatureFlags devFlags;
 
@@ -266,7 +266,7 @@ static DevelopmentFeatureFlags manageDevelopmentFeatures(const gmx::MDLogger& md
 
     if (devFlags.enableGpuPmePPComm)
     {
-        if (useGpuForPme)
+        if (pmeRunMode == PmeRunMode::GPU)
         {
             GMX_LOG(mdlog.warning)
                     .asParagraph()
@@ -276,12 +276,23 @@ static DevelopmentFeatureFlags manageDevelopmentFeatures(const gmx::MDLogger& md
         }
         else
         {
+            std::string clarification;
+            if (pmeRunMode == PmeRunMode::Mixed)
+            {
+                clarification =
+                        "PME FFT and gather are not offloaded to the GPU (PME is running in mixed "
+                        "mode).";
+            }
+            else
+            {
+                clarification = "PME is not offloaded to the GPU.";
+            }
             GMX_LOG(mdlog.warning)
                     .asParagraph()
-                    .appendTextFormatted(
+                    .appendText(
                             "NOTE: GMX_GPU_PME_PP_COMMS environment variable detected, but the "
-                            "'GPU PME-PP communications' feature was not enabled as PME is not "
-                            "offloaded to the GPU.");
+                            "'GPU PME-PP communications' feature was not enabled as "
+                            + clarification);
             devFlags.enableGpuPmePPComm = false;
         }
     }
@@ -889,7 +900,7 @@ int Mdrunner::mdrunner()
     // Initialize development feature flags that enabled by environment variable
     // and report those features that are enabled.
     const DevelopmentFeatureFlags devFlags =
-            manageDevelopmentFeatures(mdlog, useGpuForNonbonded, useGpuForPme);
+            manageDevelopmentFeatures(mdlog, useGpuForNonbonded, pmeRunMode);
 
     // NOTE: The devFlags need decideWhetherToUseGpusForNonbonded(...) and decideWhetherToUseGpusForPme(...) for overrides,
     //       decideWhetherToUseGpuForUpdate() needs devFlags for the '-update auto' override, hence the interleaving.
diff --git a/src/gromacs/taskassignment/decidesimulationworkload.cpp b/src/gromacs/taskassignment/decidesimulationworkload.cpp
index afba4097f4..16849d4e5d 100644
--- a/src/gromacs/taskassignment/decidesimulationworkload.cpp
+++ b/src/gromacs/taskassignment/decidesimulationworkload.cpp
@@ -64,12 +64,12 @@ SimulationWorkload createSimulationWorkload(bool       useGpuForNonbonded,
     simulationWorkload.useGpuNonbonded = useGpuForNonbonded;
     simulationWorkload.useCpuPme       = (pmeRunMode == PmeRunMode::CPU);
     simulationWorkload.useGpuPme = (pmeRunMode == PmeRunMode::GPU || pmeRunMode == PmeRunMode::Mixed);
-    simulationWorkload.useGpuPmeFft                 = (pmeRunMode == PmeRunMode::Mixed);
-    simulationWorkload.useGpuBonded                 = useGpuForBonded;
-    simulationWorkload.useGpuUpdate                 = useGpuForUpdate;
-    simulationWorkload.useGpuBufferOps              = useGpuForBufferOps || useGpuForUpdate;
-    simulationWorkload.useGpuHaloExchange           = useGpuHaloExchange;
-    simulationWorkload.useGpuPmePpCommunication     = useGpuPmePpComm;
+    simulationWorkload.useGpuPmeFft             = (pmeRunMode == PmeRunMode::Mixed);
+    simulationWorkload.useGpuBonded             = useGpuForBonded;
+    simulationWorkload.useGpuUpdate             = useGpuForUpdate;
+    simulationWorkload.useGpuBufferOps          = useGpuForBufferOps || useGpuForUpdate;
+    simulationWorkload.useGpuHaloExchange       = useGpuHaloExchange;
+    simulationWorkload.useGpuPmePpCommunication = useGpuPmePpComm && (pmeRunMode == PmeRunMode::GPU);
     simulationWorkload.useGpuDirectCommunication    = useGpuHaloExchange || useGpuPmePpComm;
     simulationWorkload.haveEwaldSurfaceContribution = haveEwaldSurfaceContribution;
 
-- 
2.22.0