Replace bool with enum class in DomdecOptions

The logic is simpler if we just use DDBondedChecking directly in
DomdecOptions. The enum values have changed so that they match those
for the bool type. Added a test and an assertion to confirm that this
works as expected, ie the default for mdrun -ddcheck is unchanged

diff --git a/src/gromacs/commandline/tests/pargs.cpp b/src/gromacs/commandline/tests/pargs.cpp
index 2377de53a8bfbc0020393c89bff1523c0366aaa4..eeefe76ec3aa0927008747fe67f20a2445a85a47 100644 (file)
--- a/src/gromacs/commandline/tests/pargs.cpp
+++ b/src/gromacs/commandline/tests/pargs.cpp
@@ -2,7 +2,7 @@
  * This file is part of the GROMACS molecular simulation package.
  *
  * Copyright (c) 2013,2014,2015,2016,2017 by the GROMACS development team.
- * Copyright (c) 2018,2019,2020, by the GROMACS development team, led by
+ * Copyright (c) 2018,2019,2020,2021, 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.
@@ -191,6 +191,40 @@ TEST_F(ParseCommonArgsTest, ParsesBooleanArgs)
     EXPECT_TRUE(value3);
 }
 
+TEST_F(ParseCommonArgsTest, ParsesBooleanArgsToValuesOfSuitableEnum)
+{
+    enum class LikeBool
+    {
+        No  = 0,
+        Yes = 1
+    };
+    // Set up the default values
+    LikeBool value1 = LikeBool::No;
+    LikeBool value2 = LikeBool::No;
+    LikeBool value3 = LikeBool::No;
+    LikeBool value4 = LikeBool::Yes;
+    LikeBool value5 = LikeBool::Yes;
+    LikeBool value6 = LikeBool::Yes;
+    // Set up 6 options
+    t_pargs pa[] = { { "-b1", FALSE, etBOOL, { &value1 }, "Description" },
+                     { "-b2", FALSE, etBOOL, { &value2 }, "Description" },
+                     { "-b3", FALSE, etBOOL, { &value3 }, "Description" },
+                     { "-b4", FALSE, etBOOL, { &value4 }, "Description" },
+                     { "-b5", FALSE, etBOOL, { &value5 }, "Description" },
+                     { "-b6", FALSE, etBOOL, { &value6 }, "Description" } };
+    // Set some to yes and no, leave some as default
+    const char* const cmdline[] = { "test", "-b1", "-nob2", "-b4", "-nob5" };
+
+    parseFromArray(cmdline, 0, {}, pa);
+    // Expetactions
+    EXPECT_EQ(value1, LikeBool::Yes) << "set to yes";
+    EXPECT_EQ(value2, LikeBool::No) << "set to no";
+    EXPECT_EQ(value3, LikeBool::No) << "preserves the default of no";
+    EXPECT_EQ(value4, LikeBool::Yes) << "set to yes";
+    EXPECT_EQ(value5, LikeBool::No) << "set to no";
+    EXPECT_EQ(value6, LikeBool::Yes) << "preserves the default of yes";
+}
+
 TEST_F(ParseCommonArgsTest, ParsesVectorArgs)
 {
     rvec              value1 = { 0, 0, 0 }, value2 = { 0, 0, 0 }, value3 = { 1, 2, 3 };

diff --git a/src/gromacs/domdec/domdec.cpp b/src/gromacs/domdec/domdec.cpp
index 11478bba0f7391288a580e48944cc4dc4eb93ca7..e6e3df58e390d4f74105f7b8d876339b7f4c6dc9 100644 (file)
--- a/src/gromacs/domdec/domdec.cpp
+++ b/src/gromacs/domdec/domdec.cpp
@@ -2137,11 +2137,7 @@ static DDSystemInfo getSystemInfo(const gmx::MDLogger&           mdlog,
 
             if (ddRole == DDRole::Master)
             {
-                const DDBondedChecking ddBondedChecking = options.checkBondedInteractions
-                                                                  ? DDBondedChecking::All
-                                                                  : DDBondedChecking::ExcludeZeroLimit;
-
-                dd_bonded_cg_distance(mdlog, mtop, ir, xGlobal, box, ddBondedChecking, &r_2b, &r_mb);
+                dd_bonded_cg_distance(mdlog, mtop, ir, xGlobal, box, options.ddBondedChecking, &r_2b, &r_mb);
             }
             gmx_bcast(sizeof(r_2b), &r_2b, communicator);
             gmx_bcast(sizeof(r_mb), &r_mb, communicator);
@@ -2500,7 +2496,7 @@ void dd_init_bondeds(FILE*                           fplog,
                      const gmx_mtop_t&               mtop,
                      const gmx::VirtualSitesHandler* vsite,
                      const t_inputrec&               inputrec,
-                     const DDBondedChecking          ddBondedChecking,
+                     const gmx::DDBondedChecking     ddBondedChecking,
                      gmx::ArrayRef<cginfo_mb_t>      cginfo_mb)
 {
     dd_make_reverse_top(fplog, dd, mtop, vsite, inputrec, ddBondedChecking);

diff --git a/src/gromacs/domdec/domdec.h b/src/gromacs/domdec/domdec.h
index 9b3d8c0e068c5493d0df916f4d4f15a001477365..a62a0c91f244a55562f1de87514af44989a4e8d4 100644 (file)
--- a/src/gromacs/domdec/domdec.h
+++ b/src/gromacs/domdec/domdec.h
@@ -94,6 +94,7 @@ class RangePartitioning;
 class VirtualSitesHandler;
 template<typename>
 class ArrayRef;
+enum class DDBondedChecking;
 } // namespace gmx
 
 /*! \brief Returns the global topology atom number belonging to local atom index i.
@@ -156,20 +157,13 @@ bool ddHaveSplitConstraints(const gmx_domdec_t& dd);
 /*! \brief Return whether update groups are used */
 bool ddUsesUpdateGroups(const gmx_domdec_t& dd);
 
-/*! \brief Options for checking bonded interactions */
-enum class DDBondedChecking
-{
-    All,             //!< Check all bonded interactions
-    ExcludeZeroLimit //!< Do not check bonded interactions that go to 0 for large distances
-};
-
 /*! \brief Initialize data structures for bonded interactions */
 void dd_init_bondeds(FILE*                           fplog,
                      gmx_domdec_t*                   dd,
                      const gmx_mtop_t&               mtop,
                      const gmx::VirtualSitesHandler* vsite,
                      const t_inputrec&               inputrec,
-                     DDBondedChecking                ddBondedChecking,
+                     gmx::DDBondedChecking           ddBondedChecking,
                      gmx::ArrayRef<cginfo_mb_t>      cginfo_mb);
 
 /*! \brief Returns whether molecules are always whole, i.e. not broken by PBC */
@@ -279,7 +273,7 @@ void dd_make_reverse_top(FILE*                           fplog,
                          const gmx_mtop_t&               mtop,
                          const gmx::VirtualSitesHandler* vsite,
                          const t_inputrec&               inputrec,
-                         DDBondedChecking                ddBondedChecking);
+                         gmx::DDBondedChecking           ddBondedChecking);
 
 /*! \brief Return whether the total bonded interaction count across
  * domains should be checked this step. */
@@ -340,7 +334,7 @@ void dd_bonded_cg_distance(const gmx::MDLogger&           mdlog,
                            const t_inputrec&              ir,
                            gmx::ArrayRef<const gmx::RVec> x,
                            const matrix                   box,
-                           DDBondedChecking               ddBondedChecking,
+                           gmx::DDBondedChecking          ddBondedChecking,
                            real*                          r_2b,
                            real*                          r_mb);
 

diff --git a/src/gromacs/domdec/domdec_topology.cpp b/src/gromacs/domdec/domdec_topology.cpp
index 992573a7eb8f68ee5b4a92df62448c0cc74e13f8..d9452d2a1d430151c578de479794e5e8d7407a2d 100644 (file)
--- a/src/gromacs/domdec/domdec_topology.cpp
+++ b/src/gromacs/domdec/domdec_topology.cpp
@@ -58,6 +58,7 @@
 #include "gromacs/domdec/domdec.h"
 #include "gromacs/domdec/domdec_network.h"
 #include "gromacs/domdec/ga2la.h"
+#include "gromacs/domdec/options.h"
 #include "gromacs/gmxlib/network.h"
 #include "gromacs/math/vec.h"
 #include "gromacs/mdlib/forcerec.h"
@@ -92,6 +93,7 @@
 #include "dump.h"
 
 using gmx::ArrayRef;
+using gmx::DDBondedChecking;
 using gmx::ListOfLists;
 using gmx::RVec;
 
@@ -790,6 +792,9 @@ void dd_make_reverse_top(FILE*                           fplog,
      * Otherwise we need to assign them to multiple domains and set up
      * the parallel version constraint algorithm(s).
      */
+    GMX_RELEASE_ASSERT(ddBondedChecking == DDBondedChecking::ExcludeZeroLimit
+                               || ddBondedChecking == DDBondedChecking::All,
+                       "Invalid enum value for mdrun -ddcheck");
     const ReverseTopOptions rtOptions(ddBondedChecking,
                                       !dd->comm->systemInfo.haveSplitConstraints,
                                       !dd->comm->systemInfo.haveSplitSettles);

diff --git a/src/gromacs/domdec/options.h b/src/gromacs/domdec/options.h
index 0ed617f677031468c7326e6ce822158c4b60d6ca..2f6e858f926bfbc7f633f90c1310c68707c370cd 100644 (file)
--- a/src/gromacs/domdec/options.h
+++ b/src/gromacs/domdec/options.h
@@ -1,7 +1,7 @@
 /*
  * This file is part of the GROMACS molecular simulation package.
  *
- * Copyright (c) 2018,2019, by the GROMACS development team, led by
+ * Copyright (c) 2018,2019,2021, 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.
@@ -73,11 +73,20 @@ enum class DlbOption
     Count             //!< The number of options
 };
 
+/*! \brief Options for checking bonded interactions.
+ *
+ * These values must match the bool false and true used for mdrun -ddcheck */
+enum class DDBondedChecking
+{
+    ExcludeZeroLimit = 0, //!< Do not check bonded interactions that go to 0 for large distances
+    All              = 1  //!< Check all bonded interactions
+};
+
 /*! \libinternal \brief Structure containing all (command line) options for the domain decomposition */
 struct DomdecOptions
 {
-    //! If true, check that all bonded interactions have been assigned to exactly one domain/rank.
-    bool checkBondedInteractions = true;
+    //! Style of bonded-interaction checking
+    DDBondedChecking ddBondedChecking = DDBondedChecking::All;
     //! If true, don't communicate all atoms between the non-bonded cut-off and the larger bonded cut-off, but only those that have non-local bonded interactions. This significantly reduces the communication volume.
     bool useBondedCommunication = true;
     //! The domain decomposition grid cell count, 0 means let domdec choose based on the number of ranks.

diff --git a/src/gromacs/mdrun/legacymdrunoptions.h b/src/gromacs/mdrun/legacymdrunoptions.h
index 95abef6a57f49e4bcdc9e303cf19ce79ded5701d..678f741648921d772abe84e54a6f2d9ae4547a8f 100644 (file)
--- a/src/gromacs/mdrun/legacymdrunoptions.h
+++ b/src/gromacs/mdrun/legacymdrunoptions.h
@@ -227,7 +227,7 @@ public:
         { "-ddcheck",
           FALSE,
           etBOOL,
-          { &domdecOptions.checkBondedInteractions },
+          { &domdecOptions.ddBondedChecking },
           "Check for all bonded interactions with DD" },
         { "-ddbondcomm",
           FALSE,

diff --git a/src/gromacs/mdrun/runner.cpp b/src/gromacs/mdrun/runner.cpp
index 1f8d599bc5ce24d3c6af3f7a9ea202bc764cb0f5..e7874c8d0c0f5ce275f171de08ae184beec75ecd 100644 (file)
--- a/src/gromacs/mdrun/runner.cpp
+++ b/src/gromacs/mdrun/runner.cpp
@@ -1960,14 +1960,8 @@ int Mdrunner::mdrunner()
             /* This call is not included in init_domain_decomposition mainly
              * because fr->cginfo_mb is set later.
              */
-            dd_init_bondeds(fplog,
-                            cr->dd,
-                            mtop,
-                            vsite.get(),
-                            *inputrec,
-                            domdecOptions.checkBondedInteractions ? DDBondedChecking::All
-                                                                  : DDBondedChecking::ExcludeZeroLimit,
-                            fr->cginfo_mb);
+            dd_init_bondeds(
+                    fplog, cr->dd, mtop, vsite.get(), *inputrec, domdecOptions.ddBondedChecking, fr->cginfo_mb);
         }
 
         if (runScheduleWork.simulationWork.useGpuBufferOps)