Remove foreign energy groups from gmx_enerdata_t

These members are only used for local storage and can be eliminated.

Added function to zero the energy group pair terms.

This will aid in further refactoring of the nonboded FEP kernels.

diff --git a/src/gromacs/listed_forces/listed_forces.cpp b/src/gromacs/listed_forces/listed_forces.cpp
index 85967f1b7f45caa70472ca266741b086630fc894..b0e40a9474656a3604a9175b6ef067af882851e3 100644 (file)
--- a/src/gromacs/listed_forces/listed_forces.cpp
+++ b/src/gromacs/listed_forces/listed_forces.cpp
@@ -91,7 +91,8 @@ ListedForces::ListedForces(const gmx_ffparams_t&      ffparams,
                            FILE*                      fplog) :
     idefSelection_(ffparams),
     threading_(std::make_unique<bonded_threading_t>(numThreads, numEnergyGroups, fplog)),
-    interactionSelection_(interactionSelection)
+    interactionSelection_(interactionSelection),
+    foreignEnergyGroups_(std::make_unique<gmx_grppairener_t>(numEnergyGroups))
 {
 }
 
@@ -714,7 +715,7 @@ void calc_listed_lambda(const InteractionDefinitions& idef,
                         gmx::ArrayRef<real>           forceBufferLambda,
                         gmx::ArrayRef<gmx::RVec>      shiftForceBufferLambda,
                         gmx_grppairener_t*            grpp,
-                        real*                         epot,
+                        gmx::ArrayRef<real>           epot,
                         gmx::ArrayRef<real>           dvdl,
                         t_nrnb*                       nrnb,
                         gmx::ArrayRef<const real>     lambda,
@@ -897,8 +898,8 @@ void ListedForces::calculate(struct gmx_wallcycle*                     wcycle,
             for (int i = 0; i < 1 + enerd->foreignLambdaTerms.numLambdas(); i++)
             {
                 gmx::EnumerationArray<FreeEnergyPerturbationCouplingType, real> lam_i;
-
-                reset_foreign_enerdata(enerd);
+                foreignEnergyGroups_->clear();
+                std::array<real, F_NRE> foreign_term = { 0 };
                 for (auto j : keysOf(lam_i))
                 {
                     lam_i[j] = (i == 0 ? lambda[static_cast<int>(j)] : fepvals->all_lambda[j][i - 1]);
@@ -910,18 +911,18 @@ void ListedForces::calculate(struct gmx_wallcycle*                     wcycle,
                                    pbc,
                                    forceBufferLambda_,
                                    shiftForceBufferLambda_,
-                                   &(enerd->foreign_grpp),
-                                   enerd->foreign_term.data(),
+                                   foreignEnergyGroups_.get(),
+                                   foreign_term,
                                    dvdl,
                                    nrnb,
                                    lam_i,
                                    md,
                                    fcdata,
                                    global_atom_index);
-                sum_epot(enerd->foreign_grpp, enerd->foreign_term.data());
+                sum_epot(*foreignEnergyGroups_, foreign_term.data());
                 const double dvdlSum = std::accumulate(std::begin(dvdl), std::end(dvdl), 0.);
                 std::fill(std::begin(dvdl), std::end(dvdl), 0.0);
-                enerd->foreignLambdaTerms.accumulate(i, enerd->foreign_term[F_EPOT], dvdlSum);
+                enerd->foreignLambdaTerms.accumulate(i, foreign_term[F_EPOT], dvdlSum);
             }
             wallcycle_sub_stop(wcycle, WallCycleSubCounter::ListedFep);
         }

diff --git a/src/gromacs/listed_forces/listed_forces.h b/src/gromacs/listed_forces/listed_forces.h
index 34e297b36d5b72a2f1a3ce3585e34d54404f1bbe..6d531ea8b07b5dcf2dc6d297c07da24e7f5250fe 100644 (file)
--- a/src/gromacs/listed_forces/listed_forces.h
+++ b/src/gromacs/listed_forces/listed_forces.h
@@ -233,6 +233,8 @@ private:
     std::vector<real> forceBufferLambda_;
     //! Shift force buffer for free-energy forces
     std::vector<gmx::RVec> shiftForceBufferLambda_;
+    //! Temporary array for storing foreign lambda group pair energies
+    std::unique_ptr<gmx_grppairener_t> foreignEnergyGroups_;
 
     GMX_DISALLOW_COPY_AND_ASSIGN(ListedForces);
 };

diff --git a/src/gromacs/mdlib/enerdata_utils.cpp b/src/gromacs/mdlib/enerdata_utils.cpp
index f76dca56a1e3fafd5398f69f6b992b750f26d014..40288fa1c60ef179a04657352446509928238784 100644 (file)
--- a/src/gromacs/mdlib/enerdata_utils.cpp
+++ b/src/gromacs/mdlib/enerdata_utils.cpp
@@ -80,7 +80,7 @@ void ForeignLambdaTerms::zeroAllTerms()
 }
 
 gmx_enerdata_t::gmx_enerdata_t(int numEnergyGroups, int numFepLambdas) :
-    grpp(numEnergyGroups), foreignLambdaTerms(numFepLambdas), foreign_grpp(numEnergyGroups)
+    grpp(numEnergyGroups), foreignLambdaTerms(numFepLambdas)
 {
 }
 
@@ -311,25 +311,15 @@ void accumulateKineticLambdaComponents(gmx_enerdata_t*           enerd,
     enerd->term[F_DVDL_CONSTR] = 0;
 }
 
-void reset_foreign_enerdata(gmx_enerdata_t* enerd)
+void gmx_grppairener_t::clear()
 {
-    int i, j;
-
-    /* First reset all foreign energy components.  Foreign energies always called on
-       neighbor search steps */
-    for (i = 0; (i < static_cast<int>(NonBondedEnergyTerms::Count)); i++)
+    for (int i = 0; (i < static_cast<int>(NonBondedEnergyTerms::Count)); i++)
     {
-        for (j = 0; (j < enerd->grpp.nener); j++)
+        for (int j = 0; (j < nener); j++)
         {
-            enerd->foreign_grpp.energyGroupPairTerms[i][j] = 0.0;
+            energyGroupPairTerms[i][j] = 0.0;
         }
     }
-
-    /* potential energy components */
-    for (i = 0; (i <= F_EPOT); i++)
-    {
-        enerd->foreign_term[i] = 0.0;
-    }
 }
 
 void reset_dvdl_enerdata(gmx_enerdata_t* enerd)
@@ -367,7 +357,6 @@ void reset_enerdata(gmx_enerdata_t* enerd)
     enerd->term[F_DVDL_RESTRAINT] = 0.0_real;
     enerd->term[F_DKDL]           = 0.0_real;
     enerd->foreignLambdaTerms.zeroAllTerms();
-    /* reset foreign energy data and dvdl - separate functions since they are also called elsewhere */
-    reset_foreign_enerdata(enerd);
+    /* reset dvdl - separate function since it is also called elsewhere */
     reset_dvdl_enerdata(enerd);
 }

diff --git a/src/gromacs/mdlib/enerdata_utils.h b/src/gromacs/mdlib/enerdata_utils.h
index 5e0b066e0c6a493ef2a94be3b620264fcdbf30d5..a36ee636605c7783ebe7502cea81ac009285c211 100644 (file)
--- a/src/gromacs/mdlib/enerdata_utils.h
+++ b/src/gromacs/mdlib/enerdata_utils.h
@@ -4,7 +4,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 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.
@@ -48,9 +48,6 @@ struct gmx_grppairener_t;
 struct t_fepvals;
 struct t_lambda;
 
-void reset_foreign_enerdata(gmx_enerdata_t* enerd);
-/* Resets only the foreign energy data */
-
 void reset_dvdl_enerdata(gmx_enerdata_t* enerd);
 /* Resets only the dvdl energy data */
 

diff --git a/src/gromacs/mdlib/energyoutput.cpp b/src/gromacs/mdlib/energyoutput.cpp
index fa5b8dca1075c0a3851f74502701af9784e03df2..a0b0abb1d57a655ee9f7601161bcb79adc74ff74 100644 (file)
--- a/src/gromacs/mdlib/energyoutput.cpp
+++ b/src/gromacs/mdlib/energyoutput.cpp
@@ -1209,8 +1209,8 @@ void EnergyOutput::printStepToEnergyFile(ener_file* fp_ene,
             fr.block[b].sub[0].type = XdrDataType::Float;
             fr.block[b].sub[0].fval = block[b];
 #else
-            fr.block[b].sub[0].type  = XdrDataType::Double;
-            fr.block[b].sub[0].dval  = block[b];
+            fr.block[b].sub[0].type = XdrDataType::Double;
+            fr.block[b].sub[0].dval = block[b];
 #endif
         }
 

diff --git a/src/gromacs/mdtypes/enerdata.h b/src/gromacs/mdtypes/enerdata.h
index 3398cd69aa98cde1bbd5c8740b61dbd1695eb87a..c24a30b2aa9bbc93abed3ff532c14255b32b4d7d 100644 (file)
--- a/src/gromacs/mdtypes/enerdata.h
+++ b/src/gromacs/mdtypes/enerdata.h
@@ -71,6 +71,8 @@ struct gmx_grppairener_t
         }
     }
 
+    void clear();
+
     int nener; /* The number of energy group pairs */
     gmx::EnumerationArray<NonBondedEnergyTerms, std::vector<real>> energyGroupPairTerms; /* Energy terms for each pair of groups */
 };
@@ -210,11 +212,6 @@ struct gmx_enerdata_t
 
     //! Foreign lambda energies and dH/dl
     ForeignLambdaTerms foreignLambdaTerms;
-
-    //! Alternate, temporary array for storing foreign lambda energies
-    std::array<real, F_NRE> foreign_term = { 0 };
-    //! Alternate, temporary  array for storing foreign lambda group pair energies
-    struct gmx_grppairener_t foreign_grpp;
 };
 
 #endif

diff --git a/src/gromacs/nbnxm/kerneldispatch.cpp b/src/gromacs/nbnxm/kerneldispatch.cpp
index 3e7da8adf8ff31a893e3a64ba749174bcb2045d2..28769f6ff131b7014f319bd508f5b75ea3bd2d9c 100644 (file)
--- a/src/gromacs/nbnxm/kerneldispatch.cpp
+++ b/src/gromacs/nbnxm/kerneldispatch.cpp
@@ -604,9 +604,9 @@ void nonbonded_verlet_t::dispatchFreeEnergyKernel(gmx::InteractionLocality
         kernelLambda = lam_i;
         kernelDvdl   = dvdl_nb;
         gmx::ArrayRef<real> energygrp_elec =
-                enerd->foreign_grpp.energyGroupPairTerms[NonBondedEnergyTerms::CoulombSR];
+                foreignEnergyGroups_->energyGroupPairTerms[NonBondedEnergyTerms::CoulombSR];
         gmx::ArrayRef<real> energygrp_vdw =
-                enerd->foreign_grpp.energyGroupPairTerms[NonBondedEnergyTerms::LJSR];
+                foreignEnergyGroups_->energyGroupPairTerms[NonBondedEnergyTerms::LJSR];
 
         for (gmx::index i = 0; i < 1 + enerd->foreignLambdaTerms.numLambdas(); i++)
         {
@@ -615,7 +615,7 @@ void nonbonded_verlet_t::dispatchFreeEnergyKernel(gmx::InteractionLocality
             {
                 lam_i[j] = (i == 0 ? lambda[j] : fepvals->all_lambda[j][i - 1]);
             }
-            reset_foreign_enerdata(enerd);
+            foreignEnergyGroups_->clear();
 #pragma omp parallel for schedule(static) num_threads(nbl_fep.ssize())
             for (gmx::index th = 0; th < nbl_fep.ssize(); th++)
             {
@@ -644,11 +644,11 @@ void nonbonded_verlet_t::dispatchFreeEnergyKernel(gmx::InteractionLocality
                 }
                 GMX_CATCH_ALL_AND_EXIT_WITH_FATAL_ERROR
             }
-
-            sum_epot(enerd->foreign_grpp, enerd->foreign_term.data());
+            std::array<real, F_NRE> foreign_term = { 0 };
+            sum_epot(*foreignEnergyGroups_, foreign_term.data());
             enerd->foreignLambdaTerms.accumulate(
                     i,
-                    enerd->foreign_term[F_EPOT],
+                    foreign_term[F_EPOT],
                     dvdl_nb[FreeEnergyPerturbationCouplingType::Vdw]
                             + dvdl_nb[FreeEnergyPerturbationCouplingType::Coul]);
         }

diff --git a/src/gromacs/nbnxm/nbnxm.h b/src/gromacs/nbnxm/nbnxm.h
index 7939f1499d3d7abbe9debb0ef84f1d546e19963b..dfaaff41dc3a52a1ec818855d81dec37688cda9c 100644 (file)
--- a/src/gromacs/nbnxm/nbnxm.h
+++ b/src/gromacs/nbnxm/nbnxm.h
@@ -138,6 +138,7 @@ struct t_lambda;
 struct t_nrnb;
 struct t_forcerec;
 struct t_inputrec;
+struct gmx_grppairener_t;
 
 class GpuEventSynchronizer;
 
@@ -429,6 +430,8 @@ private:
     Nbnxm::KernelSetup kernelSetup_;
     //! \brief Pointer to wallcycle structure.
     gmx_wallcycle* wcycle_;
+    //! Temporary array for storing foreign lambda group pair energies
+    std::unique_ptr<gmx_grppairener_t> foreignEnergyGroups_;
 
 public:
     //! GPU Nbnxm data, only used with a physical GPU (TODO: use unique_ptr)

diff --git a/src/gromacs/nbnxm/nbnxm_setup.cpp b/src/gromacs/nbnxm/nbnxm_setup.cpp
index b33390c7e25311feb48ce106ae1d8bce6dfd6d4d..42e3511853eea9fb265adbb91a468805dadc9bd6 100644 (file)
--- a/src/gromacs/nbnxm/nbnxm_setup.cpp
+++ b/src/gromacs/nbnxm/nbnxm_setup.cpp
@@ -42,11 +42,14 @@
 
 #include "gmxpre.h"
 
+#include <memory>
+
 #include "gromacs/domdec/domdec.h"
 #include "gromacs/domdec/domdec_struct.h"
 #include "gromacs/hardware/hw_info.h"
 #include "gromacs/mdlib/gmx_omp_nthreads.h"
 #include "gromacs/mdtypes/commrec.h"
+#include "gromacs/mdtypes/enerdata.h"
 #include "gromacs/mdtypes/forcerec.h"
 #include "gromacs/mdtypes/inputrec.h"
 #include "gromacs/mdtypes/interaction_const.h"
@@ -220,7 +223,7 @@ const char* lookup_kernel_name(const KernelType kernelType)
         case KernelType::Cpu4xN_Simd_2xNN:
 #if GMX_SIMD
             return "SIMD";
-#else  // GMX_SIMD
+#else // GMX_SIMD
             return "not available";
 #endif // GMX_SIMD
         case KernelType::Gpu8x8x8: return "GPU";
@@ -482,6 +485,7 @@ nonbonded_verlet_t::nonbonded_verlet_t(std::unique_ptr<PairlistSets>     pairlis
     nbat(std::move(nbat_in)),
     kernelSetup_(kernelSetup),
     wcycle_(wcycle),
+    foreignEnergyGroups_(std::make_unique<gmx_grppairener_t>(nbat->params().nenergrp)),
     gpu_nbv(gpu_nbv_ptr)
 {
     GMX_RELEASE_ASSERT(pairlistSets_, "Need valid pairlistSets");