Clean up multisim, communication, and REMD code
authorMark Abraham <mark.j.abraham@gmail.com>
Sun, 26 May 2019 22:00:46 +0000 (00:00 +0200)
committerMark Abraham <mark.j.abraham@gmail.com>
Tue, 11 Jun 2019 16:03:47 +0000 (18:03 +0200)
Moved multisim declarations and code to one place.

Moved mpi_in_place_buf_t to its own files in a low-level module, and
made sure we tested a build that needed it. Removed duplicate tng
option declaration while I was there.

Moved a check for REMD to its code, removed another that was redundant
with that for multisim requiring more than one simulation.

Added some t_commrec and gmx_multisim_t cleanup code, but not all
of it can work before some more cleanup takes place.

Change-Id: Ie591f50a2162dce2e83552102f8633c8d49553a7

16 files changed:
admin/builds/gromacs.py
admin/builds/pre-submit-matrix.txt
src/gromacs/awh/biassharing.cpp
src/gromacs/awh/biasstate.cpp
src/gromacs/gmxlib/network.cpp
src/gromacs/gmxlib/network.h
src/gromacs/listed_forces/disre.cpp
src/gromacs/mdlib/md_support.cpp
src/gromacs/mdlib/tests/constr.cpp
src/gromacs/mdrun/legacymdrunoptions.cpp
src/gromacs/mdrun/replicaexchange.cpp
src/gromacs/mdrunutility/multisim.cpp
src/gromacs/mdrunutility/multisim.h
src/gromacs/mdtypes/commrec.h
src/gromacs/utility/mpiinplacebuffers.cpp [new file with mode: 0644]
src/gromacs/utility/mpiinplacebuffers.h [new file with mode: 0644]

index 4e265d75eedfc35a8195cde955f2a18eb78ecb3c..d734dae568a47cc28ee73d036887c30faead5d13 100644 (file)
@@ -59,7 +59,7 @@ extra_options = {
     'npme': Option.string,
     'gpu_id': Option.string,
     'hwloc': Option.bool,
-    'tng': Option.bool
+    'mpiinplace': Option.bool
 }
 
 extra_projects = [Project.REGRESSIONTESTS]
@@ -117,6 +117,8 @@ def do_build(context):
         cmake_opts['GMX_THREAD_MPI'] = 'OFF'
     if context.opts.mpi:
         cmake_opts['GMX_MPI'] = 'ON'
+    if context.opts.mpiinplace is False:
+        cmake_opts['GMX_MPI_IN_PLACE'] = 'OFF'
     if context.opts.openmp is False:
         cmake_opts['GMX_OPENMP'] = 'OFF'
     if context.opts.tng is False:
index 1b68e3bf138e04eac2b394ba19e566a0a4031c78..79b4471bf3da7bb47acff368594e5451268bb17c 100644 (file)
@@ -71,7 +71,8 @@ icc-19 no-thread-mpi double openmp mkl cmake-3.9.6 simd=none release hwloc
 # Test external clFFT (for build speed)
 # Test MPI + OpenCL
 # Test AVX2_256 SIMD
-gcc-6 openmp gpuhw=nvidia opencl-1.2 clFFT-2.14 cmake-3.9.6 mpi simd=avx2_256
+# Test without MPI_IN_PLACE
+gcc-6 openmp gpuhw=nvidia opencl-1.2 clFFT-2.14 cmake-3.9.6 mpi no-mpiinplace simd=avx2_256
 
 # Test AMD OpenCL
 # Test external clFFT (for build speed)
index ca752a9845a5fdb84a60b34e72272cfbd63e538b..dbf7a40ab81918b3f1cf0b5a011f797d130d38de 100644 (file)
@@ -48,6 +48,7 @@
 #include <vector>
 
 #include "gromacs/gmxlib/network.h"
+#include "gromacs/mdrunutility/multisim.h"
 #include "gromacs/mdtypes/awh_params.h"
 #include "gromacs/mdtypes/commrec.h"
 #include "gromacs/utility/exceptions.h"
index 8145e4d5e6e4cfefe16a54cfb28f5381819634d8..7247a260eb316fa9301dc26d11a91ec1d351769e 100644 (file)
@@ -58,6 +58,7 @@
 #include "gromacs/fileio/xvgr.h"
 #include "gromacs/gmxlib/network.h"
 #include "gromacs/math/utilities.h"
+#include "gromacs/mdrunutility/multisim.h"
 #include "gromacs/mdtypes/awh_history.h"
 #include "gromacs/mdtypes/awh_params.h"
 #include "gromacs/mdtypes/commrec.h"
index 3acd596aca74c9727223d135ba3935d5b35adce5..945c6cecb616a71af24c0b0b777f7015d9955842 100644 (file)
@@ -3,7 +3,7 @@
  *
  * Copyright (c) 1991-2000, University of Groningen, The Netherlands.
  * Copyright (c) 2001-2004, The GROMACS development team.
- * Copyright (c) 2013,2014,2015,2016,2017,2018, by the GROMACS development team, led by
+ * Copyright (c) 2013,2014,2015,2016,2017,2018,2019, 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.
@@ -52,6 +52,8 @@
 #include "gromacs/utility/fatalerror.h"
 #include "gromacs/utility/futil.h"
 #include "gromacs/utility/gmxmpi.h"
+#include "gromacs/utility/mpiinplacebuffers.h"
+#include "gromacs/utility/real.h"
 #include "gromacs/utility/smalloc.h"
 
 /* The source code in this file should be thread-safe.
@@ -98,10 +100,10 @@ t_commrec *init_commrec()
 #if GMX_MPI && !MPI_IN_PLACE_EXISTS
     /* initialize the MPI_IN_PLACE replacement buffers */
     snew(cr->mpb, 1);
-    cr->mpb->ibuf        = NULL;
-    cr->mpb->libuf       = NULL;
-    cr->mpb->fbuf        = NULL;
-    cr->mpb->dbuf        = NULL;
+    cr->mpb->ibuf        = nullptr;
+    cr->mpb->libuf       = nullptr;
+    cr->mpb->fbuf        = nullptr;
+    cr->mpb->dbuf        = nullptr;
     cr->mpb->ibuf_alloc  = 0;
     cr->mpb->libuf_alloc = 0;
     cr->mpb->fbuf_alloc  = 0;
@@ -111,18 +113,6 @@ t_commrec *init_commrec()
     return cr;
 }
 
-void done_mpi_in_place_buf(mpi_in_place_buf_t *buf)
-{
-    if (nullptr != buf)
-    {
-        sfree(buf->ibuf);
-        sfree(buf->libuf);
-        sfree(buf->fbuf);
-        sfree(buf->dbuf);
-        sfree(buf);
-    }
-}
-
 void done_commrec(t_commrec *cr)
 {
     if (MASTER(cr))
@@ -134,6 +124,25 @@ void done_commrec(t_commrec *cr)
         }
         done_mpi_in_place_buf(cr->mpb);
     }
+#if GMX_MPI
+    // TODO We need to be able to free communicators, but the
+    // structure of the commrec and domdec initialization code makes
+    // it hard to avoid both leaks and double frees.
+    bool mySimIsMyGroup = (cr->mpi_comm_mysim == cr->mpi_comm_mygroup);
+    if (cr->mpi_comm_mysim != MPI_COMM_NULL &&
+        cr->mpi_comm_mysim != MPI_COMM_WORLD)
+    {
+        // TODO see above
+        // MPI_Comm_free(&cr->mpi_comm_mysim);
+    }
+    if (!mySimIsMyGroup &&
+        cr->mpi_comm_mygroup != MPI_COMM_NULL &&
+        cr->mpi_comm_mygroup != MPI_COMM_WORLD)
+    {
+        // TODO see above
+        // MPI_Comm_free(&cr->mpi_comm_mygroup);
+    }
+#endif
     sfree(cr);
 }
 
@@ -524,233 +533,6 @@ void gmx_sumli(int gmx_unused nr, int64_t gmx_unused r[], const t_commrec gmx_un
 #endif
 }
 
-
-
-#if GMX_MPI
-static void gmx_sumd_comm(int nr, double r[], MPI_Comm mpi_comm)
-{
-#if MPI_IN_PLACE_EXISTS
-    MPI_Allreduce(MPI_IN_PLACE, r, nr, MPI_DOUBLE, MPI_SUM, mpi_comm);
-#else
-    /* this function is only used in code that is not performance critical,
-       (during setup, when comm_rec is not the appropriate communication
-       structure), so this isn't as bad as it looks. */
-    double *buf;
-    int     i;
-
-    snew(buf, nr);
-    MPI_Allreduce(r, buf, nr, MPI_DOUBLE, MPI_SUM, mpi_comm);
-    for (i = 0; i < nr; i++)
-    {
-        r[i] = buf[i];
-    }
-    sfree(buf);
-#endif
-}
-#endif
-
-#if GMX_MPI
-static void gmx_sumf_comm(int nr, float r[], MPI_Comm mpi_comm)
-{
-#if MPI_IN_PLACE_EXISTS
-    MPI_Allreduce(MPI_IN_PLACE, r, nr, MPI_FLOAT, MPI_SUM, mpi_comm);
-#else
-    /* this function is only used in code that is not performance critical,
-       (during setup, when comm_rec is not the appropriate communication
-       structure), so this isn't as bad as it looks. */
-    float *buf;
-    int    i;
-
-    snew(buf, nr);
-    MPI_Allreduce(r, buf, nr, MPI_FLOAT, MPI_SUM, mpi_comm);
-    for (i = 0; i < nr; i++)
-    {
-        r[i] = buf[i];
-    }
-    sfree(buf);
-#endif
-}
-#endif
-
-void gmx_sumd_sim(int gmx_unused nr, double gmx_unused r[], const gmx_multisim_t gmx_unused *ms)
-{
-#if !GMX_MPI
-    gmx_call("gmx_sumd_sim");
-#else
-    gmx_sumd_comm(nr, r, ms->mpi_comm_masters);
-#endif
-}
-
-void gmx_sumf_sim(int gmx_unused nr, float gmx_unused r[], const gmx_multisim_t gmx_unused *ms)
-{
-#if !GMX_MPI
-    gmx_call("gmx_sumf_sim");
-#else
-    gmx_sumf_comm(nr, r, ms->mpi_comm_masters);
-#endif
-}
-
-void gmx_sumi_sim(int gmx_unused nr, int gmx_unused r[], const gmx_multisim_t gmx_unused *ms)
-{
-#if !GMX_MPI
-    gmx_call("gmx_sumi_sim");
-#else
-#if MPI_IN_PLACE_EXISTS
-    MPI_Allreduce(MPI_IN_PLACE, r, nr, MPI_INT, MPI_SUM, ms->mpi_comm_masters);
-#else
-    /* this is thread-unsafe, but it will do for now: */
-    int i;
-
-    if (nr > ms->mpb->ibuf_alloc)
-    {
-        ms->mpb->ibuf_alloc = nr;
-        srenew(ms->mpb->ibuf, ms->mpb->ibuf_alloc);
-    }
-    MPI_Allreduce(r, ms->mpb->ibuf, nr, MPI_INT, MPI_SUM, ms->mpi_comm_masters);
-    for (i = 0; i < nr; i++)
-    {
-        r[i] = ms->mpb->ibuf[i];
-    }
-#endif
-#endif
-}
-
-void gmx_sumli_sim(int gmx_unused nr, int64_t gmx_unused r[], const gmx_multisim_t gmx_unused *ms)
-{
-#if !GMX_MPI
-    gmx_call("gmx_sumli_sim");
-#else
-#if MPI_IN_PLACE_EXISTS
-    MPI_Allreduce(MPI_IN_PLACE, r, nr, MPI_INT64_T, MPI_SUM,
-                  ms->mpi_comm_masters);
-#else
-    /* this is thread-unsafe, but it will do for now: */
-    int i;
-
-    if (nr > ms->mpb->libuf_alloc)
-    {
-        ms->mpb->libuf_alloc = nr;
-        srenew(ms->mpb->libuf, ms->mpb->libuf_alloc);
-    }
-    MPI_Allreduce(r, ms->mpb->libuf, nr, MPI_INT64_T, MPI_SUM,
-                  ms->mpi_comm_masters);
-    for (i = 0; i < nr; i++)
-    {
-        r[i] = ms->mpb->libuf[i];
-    }
-#endif
-#endif
-}
-
-void check_multi_int(FILE *log, const gmx_multisim_t *ms, int val,
-                     const char *name,
-                     gmx_bool bQuiet)
-{
-    int     *ibuf, p;
-    gmx_bool bCompatible;
-
-    if (nullptr != log && !bQuiet)
-    {
-        fprintf(log, "Multi-checking %s ... ", name);
-    }
-
-    if (ms == nullptr)
-    {
-        gmx_fatal(FARGS,
-                  "check_multi_int called with a NULL communication pointer");
-    }
-
-    snew(ibuf, ms->nsim);
-    ibuf[ms->sim] = val;
-    gmx_sumi_sim(ms->nsim, ibuf, ms);
-
-    bCompatible = TRUE;
-    for (p = 1; p < ms->nsim; p++)
-    {
-        bCompatible = bCompatible && (ibuf[p-1] == ibuf[p]);
-    }
-
-    if (bCompatible)
-    {
-        if (nullptr != log && !bQuiet)
-        {
-            fprintf(log, "OK\n");
-        }
-    }
-    else
-    {
-        if (nullptr != log)
-        {
-            fprintf(log, "\n%s is not equal for all subsystems\n", name);
-            for (p = 0; p < ms->nsim; p++)
-            {
-                fprintf(log, "  subsystem %d: %d\n", p, ibuf[p]);
-            }
-        }
-        gmx_fatal(FARGS, "The %d subsystems are not compatible\n", ms->nsim);
-    }
-
-    sfree(ibuf);
-}
-
-void check_multi_int64(FILE *log, const gmx_multisim_t *ms,
-                       int64_t val, const char *name,
-                       gmx_bool bQuiet)
-{
-    int64_t          *ibuf;
-    int               p;
-    gmx_bool          bCompatible;
-
-    if (nullptr != log && !bQuiet)
-    {
-        fprintf(log, "Multi-checking %s ... ", name);
-    }
-
-    if (ms == nullptr)
-    {
-        gmx_fatal(FARGS,
-                  "check_multi_int called with a NULL communication pointer");
-    }
-
-    snew(ibuf, ms->nsim);
-    ibuf[ms->sim] = val;
-    gmx_sumli_sim(ms->nsim, ibuf, ms);
-
-    bCompatible = TRUE;
-    for (p = 1; p < ms->nsim; p++)
-    {
-        bCompatible = bCompatible && (ibuf[p-1] == ibuf[p]);
-    }
-
-    if (bCompatible)
-    {
-        if (nullptr != log && !bQuiet)
-        {
-            fprintf(log, "OK\n");
-        }
-    }
-    else
-    {
-        // TODO Part of this error message would also be good to go to
-        // stderr (from one rank of one sim only)
-        if (nullptr != log)
-        {
-            fprintf(log, "\n%s is not equal for all subsystems\n", name);
-            for (p = 0; p < ms->nsim; p++)
-            {
-                char strbuf[255];
-                /* first make the format string */
-                snprintf(strbuf, 255, "  subsystem %%d: %s\n",
-                         "%" PRId64);
-                fprintf(log, strbuf, p, ibuf[p]);
-            }
-        }
-        gmx_fatal(FARGS, "The %d subsystems are not compatible\n", ms->nsim);
-    }
-
-    sfree(ibuf);
-}
-
 const char *opt2fn_master(const char *opt, int nfile, const t_filenm fnm[],
                           t_commrec *cr)
 {
index 5919aff4373df4beedeea4c6c389b83e0321e2e7..a6fc31d52f3c347818c8fb66d55124ffee1f1bd7 100644 (file)
@@ -3,7 +3,7 @@
  *
  * Copyright (c) 1991-2000, University of Groningen, The Netherlands.
  * Copyright (c) 2001-2004, The GROMACS development team.
- * Copyright (c) 2013,2014,2015,2016,2017,2018, by the GROMACS development team, led by
+ * Copyright (c) 2013,2014,2015,2016,2017,2018,2019, 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.
@@ -47,7 +47,6 @@
 #include "gromacs/utility/gmxmpi.h"
 #include "gromacs/utility/stringutil.h"
 
-struct gmx_multisim_t;
 struct t_commrec;
 struct t_filenm;
 
@@ -92,36 +91,10 @@ void gmx_sumf(int nr, float r[], const struct t_commrec *cr);
 void gmx_sumd(int nr, double r[], const struct t_commrec *cr);
 /* Calculate the global sum of an array of doubles */
 
-void gmx_sumi_sim(int nr, int r[], const struct gmx_multisim_t *ms);
-/* Calculate the sum over the simulations of an array of ints */
-
-void gmx_sumli_sim(int nr, int64_t r[], const struct gmx_multisim_t *ms);
-/* Calculate the sum over the simulations of an array of large ints */
-
-void gmx_sumf_sim(int nr, float r[], const struct gmx_multisim_t *ms);
-/* Calculate the sum over the simulations of an array of floats */
-
-void gmx_sumd_sim(int nr, double r[], const struct gmx_multisim_t *ms);
-/* Calculate the sum over the simulations of an array of doubles */
-
-void check_multi_int(FILE *log, const gmx_multisim_t *ms,
-                     int val, const char *name,
-                     gmx_bool bQuiet);
-void check_multi_int64(FILE *log, const gmx_multisim_t *ms,
-                       int64_t val, const char *name,
-                       gmx_bool bQuiet);
-/* Check if val is the same on all processors for a mdrun -multidir run
- * The string name is used to print to the log file and in a fatal error
- * if the val's don't match. If bQuiet is true and the check passes,
- * no output is written.
- */
-
 #if GMX_DOUBLE
 #define gmx_sum       gmx_sumd
-#define gmx_sum_sim   gmx_sumd_sim
 #else
 #define gmx_sum       gmx_sumf
-#define gmx_sum_sim   gmx_sumf_sim
 #endif
 
 const char *opt2fn_master(const char *opt, int nfile,
index f827ada753d79784347a34b6f590b7bc45ddbc60..841cc0d9e43149c3e289b85dddc73ca98de332b5 100644 (file)
@@ -50,6 +50,7 @@
 #include "gromacs/gmxlib/network.h"
 #include "gromacs/math/functions.h"
 #include "gromacs/math/vec.h"
+#include "gromacs/mdrunutility/multisim.h"
 #include "gromacs/mdtypes/commrec.h"
 #include "gromacs/mdtypes/fcdata.h"
 #include "gromacs/mdtypes/inputrec.h"
index 9426748fcd5f114946a4d499ed1ce9239fa63c6b..3fd69ace3cabf1cc8d52d41be06b95795384f055 100644 (file)
@@ -54,6 +54,7 @@
 #include "gromacs/mdlib/tgroup.h"
 #include "gromacs/mdlib/update.h"
 #include "gromacs/mdlib/vcm.h"
+#include "gromacs/mdrunutility/multisim.h"
 #include "gromacs/mdtypes/commrec.h"
 #include "gromacs/mdtypes/df_history.h"
 #include "gromacs/mdtypes/enerdata.h"
index 983958cb507a4978d9618199f93bdb65caa3b112..e922d266aacedf519db2c3c4fe15f4aab8069610 100644 (file)
@@ -72,6 +72,7 @@
 #include "gromacs/mdlib/lincs.h"
 #include "gromacs/mdlib/lincs_cuda.h"
 #include "gromacs/mdlib/shake.h"
+#include "gromacs/mdrunutility/multisim.h"
 #include "gromacs/mdtypes/commrec.h"
 #include "gromacs/mdtypes/inputrec.h"
 #include "gromacs/mdtypes/mdatom.h"
index 21edb5c9bf445ce5c77a26104d67b04b281ae236..d773b35312104f2f006b0258b51256675a24ca86 100644 (file)
@@ -143,16 +143,6 @@ int LegacyMdrunOptions::updateFromCommandLine(int argc, char **argv, ArrayRef<co
                                                               ssize(filenames),
                                                               filenames.data());
 
-    if (replExParams.exchangeInterval != 0 && multidir.size() < 2)
-    {
-        gmx_fatal(FARGS, "Need at least two replicas for replica exchange (use option -multidir)");
-    }
-
-    if (replExParams.numExchanges < 0)
-    {
-        gmx_fatal(FARGS, "Replica exchange number of exchanges needs to be positive");
-    }
-
     ms = init_multisystem(MPI_COMM_WORLD, multidir);
 
     /* Prepare the intra-simulation communication */
index b80d59e44c4405f40b209d213463034af1c303ef..9ff7fa478b3ec93add58a617a34824f1ad68e818 100644 (file)
@@ -208,6 +208,11 @@ init_replica_exchange(FILE                            *fplog,
     {
         gmx_fatal(FARGS, "Nothing to exchange with only one replica, maybe you forgot to set the -multidir option of mdrun?");
     }
+    if (replExParams.numExchanges < 0)
+    {
+        gmx_fatal(FARGS, "Replica exchange number of exchanges needs to be positive");
+    }
+
     if (!EI_DYNAMICS(ir->eI))
     {
         gmx_fatal(FARGS, "Replica exchange is only supported by dynamical simulations");
index a74dfc0c1b22739258e27fca5f958afecbdd4739..7a6d16eddaa184148e15744bca53d2368baa517b 100644 (file)
@@ -49,6 +49,7 @@
 #include "gromacs/mdtypes/commrec.h"
 #include "gromacs/utility/fatalerror.h"
 #include "gromacs/utility/futil.h"
+#include "gromacs/utility/mpiinplacebuffers.h"
 #include "gromacs/utility/smalloc.h"
 
 gmx_multisim_t *init_multisystem(MPI_Comm                         comm,
@@ -113,10 +114,10 @@ gmx_multisim_t *init_multisystem(MPI_Comm                         comm,
 #if !MPI_IN_PLACE_EXISTS
     /* initialize the MPI_IN_PLACE replacement buffers */
     snew(ms->mpb, 1);
-    ms->mpb->ibuf        = NULL;
-    ms->mpb->libuf       = NULL;
-    ms->mpb->fbuf        = NULL;
-    ms->mpb->dbuf        = NULL;
+    ms->mpb->ibuf        = nullptr;
+    ms->mpb->libuf       = nullptr;
+    ms->mpb->fbuf        = nullptr;
+    ms->mpb->dbuf        = nullptr;
     ms->mpb->ibuf_alloc  = 0;
     ms->mpb->libuf_alloc = 0;
     ms->mpb->fbuf_alloc  = 0;
@@ -135,11 +136,251 @@ gmx_multisim_t *init_multisystem(MPI_Comm                         comm,
 
 void done_multisim(gmx_multisim_t *ms)
 {
-    if (nullptr != ms)
+    if (ms == nullptr)
     {
-        done_mpi_in_place_buf(ms->mpb);
-        delete ms;
+        return;
     }
+    done_mpi_in_place_buf(ms->mpb);
+
+#if GMX_MPI
+    // TODO This would work better if the result of MPI_Comm_split was
+    // put into an RAII-style guard, such as gmx::unique_cptr.
+    if (ms->mpi_comm_masters != MPI_COMM_NULL &&
+        ms->mpi_comm_masters != MPI_COMM_WORLD)
+    {
+        MPI_Comm_free(&ms->mpi_comm_masters);
+    }
+    if (ms->mpi_group_masters != MPI_GROUP_NULL)
+    {
+        MPI_Group_free(&ms->mpi_group_masters);
+    }
+#endif
+    delete ms;
+}
+
+#if GMX_MPI
+static void gmx_sumd_comm(int nr, double r[], MPI_Comm mpi_comm)
+{
+#if MPI_IN_PLACE_EXISTS
+    MPI_Allreduce(MPI_IN_PLACE, r, nr, MPI_DOUBLE, MPI_SUM, mpi_comm);
+#else
+    /* this function is only used in code that is not performance critical,
+       (during setup, when comm_rec is not the appropriate communication
+       structure), so this isn't as bad as it looks. */
+    double *buf;
+    int     i;
+
+    snew(buf, nr);
+    MPI_Allreduce(r, buf, nr, MPI_DOUBLE, MPI_SUM, mpi_comm);
+    for (i = 0; i < nr; i++)
+    {
+        r[i] = buf[i];
+    }
+    sfree(buf);
+#endif
+}
+#endif
+
+#if GMX_MPI
+static void gmx_sumf_comm(int nr, float r[], MPI_Comm mpi_comm)
+{
+#if MPI_IN_PLACE_EXISTS
+    MPI_Allreduce(MPI_IN_PLACE, r, nr, MPI_FLOAT, MPI_SUM, mpi_comm);
+#else
+    /* this function is only used in code that is not performance critical,
+       (during setup, when comm_rec is not the appropriate communication
+       structure), so this isn't as bad as it looks. */
+    float *buf;
+    int    i;
+
+    snew(buf, nr);
+    MPI_Allreduce(r, buf, nr, MPI_FLOAT, MPI_SUM, mpi_comm);
+    for (i = 0; i < nr; i++)
+    {
+        r[i] = buf[i];
+    }
+    sfree(buf);
+#endif
+}
+#endif
+
+void gmx_sumd_sim(int gmx_unused nr, double gmx_unused r[], const gmx_multisim_t gmx_unused *ms)
+{
+#if !GMX_MPI
+    gmx_call("gmx_sumd_sim");
+#else
+    gmx_sumd_comm(nr, r, ms->mpi_comm_masters);
+#endif
+}
+
+void gmx_sumf_sim(int gmx_unused nr, float gmx_unused r[], const gmx_multisim_t gmx_unused *ms)
+{
+#if !GMX_MPI
+    gmx_call("gmx_sumf_sim");
+#else
+    gmx_sumf_comm(nr, r, ms->mpi_comm_masters);
+#endif
+}
+
+void gmx_sumi_sim(int gmx_unused nr, int gmx_unused r[], const gmx_multisim_t gmx_unused *ms)
+{
+#if !GMX_MPI
+    gmx_call("gmx_sumi_sim");
+#else
+#if MPI_IN_PLACE_EXISTS
+    MPI_Allreduce(MPI_IN_PLACE, r, nr, MPI_INT, MPI_SUM, ms->mpi_comm_masters);
+#else
+    /* this is thread-unsafe, but it will do for now: */
+    int i;
+
+    if (nr > ms->mpb->ibuf_alloc)
+    {
+        ms->mpb->ibuf_alloc = nr;
+        srenew(ms->mpb->ibuf, ms->mpb->ibuf_alloc);
+    }
+    MPI_Allreduce(r, ms->mpb->ibuf, nr, MPI_INT, MPI_SUM, ms->mpi_comm_masters);
+    for (i = 0; i < nr; i++)
+    {
+        r[i] = ms->mpb->ibuf[i];
+    }
+#endif
+#endif
+}
+
+void gmx_sumli_sim(int gmx_unused nr, int64_t gmx_unused r[], const gmx_multisim_t gmx_unused *ms)
+{
+#if !GMX_MPI
+    gmx_call("gmx_sumli_sim");
+#else
+#if MPI_IN_PLACE_EXISTS
+    MPI_Allreduce(MPI_IN_PLACE, r, nr, MPI_INT64_T, MPI_SUM,
+                  ms->mpi_comm_masters);
+#else
+    /* this is thread-unsafe, but it will do for now: */
+    int i;
+
+    if (nr > ms->mpb->libuf_alloc)
+    {
+        ms->mpb->libuf_alloc = nr;
+        srenew(ms->mpb->libuf, ms->mpb->libuf_alloc);
+    }
+    MPI_Allreduce(r, ms->mpb->libuf, nr, MPI_INT64_T, MPI_SUM,
+                  ms->mpi_comm_masters);
+    for (i = 0; i < nr; i++)
+    {
+        r[i] = ms->mpb->libuf[i];
+    }
+#endif
+#endif
+}
+
+void check_multi_int(FILE *log, const gmx_multisim_t *ms, int val,
+                     const char *name,
+                     gmx_bool bQuiet)
+{
+    int     *ibuf, p;
+    gmx_bool bCompatible;
+
+    if (nullptr != log && !bQuiet)
+    {
+        fprintf(log, "Multi-checking %s ... ", name);
+    }
+
+    if (ms == nullptr)
+    {
+        gmx_fatal(FARGS,
+                  "check_multi_int called with a NULL communication pointer");
+    }
+
+    snew(ibuf, ms->nsim);
+    ibuf[ms->sim] = val;
+    gmx_sumi_sim(ms->nsim, ibuf, ms);
+
+    bCompatible = TRUE;
+    for (p = 1; p < ms->nsim; p++)
+    {
+        bCompatible = bCompatible && (ibuf[p-1] == ibuf[p]);
+    }
+
+    if (bCompatible)
+    {
+        if (nullptr != log && !bQuiet)
+        {
+            fprintf(log, "OK\n");
+        }
+    }
+    else
+    {
+        if (nullptr != log)
+        {
+            fprintf(log, "\n%s is not equal for all subsystems\n", name);
+            for (p = 0; p < ms->nsim; p++)
+            {
+                fprintf(log, "  subsystem %d: %d\n", p, ibuf[p]);
+            }
+        }
+        gmx_fatal(FARGS, "The %d subsystems are not compatible\n", ms->nsim);
+    }
+
+    sfree(ibuf);
+}
+
+void check_multi_int64(FILE *log, const gmx_multisim_t *ms,
+                       int64_t val, const char *name,
+                       gmx_bool bQuiet)
+{
+    int64_t          *ibuf;
+    int               p;
+    gmx_bool          bCompatible;
+
+    if (nullptr != log && !bQuiet)
+    {
+        fprintf(log, "Multi-checking %s ... ", name);
+    }
+
+    if (ms == nullptr)
+    {
+        gmx_fatal(FARGS,
+                  "check_multi_int called with a NULL communication pointer");
+    }
+
+    snew(ibuf, ms->nsim);
+    ibuf[ms->sim] = val;
+    gmx_sumli_sim(ms->nsim, ibuf, ms);
+
+    bCompatible = TRUE;
+    for (p = 1; p < ms->nsim; p++)
+    {
+        bCompatible = bCompatible && (ibuf[p-1] == ibuf[p]);
+    }
+
+    if (bCompatible)
+    {
+        if (nullptr != log && !bQuiet)
+        {
+            fprintf(log, "OK\n");
+        }
+    }
+    else
+    {
+        // TODO Part of this error message would also be good to go to
+        // stderr (from one rank of one sim only)
+        if (nullptr != log)
+        {
+            fprintf(log, "\n%s is not equal for all subsystems\n", name);
+            for (p = 0; p < ms->nsim; p++)
+            {
+                char strbuf[255];
+                /* first make the format string */
+                snprintf(strbuf, 255, "  subsystem %%d: %s\n",
+                         "%" PRId64);
+                fprintf(log, strbuf, p, ibuf[p]);
+            }
+        }
+        gmx_fatal(FARGS, "The %d subsystems are not compatible\n", ms->nsim);
+    }
+
+    sfree(ibuf);
 }
 
 bool isMasterSim(const gmx_multisim_t *ms)
index ad8ba7731b3b6072f5250391e7e40fe56092f429..69395629bd6d7ec54d1ca847ca770629d3274187 100644 (file)
 #include "gromacs/utility/arrayref.h"
 #include "gromacs/utility/gmxmpi.h"
 
-struct gmx_multisim_t;
+struct mpi_in_place_buf_t;
+
+struct gmx_multisim_t
+{
+    int       nsim              = 1;
+    int       sim               = 0;
+    MPI_Group mpi_group_masters = MPI_GROUP_NULL;
+    MPI_Comm  mpi_comm_masters  = MPI_COMM_NULL;
+    /* these buffers are used as destination buffers if MPI_IN_PLACE isn't
+       supported.*/
+    mpi_in_place_buf_t *mpb = nullptr;
+};
 
 /*! \brief Initializes multi-simulations.
  *
@@ -61,6 +72,40 @@ gmx_multisim_t *init_multisystem(MPI_Comm                         comm,
 //! Cleans up multi-system handler.
 void done_multisim(gmx_multisim_t *ms);
 
+//! Calculate the sum over the simulations of an array of ints
+void gmx_sumi_sim(int nr, int r[], const gmx_multisim_t *ms);
+
+//! Calculate the sum over the simulations of an array of large ints
+void gmx_sumli_sim(int nr, int64_t r[], const gmx_multisim_t *ms);
+
+//! Calculate the sum over the simulations of an array of floats
+void gmx_sumf_sim(int nr, float r[], const gmx_multisim_t *ms);
+
+//! Calculate the sum over the simulations of an array of doubles
+void gmx_sumd_sim(int nr, double r[], const gmx_multisim_t *ms);
+
+/*! \brief Check if val is the same on all simulations for a mdrun
+ * -multidir run
+ *
+ * The string name is used to print to the log file and in a fatal error
+ * if the val's don't match. If bQuiet is true and the check passes,
+ * no output is written. */
+void check_multi_int(FILE *log, const gmx_multisim_t *ms,
+                     int val, const char *name,
+                     gmx_bool bQuiet);
+/*! \copydoc check_multi_int() */
+void check_multi_int64(FILE *log, const gmx_multisim_t *ms,
+                       int64_t val, const char *name,
+                       gmx_bool bQuiet);
+
+#if GMX_DOUBLE
+//! Convenience define for sum of reals
+#define gmx_sum_sim   gmx_sumd_sim
+#else
+//! Convenience define for sum of reals
+#define gmx_sum_sim   gmx_sumf_sim
+#endif
+
 //! Are we doing multiple independent simulations?
 static bool inline isMultiSim(const gmx_multisim_t *ms)
 {
index 247355168c8640735d431857675d67e1bc51ae70..41b5ed9f05bbb967273b5e43f28f5d6f2b4f0c63 100644 (file)
 
 #include <stddef.h>
 
-#include "gromacs/math/vectypes.h"
 #include "gromacs/utility/basedefinitions.h"
 #include "gromacs/utility/gmxassert.h"
 #include "gromacs/utility/gmxmpi.h"
-#include "gromacs/utility/real.h"
 
+struct mpi_in_place_buf_t;
 struct gmx_domdec_t;
 
-typedef struct {
-    /* these buffers are used as destination buffers if MPI_IN_PLACE isn't
-       supported.*/
-    int             *ibuf; /* for ints */
-    int              ibuf_alloc;
-
-    int64_t         *libuf;
-    int              libuf_alloc;
-
-    float           *fbuf; /* for floats */
-    int              fbuf_alloc;
-
-    double          *dbuf; /* for doubles */
-    int              dbuf_alloc;
-} mpi_in_place_buf_t;
-
-void done_mpi_in_place_buf(mpi_in_place_buf_t *buf);
-
-struct gmx_multisim_t
-{
-    int       nsim              = 1;
-    int       sim               = 0;
-    MPI_Group mpi_group_masters = MPI_GROUP_NULL;
-    MPI_Comm  mpi_comm_masters  = MPI_COMM_NULL;
-    /* these buffers are used as destination buffers if MPI_IN_PLACE isn't
-       supported.*/
-    mpi_in_place_buf_t *mpb = nullptr;
-};
-
 #define DUTY_PP  (1<<0)
 #define DUTY_PME (1<<1)
 
diff --git a/src/gromacs/utility/mpiinplacebuffers.cpp b/src/gromacs/utility/mpiinplacebuffers.cpp
new file mode 100644 (file)
index 0000000..33e775e
--- /dev/null
@@ -0,0 +1,58 @@
+/*
+ * This file is part of the GROMACS molecular simulation package.
+ *
+ * Copyright (c) 2019, 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.
+ *
+ * GROMACS is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public License
+ * as published by the Free Software Foundation; either version 2.1
+ * of the License, or (at your option) any later version.
+ *
+ * GROMACS is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with GROMACS; if not, see
+ * http://www.gnu.org/licenses, or write to the Free Software Foundation,
+ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301  USA.
+ *
+ * If you want to redistribute modifications to GROMACS, please
+ * consider that scientific software is very special. Version
+ * control is crucial - bugs must be traceable. We will be happy to
+ * consider code for inclusion in the official distribution, but
+ * derived work must not be called official GROMACS. Details are found
+ * in the README & COPYING files - if they are missing, get the
+ * official version at http://www.gromacs.org.
+ *
+ * To help us fund GROMACS development, we humbly ask that you cite
+ * the research papers on the package. Check out http://www.gromacs.org.
+ */
+/*! \internal \file
+ * \brief Defines cleanup function for the copies necessary when
+ * MPI_IN_PLACE is not supported by the MPI library.
+ *
+ * \author Mark Abraham <mark.j.abraham@gmail.com>
+ * \ingroup module_utility
+ */
+#include "gmxpre.h"
+
+#include "mpiinplacebuffers.h"
+
+#include "gromacs/utility/smalloc.h"
+
+void done_mpi_in_place_buf(mpi_in_place_buf_t *buf)
+{
+    if (nullptr != buf)
+    {
+        sfree(buf->ibuf);
+        sfree(buf->libuf);
+        sfree(buf->fbuf);
+        sfree(buf->dbuf);
+        sfree(buf);
+    }
+}
diff --git a/src/gromacs/utility/mpiinplacebuffers.h b/src/gromacs/utility/mpiinplacebuffers.h
new file mode 100644 (file)
index 0000000..64fac5a
--- /dev/null
@@ -0,0 +1,71 @@
+/*
+ * This file is part of the GROMACS molecular simulation package.
+ *
+ * Copyright (c) 2019, 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.
+ *
+ * GROMACS is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public License
+ * as published by the Free Software Foundation; either version 2.1
+ * of the License, or (at your option) any later version.
+ *
+ * GROMACS is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with GROMACS; if not, see
+ * http://www.gnu.org/licenses, or write to the Free Software Foundation,
+ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301  USA.
+ *
+ * If you want to redistribute modifications to GROMACS, please
+ * consider that scientific software is very special. Version
+ * control is crucial - bugs must be traceable. We will be happy to
+ * consider code for inclusion in the official distribution, but
+ * derived work must not be called official GROMACS. Details are found
+ * in the README & COPYING files - if they are missing, get the
+ * official version at http://www.gromacs.org.
+ *
+ * To help us fund GROMACS development, we humbly ask that you cite
+ * the research papers on the package. Check out http://www.gromacs.org.
+ */
+/*! \libinternal \file
+ * \brief Declares object that provides buffers for the copies
+ * necessary when MPI_IN_PLACE is not supported by the MPI library.
+ *
+ * The struct is declared in the header so that functionality it
+ * supports can be inlined.
+ *
+ * \author Mark Abraham <mark.j.abraham@gmail.com>
+ * \inlibraryapi
+ * \ingroup module_utility
+ */
+#ifndef GMX_UTILTIY_MPIINPLACEBUFFERS_H
+#define GMX_UTILTIY_MPIINPLACEBUFFERS_H
+
+#include <cstdint>
+
+struct mpi_in_place_buf_t
+{
+    /* these buffers are used as destination buffers if MPI_IN_PLACE isn't
+       supported.*/
+    int             *ibuf; /* for ints */
+    int              ibuf_alloc;
+
+    int64_t         *libuf;
+    int              libuf_alloc;
+
+    float           *fbuf; /* for floats */
+    int              fbuf_alloc;
+
+    double          *dbuf; /* for doubles */
+    int              dbuf_alloc;
+};
+
+//! Cleans up the buffers
+void done_mpi_in_place_buf(mpi_in_place_buf_t *buf);
+
+#endif