Divide default communicator from DD communicators

The communicators mpi_comm_mysim and mpi_comm_mygroup inside
t_commrec got initialized in init_commrec (to MPI_COMM_WORLD
if no multisim, to a subset otherwise). These communicators
were then used in subsequent setup work, before they got
reassigned during the construction of the DDBuilder object
and the construction of the actual domain decomposition object.
Effectively, this means that the same communicators (and, hence,
identical function calls) do very different things depending on
whether they get used before or after the setup of domain
decomposition. It also means that before DD set up, mpi_comm_mysim
and mpi_comm_mygroup are *identical*.

This change introduces an additional communicator within
t_commrec, mpiDefaulCommunicator, which helps to make these
implicit assumptions explicit. Consequently, this also redefines
PAR(cr), MASTER(cr), and SIMMASTER(cr).

This change will allow to move the sim and group communicators,
which are now only created at DD time, into the DD object,
logically separating the DD object from t_commrec.

Refs #2395

diff --git a/src/gromacs/domdec/box.cpp b/src/gromacs/domdec/box.cpp
index 9f40425c1bad4ab8a41255635112ab1b2c759cf1..8c342f84380cac78f50f71b896a70352cf4f466a 100644 (file)
--- a/src/gromacs/domdec/box.cpp
+++ b/src/gromacs/domdec/box.cpp
@@ -300,18 +300,19 @@ void set_ddbox(const gmx_domdec_t&            dd,
     }
 }
 
-void set_ddbox_cr(const t_commrec&               cr,
+void set_ddbox_cr(DDRole                         ddRole,
+                  MPI_Comm                       communicator,
                   const ivec*                    dd_nc,
                   const t_inputrec&              ir,
                   const matrix                   box,
                   gmx::ArrayRef<const gmx::RVec> x,
                   gmx_ddbox_t*                   ddbox)
 {
-    if (MASTER(&cr))
+    if (ddRole == DDRole::Master)
     {
         low_set_ddbox(numPbcDimensions(ir.pbcType), inputrec2nboundeddim(&ir), dd_nc, box, true, x,
                       nullptr, ddbox);
     }
 
-    gmx_bcast(sizeof(gmx_ddbox_t), ddbox, cr.mpi_comm_mygroup);
+    gmx_bcast(sizeof(gmx_ddbox_t), ddbox, communicator);
 }

diff --git a/src/gromacs/domdec/box.h b/src/gromacs/domdec/box.h
index 4cc482b4021324192e81990fbcf49b4fd92d56e4..7974b801c1e4bc3fcf8bc5a56c17dad9a560e284 100644 (file)
--- a/src/gromacs/domdec/box.h
+++ b/src/gromacs/domdec/box.h
@@ -47,6 +47,7 @@
 #include <vector>
 
 #include "gromacs/math/vectypes.h"
+#include "gromacs/utility/gmxmpi.h"
 
 namespace gmx
 {
@@ -57,6 +58,7 @@ struct gmx_ddbox_t;
 struct gmx_domdec_t;
 struct t_commrec;
 struct t_inputrec;
+enum class DDRole;
 
 /*! \brief Set the box and PBC data in \p ddbox */
 void set_ddbox(const gmx_domdec_t&            dd,
@@ -67,7 +69,8 @@ void set_ddbox(const gmx_domdec_t&            dd,
                gmx_ddbox_t*                   ddbox);
 
 /*! \brief Set the box and PBC data in \p ddbox */
-void set_ddbox_cr(const t_commrec&               cr,
+void set_ddbox_cr(DDRole                         ddRole,
+                  MPI_Comm                       communicator,
                   const ivec*                    dd_nc,
                   const t_inputrec&              ir,
                   const matrix                   box,

diff --git a/src/gromacs/domdec/domdec.cpp b/src/gromacs/domdec/domdec.cpp
index fac3bf9a27c7a0a964330f31fff3c2cf512f7c1f..ddb74cbc3d8b22b0fa95bc7f722deee82140fb52 100644 (file)
--- a/src/gromacs/domdec/domdec.cpp
+++ b/src/gromacs/domdec/domdec.cpp
@@ -1613,6 +1613,13 @@ static CartesianRankSetup makeGroupCommunicators(const gmx::MDLogger& mdlog,
 {
     CartesianRankSetup cartSetup;
 
+    // As a default, both group and sim communicators are equal to the default communicator
+    cr->mpi_comm_mygroup = cr->mpiDefaultCommunicator;
+    cr->mpi_comm_mysim   = cr->mpiDefaultCommunicator;
+    cr->nnodes           = cr->sizeOfDefaultCommunicator;
+    cr->nodeid           = cr->rankInDefaultCommunicator;
+    cr->sim_nodeid       = cr->rankInDefaultCommunicator;
+
     if (ddRankSetup.usePmeOnlyRanks)
     {
         /* Split the communicator into a PP and PME part */
@@ -1623,8 +1630,6 @@ static CartesianRankSetup makeGroupCommunicators(const gmx::MDLogger& mdlog,
     {
         /* All nodes do PP and PME */
         /* We do not require separate communicators */
-        cr->mpi_comm_mygroup = cr->mpi_comm_mysim;
-
         cartSetup.bCartesianPP     = false;
         cartSetup.bCartesianPP_PME = false;
     }
@@ -2044,7 +2049,8 @@ static bool moleculesAreAlwaysWhole(const gmx_mtop_t&
 
 /*! \brief Generate the simulation system information */
 static DDSystemInfo getSystemInfo(const gmx::MDLogger&           mdlog,
-                                  const t_commrec*               cr,
+                                  DDRole                         ddRole,
+                                  MPI_Comm                       communicator,
                                   const DomdecOptions&           options,
                                   const gmx_mtop_t&              mtop,
                                   const t_inputrec&              ir,
@@ -2155,13 +2161,13 @@ static DDSystemInfo getSystemInfo(const gmx::MDLogger&           mdlog,
         {
             real r_2b, r_mb;
 
-            if (MASTER(cr))
+            if (ddRole == DDRole::Master)
             {
                 dd_bonded_cg_distance(mdlog, &mtop, &ir, xGlobal, box,
                                       options.checkBondedInteractions, &r_2b, &r_mb);
             }
-            gmx_bcast(sizeof(r_2b), &r_2b, cr->mpi_comm_mygroup);
-            gmx_bcast(sizeof(r_mb), &r_mb, cr->mpi_comm_mygroup);
+            gmx_bcast(sizeof(r_2b), &r_2b, communicator);
+            gmx_bcast(sizeof(r_mb), &r_mb, communicator);
 
             /* We use an initial margin of 10% for the minimum cell size,
              * except when we are just below the non-bonded cut-off.
@@ -2235,7 +2241,9 @@ static DDSystemInfo getSystemInfo(const gmx::MDLogger&           mdlog,
 /*! \brief Exit with a fatal error if the DDGridSetup cannot be
  * implemented. */
 static void checkDDGridSetup(const DDGridSetup&   ddGridSetup,
-                             const t_commrec*     cr,
+                             DDRole               ddRole,
+                             MPI_Comm             communicator,
+                             int                  numNodes,
                              const DomdecOptions& options,
                              const DDSettings&    ddSettings,
                              const DDSystemInfo&  systemInfo,
@@ -2251,12 +2259,12 @@ static void checkDDGridSetup(const DDGridSetup&   ddGridSetup,
                 ddSettings.initialDlbState != DlbState::offUser ? " or -dds" : "",
                 bC ? " or your LINCS settings" : "");
 
-        gmx_fatal_collective(FARGS, cr->mpi_comm_mysim, MASTER(cr),
+        gmx_fatal_collective(FARGS, communicator, ddRole == DDRole::Master,
                              "There is no domain decomposition for %d ranks that is compatible "
                              "with the given box and a minimum cell size of %g nm\n"
                              "%s\n"
                              "Look in the log file for details on the domain decomposition",
-                             cr->nnodes - ddGridSetup.numPmeOnlyRanks, cellsizeLimit, buf);
+                             numNodes - ddGridSetup.numPmeOnlyRanks, cellsizeLimit, buf);
     }
 
     const real acs = average_cellsize_min(ddbox, ddGridSetup.numDomains);
@@ -2271,7 +2279,7 @@ static void checkDDGridSetup(const DDGridSetup&   ddGridSetup,
         else
         {
             gmx_fatal_collective(
-                    FARGS, cr->mpi_comm_mysim, MASTER(cr),
+                    FARGS, communicator, ddRole == DDRole::Master,
                     "The initial cell size (%f) is smaller than the cell size limit (%f), change "
                     "options -dd, -rdd or -rcon, see the log file for details",
                     acs, cellsizeLimit);
@@ -2280,16 +2288,16 @@ static void checkDDGridSetup(const DDGridSetup&   ddGridSetup,
 
     const int numPPRanks =
             ddGridSetup.numDomains[XX] * ddGridSetup.numDomains[YY] * ddGridSetup.numDomains[ZZ];
-    if (cr->nnodes - numPPRanks != ddGridSetup.numPmeOnlyRanks)
+    if (numNodes - numPPRanks != ddGridSetup.numPmeOnlyRanks)
     {
-        gmx_fatal_collective(FARGS, cr->mpi_comm_mysim, MASTER(cr),
+        gmx_fatal_collective(FARGS, communicator, ddRole == DDRole::Master,
                              "The size of the domain decomposition grid (%d) does not match the "
                              "number of PP ranks (%d). The total number of ranks is %d",
-                             numPPRanks, cr->nnodes - ddGridSetup.numPmeOnlyRanks, cr->nnodes);
+                             numPPRanks, numNodes - ddGridSetup.numPmeOnlyRanks, numNodes);
     }
     if (ddGridSetup.numPmeOnlyRanks > numPPRanks)
     {
-        gmx_fatal_collective(FARGS, cr->mpi_comm_mysim, MASTER(cr),
+        gmx_fatal_collective(FARGS, communicator, ddRole == DDRole::Master,
                              "The number of separate PME ranks (%d) is larger than the number of "
                              "PP ranks (%d), this is not supported.",
                              ddGridSetup.numPmeOnlyRanks, numPPRanks);
@@ -2298,7 +2306,7 @@ static void checkDDGridSetup(const DDGridSetup&   ddGridSetup,
 
 /*! \brief Set the cell size and interaction limits, as well as the DD grid */
 static DDRankSetup getDDRankSetup(const gmx::MDLogger& mdlog,
-                                  t_commrec*           cr,
+                                  int                  numNodes,
                                   const DDGridSetup&   ddGridSetup,
                                   const t_inputrec&    ir)
 {
@@ -2309,7 +2317,7 @@ static DDRankSetup getDDRankSetup(const gmx::MDLogger& mdlog,
 
     DDRankSetup ddRankSetup;
 
-    ddRankSetup.numPPRanks = cr->nnodes - ddGridSetup.numPmeOnlyRanks;
+    ddRankSetup.numPPRanks = numNodes - ddGridSetup.numPmeOnlyRanks;
     copy_ivec(ddGridSetup.numDomains, ddRankSetup.numPPCells);
 
     ddRankSetup.usePmeOnlyRanks = (ddGridSetup.numPmeOnlyRanks > 0);
@@ -2376,7 +2384,7 @@ static DDRankSetup getDDRankSetup(const gmx::MDLogger& mdlog,
 
 /*! \brief Set the cell size and interaction limits */
 static void set_dd_limits(const gmx::MDLogger& mdlog,
-                          t_commrec*           cr,
+                          DDRole               ddRole,
                           gmx_domdec_t*        dd,
                           const DomdecOptions& options,
                           const DDSettings&    ddSettings,
@@ -2485,7 +2493,7 @@ static void set_dd_limits(const gmx::MDLogger& mdlog,
                 gmx::boolToString(systemInfo.filterBondedCommunication), comm->cellsize_limit);
     }
 
-    if (MASTER(cr))
+    if (ddRole == DDRole::Master)
     {
         check_dd_restrictions(dd, ir, mdlog);
     }
@@ -2914,7 +2922,8 @@ gmx_domdec_t::gmx_domdec_t(const t_inputrec& ir) : unitCellInfo(ir) {}
  * in \c options.numPmeRanks.
  */
 static bool canMake1DDomainDecomposition(const DDSettings&              ddSettingsOriginal,
-                                         const t_commrec*               cr,
+                                         DDRole                         ddRole,
+                                         MPI_Comm                       communicator,
                                          const int                      numRanksRequested,
                                          const DomdecOptions&           options,
                                          const gmx_mtop_t&              mtop,
@@ -2925,7 +2934,8 @@ static bool canMake1DDomainDecomposition(const DDSettings&              ddSettin
     // Ensure we don't write any output from this checking routine
     gmx::MDLogger dummyLogger;
 
-    DDSystemInfo systemInfo = getSystemInfo(dummyLogger, cr, options, mtop, ir, box, xGlobal);
+    DDSystemInfo systemInfo =
+            getSystemInfo(dummyLogger, ddRole, communicator, options, mtop, ir, box, xGlobal);
 
     DDSettings ddSettings = ddSettingsOriginal;
     ddSettings.request1D  = true;
@@ -2934,8 +2944,8 @@ static bool canMake1DDomainDecomposition(const DDSettings&              ddSettin
                                         options.dlbScaling, ir, systemInfo.cellsizeLimit);
     gmx_ddbox_t ddbox = { 0 };
     DDGridSetup ddGridSetup =
-            getDDGridSetup(dummyLogger, cr, numRanksRequested, options, ddSettings, systemInfo,
-                           gridSetupCellsizeLimit, mtop, ir, box, xGlobal, &ddbox);
+            getDDGridSetup(dummyLogger, ddRole, communicator, numRanksRequested, options, ddSettings,
+                           systemInfo, gridSetupCellsizeLimit, mtop, ir, box, xGlobal, &ddbox);
 
     const bool canMake1DDD = (ddGridSetup.numDomains[XX] != 0);
 
@@ -3024,12 +3034,14 @@ DomainDecompositionBuilder::Impl::Impl(const MDLogger&      mdlog,
     mtop_(mtop),
     ir_(ir)
 {
-    GMX_LOG(mdlog_.info).appendTextFormatted("\nInitializing Domain Decomposition on %d ranks", cr_->nnodes);
+    GMX_LOG(mdlog_.info).appendTextFormatted("\nInitializing Domain Decomposition on %d ranks", cr_->sizeOfDefaultCommunicator);
 
     ddSettings_ = getDDSettings(mdlog_, options_, mdrunOptions, ir_);
 
     if (prefer1D
-        && canMake1DDomainDecomposition(ddSettings_, cr_, cr_->nnodes, options_, mtop_, ir_, box, xGlobal))
+        && canMake1DDomainDecomposition(ddSettings_, MASTER(cr_) ? DDRole::Master : DDRole::Agent,
+                                        cr->mpiDefaultCommunicator, cr_->sizeOfDefaultCommunicator,
+                                        options_, mtop_, ir_, box, xGlobal))
     {
         ddSettings_.request1D = true;
     }
@@ -3037,12 +3049,13 @@ DomainDecompositionBuilder::Impl::Impl(const MDLogger&      mdlog,
     if (ddSettings_.eFlop > 1)
     {
         /* Ensure that we have different random flop counts on different ranks */
-        srand(1 + cr_->nodeid);
+        srand(1 + cr_->rankInDefaultCommunicator);
     }
 
-    systemInfo_ = getSystemInfo(mdlog_, cr_, options_, mtop_, ir_, box, xGlobal);
+    systemInfo_ = getSystemInfo(mdlog_, MASTER(cr_) ? DDRole::Master : DDRole::Agent,
+                                cr->mpiDefaultCommunicator, options_, mtop_, ir_, box, xGlobal);
 
-    const int  numRanksRequested         = cr_->nnodes;
+    const int  numRanksRequested         = cr_->sizeOfDefaultCommunicator;
     const bool checkForLargePrimeFactors = (options_.numCells[0] <= 0);
     checkForValidRankCountRequests(numRanksRequested, EEL_PME(ir_.coulombtype),
                                    options_.numPmeRanks, checkForLargePrimeFactors);
@@ -3053,13 +3066,17 @@ DomainDecompositionBuilder::Impl::Impl(const MDLogger&      mdlog,
     const real gridSetupCellsizeLimit =
             getDDGridSetupCellSizeLimit(mdlog_, !isDlbDisabled(ddSettings_.initialDlbState),
                                         options_.dlbScaling, ir_, systemInfo_.cellsizeLimit);
-    ddGridSetup_ = getDDGridSetup(mdlog_, cr_, numRanksRequested, options_, ddSettings_, systemInfo_,
-                                  gridSetupCellsizeLimit, mtop_, ir_, box, xGlobal, &ddbox_);
-    checkDDGridSetup(ddGridSetup_, cr_, options_, ddSettings_, systemInfo_, gridSetupCellsizeLimit, ddbox_);
+    ddGridSetup_ =
+            getDDGridSetup(mdlog_, MASTER(cr_) ? DDRole::Master : DDRole::Agent,
+                           cr->mpiDefaultCommunicator, numRanksRequested, options_, ddSettings_,
+                           systemInfo_, gridSetupCellsizeLimit, mtop_, ir_, box, xGlobal, &ddbox_);
+    checkDDGridSetup(ddGridSetup_, MASTER(cr_) ? DDRole::Master : DDRole::Agent,
+                     cr->mpiDefaultCommunicator, cr->sizeOfDefaultCommunicator, options_,
+                     ddSettings_, systemInfo_, gridSetupCellsizeLimit, ddbox_);
 
     cr_->npmenodes = ddGridSetup_.numPmeOnlyRanks;
 
-    ddRankSetup_ = getDDRankSetup(mdlog_, cr_, ddGridSetup_, ir_);
+    ddRankSetup_ = getDDRankSetup(mdlog_, cr_->sizeOfDefaultCommunicator, ddGridSetup_, ir_);
 
     /* Generate the group communicator, also decides the duty of each rank */
     cartSetup_ = makeGroupCommunicators(mdlog_, ddSettings_, options_.rankOrder, ddRankSetup_, cr_,
@@ -3077,8 +3094,8 @@ gmx_domdec_t* DomainDecompositionBuilder::Impl::build(LocalAtomSetManager* atomS
     dd->comm->ddRankSetup        = ddRankSetup_;
     dd->comm->cartesianRankSetup = cartSetup_;
 
-    set_dd_limits(mdlog_, cr_, dd, options_, ddSettings_, systemInfo_, ddGridSetup_,
-                  ddRankSetup_.numPPRanks, &mtop_, &ir_, ddbox_);
+    set_dd_limits(mdlog_, MASTER(cr_) ? DDRole::Master : DDRole::Agent, dd, options_, ddSettings_,
+                  systemInfo_, ddGridSetup_, ddRankSetup_.numPPRanks, &mtop_, &ir_, ddbox_);
 
     setupGroupCommunication(mdlog_, ddSettings_, pmeRanks_, cr_, mtop_.natoms, dd);
 
@@ -3119,15 +3136,14 @@ DomainDecompositionBuilder::~DomainDecompositionBuilder() = default;
 
 } // namespace gmx
 
-static gmx_bool test_dd_cutoff(t_commrec* cr, const matrix box, gmx::ArrayRef<const gmx::RVec> x, real cutoffRequested)
+static gmx_bool test_dd_cutoff(const t_commrec* cr, const matrix box, gmx::ArrayRef<const gmx::RVec> x, real cutoffRequested)
 {
-    gmx_domdec_t* dd;
-    gmx_ddbox_t   ddbox;
-    int           d, dim, np;
-    real          inv_cell_size;
-    int           LocallyLimited;
+    gmx_ddbox_t ddbox;
+    int         d, dim, np;
+    real        inv_cell_size;
+    int         LocallyLimited;
 
-    dd = cr->dd;
+    const auto* dd = cr->dd;
 
     set_ddbox(*dd, false, box, true, x, &ddbox);
 

diff --git a/src/gromacs/domdec/domdec_setup.cpp b/src/gromacs/domdec/domdec_setup.cpp
index c22882f17b61d5a89d5ac123314ea03a52b2414c..d4c7dc342e4edef859419fe5bcaa47b45a8dda01 100644 (file)
--- a/src/gromacs/domdec/domdec_setup.cpp
+++ b/src/gromacs/domdec/domdec_setup.cpp
@@ -899,7 +899,8 @@ static int set_dd_dim(const gmx::IVec& numDDCells, const DDSettings& ddSettings,
 }
 
 DDGridSetup getDDGridSetup(const gmx::MDLogger&           mdlog,
-                           const t_commrec*               cr,
+                           DDRole                         ddRole,
+                           MPI_Comm                       communicator,
                            const int                      numRanksRequested,
                            const DomdecOptions&           options,
                            const DDSettings&              ddSettings,
@@ -926,13 +927,13 @@ DDGridSetup getDDGridSetup(const gmx::MDLogger&           mdlog,
     {
         numDomains                      = gmx::IVec(options.numCells);
         const ivec numDomainsLegacyIvec = { numDomains[XX], numDomains[YY], numDomains[ZZ] };
-        set_ddbox_cr(*cr, &numDomainsLegacyIvec, ir, box, xGlobal, ddbox);
+        set_ddbox_cr(ddRole, communicator, &numDomainsLegacyIvec, ir, box, xGlobal, ddbox);
     }
     else
     {
-        set_ddbox_cr(*cr, nullptr, ir, box, xGlobal, ddbox);
+        set_ddbox_cr(ddRole, communicator, nullptr, ir, box, xGlobal, ddbox);
 
-        if (MASTER(cr))
+        if (ddRole == DDRole::Master)
         {
             numDomains = optimizeDDCells(mdlog, numRanksRequested, numPmeOnlyRanks, cellSizeLimit,
                                          ddSettings.request1D, mtop, box, *ddbox, ir, systemInfo);
@@ -940,10 +941,10 @@ DDGridSetup getDDGridSetup(const gmx::MDLogger&           mdlog,
     }
 
     /* Communicate the information set by the master to all ranks */
-    gmx_bcast(sizeof(numDomains), numDomains, cr->mpi_comm_mygroup);
+    gmx_bcast(sizeof(numDomains), numDomains, communicator);
     if (EEL_PME(ir.coulombtype))
     {
-        gmx_bcast(sizeof(numPmeOnlyRanks), &numPmeOnlyRanks, cr->mpi_comm_mygroup);
+        gmx_bcast(sizeof(numPmeOnlyRanks), &numPmeOnlyRanks, communicator);
     }
 
     DDGridSetup ddGridSetup;

diff --git a/src/gromacs/domdec/domdec_setup.h b/src/gromacs/domdec/domdec_setup.h
index 6fbb50d283a76e92fa25920d884a39f9fff1e3a9..d982ed328a5d20b7e98e92a7fa51faecbeaae8fc 100644 (file)
--- a/src/gromacs/domdec/domdec_setup.h
+++ b/src/gromacs/domdec/domdec_setup.h
@@ -43,7 +43,9 @@
 #define GMX_DOMDEC_DOMDEC_SETUP_H
 
 #include "gromacs/math/vec.h"
+#include "gromacs/utility/gmxmpi.h"
 
+enum class DDRole;
 struct DDSettings;
 struct DDSystemInfo;
 struct gmx_ddbox_t;
@@ -104,7 +106,8 @@ real getDDGridSetupCellSizeLimit(const gmx::MDLogger& mdlog,
  * cell setup, DD cell size limits, and the initial ddbox.
  */
 DDGridSetup getDDGridSetup(const gmx::MDLogger&           mdlog,
-                           const t_commrec*               cr,
+                           DDRole                         ddRole,
+                           MPI_Comm                       communicator,
                            int                            numRanksRequested,
                            const gmx::DomdecOptions&      options,
                            const DDSettings&              ddSettings,

diff --git a/src/gromacs/fileio/checkpoint.cpp b/src/gromacs/fileio/checkpoint.cpp
index dba51ec23a469687df87f44bd5b32a6dfed530f8..f50b8182e0731a8fa330ccef9f2a7d160d565fcd 100644 (file)
--- a/src/gromacs/fileio/checkpoint.cpp
+++ b/src/gromacs/fileio/checkpoint.cpp
@@ -2545,11 +2545,12 @@ static void check_match(FILE*                           fplog,
         check_int(fplog, "#ranks", cr->nnodes, headerContents.nnodes, &mm);
     }
 
-    if (cr->nnodes > 1 && reproducibilityRequested)
+    if (cr->sizeOfDefaultCommunicator > 1 && reproducibilityRequested)
     {
+        // TODO: These checks are incorrect (see redmine #3309)
         check_int(fplog, "#PME-ranks", cr->npmenodes, headerContents.npme, &mm);
 
-        int npp = cr->nnodes;
+        int npp = cr->sizeOfDefaultCommunicator;
         if (cr->npmenodes >= 0)
         {
             npp -= cr->npmenodes;
@@ -2832,9 +2833,9 @@ void load_checkpoint(const char*                   fn,
     }
     if (PAR(cr))
     {
-        gmx_bcast(sizeof(headerContents.step), &headerContents.step, cr->mpi_comm_mygroup);
+        gmx_bcast(sizeof(headerContents.step), &headerContents.step, cr->mpiDefaultCommunicator);
         gmx::MdModulesCheckpointReadingBroadcast broadcastCheckPointData = {
-            cr->mpi_comm_mygroup, PAR(cr), headerContents.file_version
+            cr->mpiDefaultCommunicator, PAR(cr), headerContents.file_version
         };
         mdModulesNotifier.checkpointingNotifications_.notify(broadcastCheckPointData);
     }

diff --git a/src/gromacs/gmxlib/network.cpp b/src/gromacs/gmxlib/network.cpp
index 266ad3daddc319e34c8b3721798314cad28e7eca..8b50570a22b49db67e2d85c6a60b1f8fc9547ba1 100644 (file)
--- a/src/gromacs/gmxlib/network.cpp
+++ b/src/gromacs/gmxlib/network.cpp
@@ -86,21 +86,26 @@ CommrecHandle init_commrec(MPI_Comm communicator, const gmx_multisim_t* ms)
     if (ms != nullptr)
     {
 #if GMX_MPI
-        cr->nnodes = sizeOfCommunicator / ms->nsim;
-        MPI_Comm_split(communicator, ms->sim, rankInCommunicator, &cr->mpi_comm_mysim);
-        cr->mpi_comm_mygroup = cr->mpi_comm_mysim;
-        MPI_Comm_rank(cr->mpi_comm_mysim, &cr->sim_nodeid);
-        MPI_Comm_rank(cr->mpi_comm_mygroup, &cr->nodeid);
+        MPI_Comm_split(communicator, ms->sim, rankInCommunicator, &cr->mpiDefaultCommunicator);
+        cr->sizeOfDefaultCommunicator = sizeOfCommunicator / ms->nsim;
+        MPI_Comm_rank(cr->mpiDefaultCommunicator, &cr->rankInDefaultCommunicator);
+#else
+        gmx_fatal(FARGS, "Multisim can only run with MPI.");
 #endif
     }
     else
     {
-        cr->nnodes           = sizeOfCommunicator;
-        cr->nodeid           = rankInCommunicator;
-        cr->sim_nodeid       = cr->nodeid;
-        cr->mpi_comm_mysim   = communicator;
-        cr->mpi_comm_mygroup = communicator;
+        cr->mpiDefaultCommunicator    = communicator;
+        cr->sizeOfDefaultCommunicator = sizeOfCommunicator;
+        cr->rankInDefaultCommunicator = rankInCommunicator;
     }
+    // For now, we want things to go horribly wrong if this is used too early...
+    // TODO: Remove when communicators are removed from commrec (#2395)
+    cr->nnodes           = -1;
+    cr->nodeid           = -1;
+    cr->sim_nodeid       = -1;
+    cr->mpi_comm_mysim   = MPI_COMM_NULL;
+    cr->mpi_comm_mygroup = MPI_COMM_NULL;
 
     // TODO cr->duty should not be initialized here
     cr->duty = (DUTY_PP | DUTY_PME);

diff --git a/src/gromacs/mdlib/simulationsignal.cpp b/src/gromacs/mdlib/simulationsignal.cpp
index 415c91b0b6a9bbecce1d6ca5cf9f24641b2ee3da..15f44ccbf4cfadfec6671f09190c109e2c05e9d9 100644 (file)
--- a/src/gromacs/mdlib/simulationsignal.cpp
+++ b/src/gromacs/mdlib/simulationsignal.cpp
@@ -112,8 +112,11 @@ void SimulationSignaller::signalInterSim()
         // Communicate the signals between the simulations.
         gmx_sum_sim(eglsNR, mpiBuffer_.data(), ms_);
     }
-    // Communicate the signals from the master to the others.
-    gmx_bcast(eglsNR * sizeof(mpiBuffer_[0]), mpiBuffer_.data(), cr_->mpi_comm_mygroup);
+    if (DOMAINDECOMP(cr_))
+    {
+        // Communicate the signals from the master to the others.
+        gmx_bcast(eglsNR * sizeof(mpiBuffer_[0]), mpiBuffer_.data(), cr_->mpi_comm_mygroup);
+    }
 }
 
 void SimulationSignaller::setSignals()

diff --git a/src/gromacs/mdrun/runner.cpp b/src/gromacs/mdrun/runner.cpp
index 71accb8036a8631ecf33713f4050073dccfb5a8b..2e502f25dce2e1df7e46413b26fd9c5e1ffb2fd5 100644 (file)
--- a/src/gromacs/mdrun/runner.cpp
+++ b/src/gromacs/mdrun/runner.cpp
@@ -844,7 +844,8 @@ int Mdrunner::mdrunner()
         {
             inputrec = &inputrecInstance;
         }
-        init_parallel(cr->mpi_comm_mygroup, MASTER(cr), inputrec, &mtop, partialDeserializedTpr.get());
+        init_parallel(cr->mpiDefaultCommunicator, MASTER(cr), inputrec, &mtop,
+                      partialDeserializedTpr.get());
     }
     GMX_RELEASE_ASSERT(inputrec != nullptr, "All ranks should have a valid inputrec now");
     partialDeserializedTpr.reset(nullptr);
@@ -877,7 +878,7 @@ int Mdrunner::mdrunner()
                 gpuAccelerationOfNonbondedIsUseful(mdlog, *inputrec, !GMX_THREAD_MPI), gpusWereDetected);
         useGpuForPme = decideWhetherToUseGpusForPme(
                 useGpuForNonbonded, pmeTarget, userGpuTaskAssignment, *hwinfo, *inputrec, mtop,
-                cr->nnodes, domdecOptions.numPmeRanks, gpusWereDetected);
+                cr->sizeOfDefaultCommunicator, domdecOptions.numPmeRanks, gpusWereDetected);
         auto canUseGpuForBonded = buildSupportsGpuBondeds(nullptr)
                                   && inputSupportsGpuBondeds(*inputrec, mtop, nullptr);
         useGpuForBonded = decideWhetherToUseGpusForBonded(
@@ -958,7 +959,8 @@ int Mdrunner::mdrunner()
         {
             globalState = std::make_unique<t_state>();
         }
-        broadcastStateWithoutDynamics(cr->mpi_comm_mygroup, DOMAINDECOMP(cr), PAR(cr), globalState.get());
+        broadcastStateWithoutDynamics(cr->mpiDefaultCommunicator, DOMAINDECOMP(cr), PAR(cr),
+                                      globalState.get());
     }
 
     /* A parallel command line option consistency check that we can
@@ -992,7 +994,7 @@ int Mdrunner::mdrunner()
     {
         if (domdecOptions.numPmeRanks > 0)
         {
-            gmx_fatal_collective(FARGS, cr->mpi_comm_mysim, MASTER(cr),
+            gmx_fatal_collective(FARGS, cr->mpiDefaultCommunicator, MASTER(cr),
                                  "PME-only ranks are requested, but the system does not use PME "
                                  "for electrostatics or LJ");
         }
@@ -1102,7 +1104,7 @@ int Mdrunner::mdrunner()
 
     if (PAR(cr))
     {
-        gmx_bcast(sizeof(box), box, cr->mpi_comm_mygroup);
+        gmx_bcast(sizeof(box), box, cr->mpiDefaultCommunicator);
     }
 
     if (inputrec->cutoff_scheme != ecutsVERLET)
@@ -1112,6 +1114,10 @@ int Mdrunner::mdrunner()
                   "Verlet scheme, or use an earlier version of GROMACS if necessary.");
     }
     /* Update rlist and nstlist. */
+    /* Note: prepare_verlet_scheme is calling increaseNstlist(...), which (while attempting to
+     * increase rlist) tries to check if the newly chosen value fits with the DD scheme. As this is
+     * run before any DD scheme is set up, this check is never executed. See #3334 for more details.
+     */
     prepare_verlet_scheme(fplog, cr, inputrec, nstlist_cmdline, &mtop, box,
                           useGpuForNonbonded || (emulateGpuNonbonded == EmulateGpuNonbonded::Yes),
                           *hwinfo->cpuInfo);
@@ -1131,8 +1137,11 @@ int Mdrunner::mdrunner()
     else
     {
         /* PME, if used, is done on all nodes with 1D decomposition */
-        cr->npmenodes = 0;
-        cr->duty      = (DUTY_PP | DUTY_PME);
+        cr->nnodes     = cr->sizeOfDefaultCommunicator;
+        cr->sim_nodeid = cr->rankInDefaultCommunicator;
+        cr->nodeid     = cr->rankInDefaultCommunicator;
+        cr->npmenodes  = 0;
+        cr->duty       = (DUTY_PP | DUTY_PME);
 
         if (inputrec->pbcType == PbcType::Screw)
         {

diff --git a/src/gromacs/mdrunutility/tests/threadaffinitytest.cpp b/src/gromacs/mdrunutility/tests/threadaffinitytest.cpp
index 429ac458f36091bd2ae12fb61db8c3ada7557e46..56b05ae9c3ca795a42874df2d251fd0407f2c71a 100644 (file)
--- a/src/gromacs/mdrunutility/tests/threadaffinitytest.cpp
+++ b/src/gromacs/mdrunutility/tests/threadaffinitytest.cpp
@@ -1,7 +1,7 @@
 /*
  * This file is part of the GROMACS molecular simulation package.
  *
- * Copyright (c) 2016,2017,2018,2019, by the GROMACS development team, led by
+ * Copyright (c) 2016,2017,2018,2019,2020, by the GROMACS development team, led by
  * Mark Abraham, David van der Spoel, Berk Hess, and Erik Lindahl,
  * and including many others, as listed in the AUTHORS file in the
  * top-level source directory and at http://www.gromacs.org.
@@ -70,7 +70,12 @@ ThreadAffinityTestHelper::ThreadAffinityTestHelper()
     snew(cr_, 1);
     cr_->nnodes = gmx_node_num();
     cr_->nodeid = gmx_node_rank();
-    cr_->duty   = DUTY_PP;
+    // Default communicator is needed for [SIM]MASTER(cr) to work
+    // TODO: Should get cleaned up once thread affinity works with
+    //       communicators rather than the full cr (part of #2395)
+    cr_->sizeOfDefaultCommunicator = gmx_node_num();
+    cr_->rankInDefaultCommunicator = gmx_node_rank();
+    cr_->duty                      = DUTY_PP;
 #if GMX_MPI
     cr_->mpi_comm_mysim = MPI_COMM_WORLD;
 #endif

diff --git a/src/gromacs/mdtypes/commrec.h b/src/gromacs/mdtypes/commrec.h
index 804b9043d17c856bf5e28749397c3f011caad458..bef393da4275ac826234a620f896c330deb5a4d7 100644 (file)
--- a/src/gromacs/mdtypes/commrec.h
+++ b/src/gromacs/mdtypes/commrec.h
@@ -93,6 +93,10 @@ struct t_commrec
                                   a single simulation */
     MPI_Comm mpi_comm_mygroup; /* subset of mpi_comm_mysim including only
                                   the ranks in the same group (PP or PME) */
+    //! The communicator used before DD was initialized
+    MPI_Comm mpiDefaultCommunicator;
+    int      sizeOfDefaultCommunicator;
+    int      rankInDefaultCommunicator;
 
     gmx_nodecomm_t nc;
 
@@ -139,11 +143,12 @@ inline bool thisRankHasDuty(const t_commrec* cr, int duty)
  * In particular, this is true for multi-rank runs with TPI and NM, because
  * they use a decomposition that is not the domain decomposition used by
  * other simulation types. */
-#define PAR(cr) ((cr)->nnodes > 1)
+#define PAR(cr) ((cr)->sizeOfDefaultCommunicator > 1)
 
 //! True of this is the master node
-#define MASTER(cr) (((cr)->nodeid == 0) || !PAR(cr))
+#define MASTER(cr) (((cr)->rankInDefaultCommunicator == 0) || !PAR(cr))
 
+// Note that currently, master is always PP master, so this is equivalent to MASTER(cr)
 //! True if this is the particle-particle master
 #define SIMMASTER(cr) ((MASTER(cr) && thisRankHasDuty((cr), DUTY_PP)) || !PAR(cr))
 

diff --git a/src/gromacs/nbnxm/pairlist_tuning.cpp b/src/gromacs/nbnxm/pairlist_tuning.cpp
index c1757d87a8d5c179ac286fe7f2c11a62880b9161..e4c90428c2ed70705a7284655dc6851b0afcbe0d 100644 (file)
--- a/src/gromacs/nbnxm/pairlist_tuning.cpp
+++ b/src/gromacs/nbnxm/pairlist_tuning.cpp
@@ -284,6 +284,11 @@ void increaseNstlist(FILE*               fp,
         bDD  = TRUE;
         if (bBox && DOMAINDECOMP(cr))
         {
+            /* Currently (as of July 2020), the code in this if clause is never executed.
+             * increaseNstlist(...) is only called from prepare_verlet_scheme, which in turns
+             * gets called by the runner _before_ setting up DD. DOMAINDECOMP(cr) will therefore
+             * always be false here. See #3334.
+             */
             /* Check if rlist fits in the domain decomposition */
             if (inputrec2nboundeddim(ir) < DIM)
             {