From: Mark Abraham Date: Thu, 29 Apr 2021 09:04:33 +0000 (+0200) Subject: Replace bool with enum class in DomdecOptions X-Git-Url: http://biod.pnpi.spb.ru/gitweb/?a=commitdiff_plain;h=c0f8bc972937aed40f48d2a2051376b24fedcee3;p=alexxy%2Fgromacs.git 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 2377de53a8..eeefe76ec3 100644 --- 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 11478bba0f..e6e3df58e3 100644 --- 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) { 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 9b3d8c0e06..a62a0c91f2 100644 --- a/src/gromacs/domdec/domdec.h +++ b/src/gromacs/domdec/domdec.h @@ -94,6 +94,7 @@ class RangePartitioning; class VirtualSitesHandler; template 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); /*! \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 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 992573a7eb..d9452d2a1d 100644 --- 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 0ed617f677..2f6e858f92 100644 --- 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 95abef6a57..678f741648 100644 --- 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 1f8d599bc5..e7874c8d0c 100644 --- 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)