Separate energy data and element
authorPascal Merz <pascal.merz@me.com>
Tue, 14 Jul 2020 13:27:20 +0000 (13:27 +0000)
committerMark Abraham <mark.j.abraham@gmail.com>
Tue, 14 Jul 2020 13:27:20 +0000 (13:27 +0000)
The EnergyElement was both holding data which is accessed by
a majority of the elements, and was an element itself. The element
part is needed to update the energy records and save data for
trajectory writing.
The double nature of the EnergyElement created some difficulty in
initializing the elements, however - the EnergyElement
has a well defined place within the simulator loop, but has to be
created ahead of time so that other elements can get a pointer to it
to query for or save energy data during the simulation run.

This change separates the ISimulatorElement implementation which
allows EnergyElement to take part in the simulation loop
into a member object. The data part of the EnergyElement is renamed
to EnergyData, the element is EnergyData::Element. The EnergyData
can now be created ahead of element construction time, while an
object of the member class can be created later. This will make the
implementation of a builder approach significantly easier.
The life time of the element is managed by the EnergyData object to
avoid problematic life time dependencies between the two.

Refs #3437

21 files changed:
docs/doxygen/lib/modularsimulator.md
src/gromacs/mdrun/md.cpp
src/gromacs/modularsimulator/computeglobalselement.cpp
src/gromacs/modularsimulator/computeglobalselement.h
src/gromacs/modularsimulator/constraintelement.cpp
src/gromacs/modularsimulator/constraintelement.h
src/gromacs/modularsimulator/domdechelper.h
src/gromacs/modularsimulator/energydata.cpp [moved from src/gromacs/modularsimulator/energyelement.cpp with 77% similarity]
src/gromacs/modularsimulator/energydata.h [moved from src/gromacs/modularsimulator/energyelement.h with 80% similarity]
src/gromacs/modularsimulator/forceelement.cpp
src/gromacs/modularsimulator/forceelement.h
src/gromacs/modularsimulator/modularsimulator.cpp
src/gromacs/modularsimulator/parrinellorahmanbarostat.cpp
src/gromacs/modularsimulator/parrinellorahmanbarostat.h
src/gromacs/modularsimulator/pmeloadbalancehelper.h
src/gromacs/modularsimulator/propagator.h
src/gromacs/modularsimulator/simulatoralgorithm.cpp
src/gromacs/modularsimulator/simulatoralgorithm.h
src/gromacs/modularsimulator/statepropagatordata.h
src/gromacs/modularsimulator/vrescalethermostat.cpp
src/gromacs/modularsimulator/vrescalethermostat.h

index 400655f7d0bea68b578677b6937467852ea7b845..a1431bf4d07259a63e37f710bfffc0c467843670 100644 (file)
@@ -370,27 +370,27 @@ about the data that is written to file - it is only responsible
 to inform clients about trajectory steps, and providing a valid
 file pointer to the objects that need to write to trajectory.
 
-#### `StatePropagatorData`
-The `StatePropagatorData` takes part in the simulator run, as it might
+#### `StatePropagatorData::Element`
+The `StatePropagatorData::Element` takes part in the simulator run, as it might
 have to save a valid state at the right moment during the
 integration. Placing the StatePropagatorData correctly is for now the
 duty of the simulator builder - this might be automated later
 if we have enough meta-data of the variables (i.e., if
 `StatePropagatorData` knows at which time the variables currently are,
 and can decide when a valid state (full-time step of all
-variables) is reached. The `StatePropagatorData` is also a client of
+variables) is reached. The `StatePropagatorData::Element` is also a client of
 both the trajectory signaller and writer - it will save a
 state for later writeout during the simulator step if it
 knows that trajectory writing will occur later in the step,
 and it knows how to write to file given a file pointer by
 the `TrajectoryElement`.
 
-#### `EnergyElement`
-The `EnergyElement` takes part in the simulator run, as it
+#### `EnergyData::Element`
+The `EnergyData::Element` takes part in the simulator run, as it
 does either add data (at energy calculation steps), or
 record a non-calculation step (all other steps). It is the
 responsibility of the simulator builder to ensure that the
-`EnergyElement` is called at a point of the simulator run
+`EnergyData::Element` is called at a point of the simulator run
 at which it has access to a valid energy state.
 
 #### `ComputeGlobalsElement`
@@ -489,8 +489,8 @@ data approach - of the elements currently implemented, only
 domain decomposition, PME load balancing, and the initial
 constraining are using this.
 
-### `EnergyElement`
-The EnergyElement owns the EnergyObject, and is hence responsible
+### `EnergyData`
+The EnergyData owns the EnergyObject, and is hence responsible
 for saving energy data and writing it to trajectory. It also owns
 the tensors for the different virials and the pressure as well as
 the total dipole vector.
@@ -499,11 +499,11 @@ It subscribes to the trajectory signaller, the energy signaller,
 and the logging signaller to know when an energy calculation is
 needed and when a non-recording step is enough. The simulator
 builder is responsible to place the element in a location at
-which a valid energy state is available. The EnergyElement is
+which a valid energy state is available. The EnergyData is
 also a subscriber to the trajectory writer element, as it is
 responsible to write energy data to trajectory.
 
-The EnergyElement offers an interface to add virial contributions,
+The EnergyData offers an interface to add virial contributions,
 but also allows access to the raw pointers to tensor data, the
 dipole vector, and the legacy energy data structures.
 
index f891b5bce76133bf7575757a0313b6d3f683260c..bae77f14de0df5ec7eb6da270b28d16b275d77d6 100644 (file)
 #include "gromacs/mdtypes/simulation_workload.h"
 #include "gromacs/mdtypes/state.h"
 #include "gromacs/mdtypes/state_propagator_data_gpu.h"
-#include "gromacs/modularsimulator/energyelement.h"
+#include "gromacs/modularsimulator/energydata.h"
 #include "gromacs/nbnxm/gpu_data_mgmt.h"
 #include "gromacs/nbnxm/nbnxm.h"
 #include "gromacs/pbcutil/pbc.h"
@@ -448,7 +448,7 @@ void gmx::LegacySimulator::do_md()
 
     if (MASTER(cr))
     {
-        EnergyElement::initializeEnergyHistory(startingBehavior, observablesHistory, &energyOutput);
+        EnergyData::initializeEnergyHistory(startingBehavior, observablesHistory, &energyOutput);
     }
 
     preparePrevStepPullCom(ir, pull_work, mdatoms->massT, state, state_global, cr,
index 233e6fca83780692c44abc4d20e3305133055dc9..76709232dc986b10e0432b28bf3d784edcc09f72 100644 (file)
@@ -61,7 +61,7 @@ namespace gmx
 {
 template<ComputeGlobalsAlgorithm algorithm>
 ComputeGlobalsElement<algorithm>::ComputeGlobalsElement(StatePropagatorData* statePropagatorData,
-                                                        EnergyElement*       energyElement,
+                                                        EnergyData*          energyData,
                                                         FreeEnergyPerturbationElement* freeEnergyPerturbationElement,
                                                         SimulationSignals* signals,
                                                         int                nstglobalcomm,
@@ -89,7 +89,7 @@ ComputeGlobalsElement<algorithm>::ComputeGlobalsElement(StatePropagatorData* sta
     totalNumberOfBondedInteractions_(0),
     shouldCheckNumberOfBondedInteractions_(false),
     statePropagatorData_(statePropagatorData),
-    energyElement_(energyElement),
+    energyData_(energyData),
     localTopology_(nullptr),
     freeEnergyPerturbationElement_(freeEnergyPerturbationElement),
     vcm_(global_top->groups, *inputrec),
@@ -151,8 +151,7 @@ void ComputeGlobalsElement<algorithm>::elementSetup()
     // Calculate the initial half step temperature, and save the ekinh_old
     for (int i = 0; (i < inputrec_->opts.ngtc); i++)
     {
-        copy_mat(energyElement_->ekindata()->tcstat[i].ekinh,
-                 energyElement_->ekindata()->tcstat[i].ekinh_old);
+        copy_mat(energyData_->ekindata()->tcstat[i].ekinh, energyData_->ekindata()->tcstat[i].ekinh_old);
     }
 }
 
@@ -281,11 +280,11 @@ void ComputeGlobalsElement<algorithm>::compute(gmx::Step            step,
                               : statePropagatorData_->constBox();
 
     compute_globals(
-            gstat_, cr_, inputrec_, fr_, energyElement_->ekindata(), x, v, box, mdAtoms_->mdatoms(),
-            nrnb_, &vcm_, step != -1 ? wcycle_ : nullptr, energyElement_->enerdata(),
-            energyElement_->forceVirial(step), energyElement_->constraintVirial(step),
-            energyElement_->totalVirial(step), energyElement_->pressure(step), constr_, signaller,
-            lastbox, &totalNumberOfBondedInteractions_, energyElement_->needToSumEkinhOld(),
+            gstat_, cr_, inputrec_, fr_, energyData_->ekindata(), x, v, box, mdAtoms_->mdatoms(),
+            nrnb_, &vcm_, step != -1 ? wcycle_ : nullptr, energyData_->enerdata(),
+            energyData_->forceVirial(step), energyData_->constraintVirial(step),
+            energyData_->totalVirial(step), energyData_->pressure(step), constr_, signaller,
+            lastbox, &totalNumberOfBondedInteractions_, energyData_->needToSumEkinhOld(),
             flags | (shouldCheckNumberOfBondedInteractions_ ? CGLO_CHECK_NUMBER_OF_BONDED_INTERACTIONS : 0));
     checkNumberOfBondedInteractions(mdlog_, cr_, totalNumberOfBondedInteractions_, top_global_,
                                     localTopology_, x, box, &shouldCheckNumberOfBondedInteractions_);
index 26beb24180f143cf4c23ef80982e6d0dbb045e8f..02f4e1ab7a31575bc32b469924d070ab34168d7e 100644 (file)
@@ -47,7 +47,7 @@
 #include "gromacs/mdlib/simulationsignal.h"
 #include "gromacs/mdlib/vcm.h"
 
-#include "energyelement.h"
+#include "energydata.h"
 #include "modularsimulatorinterfaces.h"
 #include "statepropagatordata.h"
 #include "topologyholder.h"
@@ -107,7 +107,7 @@ class ComputeGlobalsElement final :
 public:
     //! Constructor
     ComputeGlobalsElement(StatePropagatorData*           statePropagatorData,
-                          EnergyElement*                 energyElement,
+                          EnergyData*                    energyData,
                           FreeEnergyPerturbationElement* freeEnergyPerturbationElement,
                           SimulationSignals*             signals,
                           int                            nstglobalcomm,
@@ -210,10 +210,11 @@ private:
     //! Global reduction struct
     gmx_global_stat* gstat_;
 
+    // TODO: Clarify relationship to data objects and find a more robust alternative to raw pointers (#3583)
     //! Pointer to the microstate
     StatePropagatorData* statePropagatorData_;
-    //! Pointer to the energy element (needed for the tensors and mu_tot)
-    EnergyElement* energyElement_;
+    //! Pointer to the energy data (needed for the tensors and mu_tot)
+    EnergyData* energyData_;
     //! Pointer to the local topology (only needed for checkNumberOfBondedInteractions)
     const gmx_localtop_t* localTopology_;
     //! Pointer to the free energy perturbation element
index 281d77ecd3a2692b3533b4a57c9670fe8119cc8e..9e7c7c8f43a22b5e2b210eeb82003d7b58dd4962 100644 (file)
@@ -49,7 +49,7 @@
 #include "gromacs/mdtypes/state.h"
 #include "gromacs/utility/fatalerror.h"
 
-#include "energyelement.h"
+#include "energydata.h"
 #include "freeenergyperturbationelement.h"
 #include "statepropagatordata.h"
 
@@ -58,7 +58,7 @@ namespace gmx
 template<ConstraintVariable variable>
 ConstraintsElement<variable>::ConstraintsElement(Constraints*                   constr,
                                                  StatePropagatorData*           statePropagatorData,
-                                                 EnergyElement*                 energyElement,
+                                                 EnergyData*                    energyData,
                                                  FreeEnergyPerturbationElement* freeEnergyPerturbationElement,
                                                  bool                           isMaster,
                                                  FILE*                          fplog,
@@ -69,7 +69,7 @@ ConstraintsElement<variable>::ConstraintsElement(Constraints*
     nextLogWritingStep_(-1),
     isMasterRank_(isMaster),
     statePropagatorData_(statePropagatorData),
-    energyElement_(energyElement),
+    energyData_(energyData),
     freeEnergyPerturbationElement_(freeEnergyPerturbationElement),
     constr_(constr),
     fplog_(fplog),
@@ -162,9 +162,9 @@ void ConstraintsElement<variable>::apply(Step step, bool calculateVirial, bool w
             // For some reason, the shake virial in VV is reset twice a step.
             // Energy element will only do this once per step.
             // TODO: Investigate this
-            clear_mat(energyElement_->constraintVirial(step));
+            clear_mat(energyData_->constraintVirial(step));
         }
-        energyElement_->addToConstraintVirial(vir_con, step);
+        energyData_->addToConstraintVirial(vir_con, step);
     }
 
     /* The factor of 2 correction is necessary because half of the constraint
@@ -175,7 +175,7 @@ void ConstraintsElement<variable>::apply(Step step, bool calculateVirial, bool w
      * Cf. Issue #1255
      */
     const real c_dvdlConstraintCorrectionFactor = EI_VV(inputrec_->eI) ? 2.0 : 1.0;
-    energyElement_->enerdata()->term[F_DVDL_CONSTR] += c_dvdlConstraintCorrectionFactor * dvdlambda;
+    energyData_->enerdata()->term[F_DVDL_CONSTR] += c_dvdlConstraintCorrectionFactor * dvdlambda;
 }
 
 template<ConstraintVariable variable>
index 14c6b34fc47d15ce1ecd4522b2d121c3644ae22e..88e7b359ce1fdcbde4f0043a0c1f654d6b17e167 100644 (file)
@@ -51,7 +51,7 @@
 namespace gmx
 {
 class Constraints;
-class EnergyElement;
+class EnergyData;
 class FreeEnergyPerturbationElement;
 class StatePropagatorData;
 
@@ -77,7 +77,7 @@ public:
     //! Constructor
     ConstraintsElement(Constraints*                   constr,
                        StatePropagatorData*           statePropagatorData,
-                       EnergyElement*                 energyElement,
+                       EnergyData*                    energyData,
                        FreeEnergyPerturbationElement* freeEnergyPerturbationElement,
                        bool                           isMaster,
                        FILE*                          fplog,
@@ -126,10 +126,11 @@ private:
     //! Whether we're master rank
     const bool isMasterRank_;
 
+    // TODO: Clarify relationship to data objects and find a more robust alternative to raw pointers (#3583)
     //! Pointer to the micro state
     StatePropagatorData* statePropagatorData_;
-    //! Pointer to the energy element
-    EnergyElement* energyElement_;
+    //! Pointer to the energy data
+    EnergyData* energyData_;
     //! Pointer to the free energy perturbation element
     FreeEnergyPerturbationElement* freeEnergyPerturbationElement_;
 
index f0de5d90375411d2cd0f9920cb0f51fdc92407d8..7d38ab88c4f0f714ec98fa051f500b66dc4c076f 100644 (file)
@@ -135,6 +135,7 @@ private:
     //! The global communication frequency
     const int nstglobalcomm_;
 
+    // TODO: Clarify relationship to data objects and find a more robust alternative to raw pointers (#3583)
     //! Pointer to the micro state
     StatePropagatorData* statePropagatorData_;
     //! Pointer to the topology
similarity index 77%
rename from src/gromacs/modularsimulator/energyelement.cpp
rename to src/gromacs/modularsimulator/energydata.cpp
index eb6de7e482cd75a2baf4cbf580d18b8da8299422..1146ee7f7cfbf57a53e04026a27505c75172589e 100644 (file)
@@ -41,7 +41,7 @@
 
 #include "gmxpre.h"
 
-#include "energyelement.h"
+#include "energydata.h"
 
 #include "gromacs/math/vec.h"
 #include "gromacs/mdlib/compute_io.h"
@@ -74,24 +74,22 @@ namespace gmx
 {
 class Awh;
 
-EnergyElement::EnergyElement(StatePropagatorData*           statePropagatorData,
-                             FreeEnergyPerturbationElement* freeEnergyPerturbationElement,
-                             const gmx_mtop_t*              globalTopology,
-                             const t_inputrec*              inputrec,
-                             const MDAtoms*                 mdAtoms,
-                             gmx_enerdata_t*                enerd,
-                             gmx_ekindata_t*                ekind,
-                             const Constraints*             constr,
-                             FILE*                          fplog,
-                             t_fcdata*                      fcd,
-                             const MdModulesNotifier&       mdModulesNotifier,
-                             bool                           isMasterRank,
-                             ObservablesHistory*            observablesHistory,
-                             StartingBehavior               startingBehavior) :
+EnergyData::EnergyData(StatePropagatorData*           statePropagatorData,
+                       FreeEnergyPerturbationElement* freeEnergyPerturbationElement,
+                       const gmx_mtop_t*              globalTopology,
+                       const t_inputrec*              inputrec,
+                       const MDAtoms*                 mdAtoms,
+                       gmx_enerdata_t*                enerd,
+                       gmx_ekindata_t*                ekind,
+                       const Constraints*             constr,
+                       FILE*                          fplog,
+                       t_fcdata*                      fcd,
+                       const MdModulesNotifier&       mdModulesNotifier,
+                       bool                           isMasterRank,
+                       ObservablesHistory*            observablesHistory,
+                       StartingBehavior               startingBehavior) :
+    element_(std::make_unique<Element>(this, isMasterRank)),
     isMasterRank_(isMasterRank),
-    energyWritingStep_(-1),
-    energyCalculationStep_(-1),
-    freeEnergyCalculationStep_(-1),
     forceVirialStep_(-1),
     shakeVirialStep_(-1),
     totalVirialStep_(-1),
@@ -126,7 +124,7 @@ EnergyElement::EnergyElement(StatePropagatorData*           statePropagatorData,
     }
 }
 
-void EnergyElement::scheduleTask(Step step, Time time, const RegisterRunFunctionPtr& registerRunFunction)
+void EnergyData::Element::scheduleTask(Step step, Time time, const RegisterRunFunctionPtr& registerRunFunction)
 {
     if (!isMasterRank_)
     {
@@ -139,17 +137,17 @@ void EnergyElement::scheduleTask(Step step, Time time, const RegisterRunFunction
     {
         (*registerRunFunction)(std::make_unique<SimulatorRunFunction>(
                 [this, time, isEnergyCalculationStep, isFreeEnergyCalculationStep]() {
-                    doStep(time, isEnergyCalculationStep, isFreeEnergyCalculationStep);
+                    energyData_->doStep(time, isEnergyCalculationStep, isFreeEnergyCalculationStep);
                 }));
     }
     else
     {
         (*registerRunFunction)(std::make_unique<SimulatorRunFunction>(
-                [this]() { energyOutput_->recordNonEnergyStep(); }));
+                [this]() { energyData_->energyOutput_->recordNonEnergyStep(); }));
     }
 }
 
-void EnergyElement::elementTeardown()
+void EnergyData::teardown()
 {
     if (inputrec_->nstcalcenergy > 0 && isMasterRank_)
     {
@@ -157,7 +155,12 @@ void EnergyElement::elementTeardown()
     }
 }
 
-void EnergyElement::trajectoryWriterSetup(gmx_mdoutf* outf)
+void EnergyData::Element::trajectoryWriterSetup(gmx_mdoutf* outf)
+{
+    energyData_->setup(outf);
+}
+
+void EnergyData::setup(gmx_mdoutf* outf)
 {
     pull_t* pull_work = nullptr;
     energyOutput_ = std::make_unique<EnergyOutput>(mdoutf_get_fp_ene(outf), top_global_, inputrec_,
@@ -193,18 +196,19 @@ void EnergyElement::trajectoryWriterSetup(gmx_mdoutf* outf)
     }
 }
 
-ITrajectoryWriterCallbackPtr EnergyElement::registerTrajectoryWriterCallback(TrajectoryEvent event)
+ITrajectoryWriterCallbackPtr EnergyData::Element::registerTrajectoryWriterCallback(TrajectoryEvent event)
 {
     if (event == TrajectoryEvent::EnergyWritingStep && isMasterRank_)
     {
         return std::make_unique<ITrajectoryWriterCallback>(
-                [this](gmx_mdoutf* mdoutf, Step step, Time time, bool writeTrajectory,
-                       bool writeLog) { write(mdoutf, step, time, writeTrajectory, writeLog); });
+                [this](gmx_mdoutf* mdoutf, Step step, Time time, bool writeTrajectory, bool writeLog) {
+                    energyData_->write(mdoutf, step, time, writeTrajectory, writeLog);
+                });
     }
     return nullptr;
 }
 
-SignallerCallbackPtr EnergyElement::registerTrajectorySignallerCallback(gmx::TrajectoryEvent event)
+SignallerCallbackPtr EnergyData::Element::registerTrajectorySignallerCallback(gmx::TrajectoryEvent event)
 {
     if (event == TrajectoryEvent::EnergyWritingStep && isMasterRank_)
     {
@@ -214,7 +218,7 @@ SignallerCallbackPtr EnergyElement::registerTrajectorySignallerCallback(gmx::Tra
     return nullptr;
 }
 
-SignallerCallbackPtr EnergyElement::registerEnergyCallback(EnergySignallerEvent event)
+SignallerCallbackPtr EnergyData::Element::registerEnergyCallback(EnergySignallerEvent event)
 {
     if (event == EnergySignallerEvent::EnergyCalculationStep && isMasterRank_)
     {
@@ -229,7 +233,7 @@ SignallerCallbackPtr EnergyElement::registerEnergyCallback(EnergySignallerEvent
     return nullptr;
 }
 
-void EnergyElement::doStep(Time time, bool isEnergyCalculationStep, bool isFreeEnergyCalculationStep)
+void EnergyData::doStep(Time time, bool isEnergyCalculationStep, bool isFreeEnergyCalculationStep)
 {
     enerd_->term[F_ETOT] = enerd_->term[F_EPOT] + enerd_->term[F_EKIN];
     if (vRescaleThermostat_)
@@ -259,7 +263,7 @@ void EnergyElement::doStep(Time time, bool isEnergyCalculationStep, bool isFreeE
                                        forceVirial_, totalVirial_, pressure_, ekind_, muTot_, constr_);
 }
 
-void EnergyElement::write(gmx_mdoutf* outf, Step step, Time time, bool writeTrajectory, bool writeLog)
+void EnergyData::write(gmx_mdoutf* outf, Step step, Time time, bool writeTrajectory, bool writeLog)
 {
     if (writeLog)
     {
@@ -275,7 +279,7 @@ void EnergyElement::write(gmx_mdoutf* outf, Step step, Time time, bool writeTraj
                                          writeLog ? fplog_ : nullptr, step, time, fcd_, awh);
 }
 
-void EnergyElement::addToForceVirial(const tensor virial, Step step)
+void EnergyData::addToForceVirial(const tensor virial, Step step)
 {
     if (step > forceVirialStep_)
     {
@@ -285,7 +289,7 @@ void EnergyElement::addToForceVirial(const tensor virial, Step step)
     m_add(forceVirial_, virial, forceVirial_);
 }
 
-void EnergyElement::addToConstraintVirial(const tensor virial, Step step)
+void EnergyData::addToConstraintVirial(const tensor virial, Step step)
 {
     if (step > shakeVirialStep_)
     {
@@ -295,7 +299,7 @@ void EnergyElement::addToConstraintVirial(const tensor virial, Step step)
     m_add(shakeVirial_, virial, shakeVirial_);
 }
 
-rvec* EnergyElement::forceVirial(Step gmx_unused step)
+rvec* EnergyData::forceVirial(Step gmx_unused step)
 {
     if (step > forceVirialStep_)
     {
@@ -307,7 +311,7 @@ rvec* EnergyElement::forceVirial(Step gmx_unused step)
     return forceVirial_;
 }
 
-rvec* EnergyElement::constraintVirial(Step gmx_unused step)
+rvec* EnergyData::constraintVirial(Step gmx_unused step)
 {
     if (step > shakeVirialStep_)
     {
@@ -319,7 +323,7 @@ rvec* EnergyElement::constraintVirial(Step gmx_unused step)
     return shakeVirial_;
 }
 
-rvec* EnergyElement::totalVirial(Step gmx_unused step)
+rvec* EnergyData::totalVirial(Step gmx_unused step)
 {
     if (step > totalVirialStep_)
     {
@@ -331,7 +335,7 @@ rvec* EnergyElement::totalVirial(Step gmx_unused step)
     return totalVirial_;
 }
 
-rvec* EnergyElement::pressure(Step gmx_unused step)
+rvec* EnergyData::pressure(Step gmx_unused step)
 {
     if (step > pressureStep_)
     {
@@ -343,46 +347,47 @@ rvec* EnergyElement::pressure(Step gmx_unused step)
     return pressure_;
 }
 
-real* EnergyElement::muTot()
+real* EnergyData::muTot()
 {
     return muTot_;
 }
 
-gmx_enerdata_t* EnergyElement::enerdata()
+gmx_enerdata_t* EnergyData::enerdata()
 {
     return enerd_;
 }
 
-gmx_ekindata_t* EnergyElement::ekindata()
+gmx_ekindata_t* EnergyData::ekindata()
 {
     return ekind_;
 }
 
-bool* EnergyElement::needToSumEkinhOld()
+bool* EnergyData::needToSumEkinhOld()
 {
     return &needToSumEkinhOld_;
 }
 
-void EnergyElement::writeCheckpoint(t_state gmx_unused* localState, t_state* globalState)
+void EnergyData::Element::writeCheckpoint(t_state gmx_unused* localState, t_state* globalState)
 {
     if (isMasterRank_)
     {
-        if (needToSumEkinhOld_)
+        if (energyData_->needToSumEkinhOld_)
         {
             globalState->ekinstate.bUpToDate = false;
         }
         else
         {
-            update_ekinstate(&globalState->ekinstate, ekind_);
+            update_ekinstate(&globalState->ekinstate, energyData_->ekind_);
             globalState->ekinstate.bUpToDate = true;
         }
-        energyOutput_->fillEnergyHistory(observablesHistory_->energyHistory.get());
+        energyData_->energyOutput_->fillEnergyHistory(
+                energyData_->observablesHistory_->energyHistory.get());
     }
 }
 
-void EnergyElement::initializeEnergyHistory(StartingBehavior    startingBehavior,
-                                            ObservablesHistory* observablesHistory,
-                                            EnergyOutput*       energyOutput)
+void EnergyData::initializeEnergyHistory(StartingBehavior    startingBehavior,
+                                         ObservablesHistory* observablesHistory,
+                                         EnergyOutput*       energyOutput)
 {
     if (startingBehavior != StartingBehavior::NewSimulation)
     {
@@ -424,7 +429,7 @@ void EnergyElement::initializeEnergyHistory(StartingBehavior    startingBehavior
     energyOutput->fillEnergyHistory(observablesHistory->energyHistory.get());
 }
 
-void EnergyElement::setVRescaleThermostat(const gmx::VRescaleThermostat* vRescaleThermostat)
+void EnergyData::setVRescaleThermostat(const gmx::VRescaleThermostat* vRescaleThermostat)
 {
     vRescaleThermostat_ = vRescaleThermostat;
     if (vRescaleThermostat_)
@@ -433,7 +438,7 @@ void EnergyElement::setVRescaleThermostat(const gmx::VRescaleThermostat* vRescal
     }
 }
 
-void EnergyElement::setParrinelloRahamnBarostat(const gmx::ParrinelloRahmanBarostat* parrinelloRahmanBarostat)
+void EnergyData::setParrinelloRahamnBarostat(const gmx::ParrinelloRahmanBarostat* parrinelloRahmanBarostat)
 {
     parrinelloRahmanBarostat_ = parrinelloRahmanBarostat;
     if (parrinelloRahmanBarostat_)
@@ -442,4 +447,18 @@ void EnergyElement::setParrinelloRahamnBarostat(const gmx::ParrinelloRahmanBaros
     }
 }
 
+EnergyData::Element* EnergyData::element()
+{
+    return element_.get();
+}
+
+EnergyData::Element::Element(EnergyData* energyData, bool isMasterRank) :
+    energyData_(energyData),
+    isMasterRank_(isMasterRank),
+    energyWritingStep_(-1),
+    energyCalculationStep_(-1),
+    freeEnergyCalculationStep_(-1)
+{
+}
+
 } // namespace gmx
similarity index 80%
rename from src/gromacs/modularsimulator/energyelement.h
rename to src/gromacs/modularsimulator/energydata.h
index b5a1b78a08c38d19e8f3e29187b25933a91350d8..935df8c51305d3378ef2b05485d39bcf6dee3308 100644 (file)
@@ -71,71 +71,48 @@ struct MdModulesNotifier;
 
 /*! \internal
  * \ingroup module_modularsimulator
- * \brief Element managing energies
+ * \brief Data class managing energies
  *
- * The EnergyElement owns the EnergyObject, and is hence responsible
- * for saving energy data and writing it to trajectory. It also owns
+ * The EnergyData owns the EnergyObject,
  * the tensors for the different virials and the pressure as well as
- * the total dipole vector.
+ * the total dipole vector. It has a member class which is part of the
+ * simulator loop and and is responsible
+ * for saving energy data and writing it to trajectory.
  *
- * It subscribes to the trajectory signaller, the energy signaller,
- * and the logging signaller to know when an energy calculation is
- * needed and when a non-recording step is enough. The simulator
- * builder is responsible to place the element in a location at
- * which a valid energy state is available. The EnergyElement is
- * also a subscriber to the trajectory writer element, as it is
- * responsible to write energy data to trajectory.
- *
- * The EnergyElement offers an interface to add virial contributions,
+ * The EnergyData offers an interface to add virial contributions,
  * but also allows access to the raw pointers to tensor data, the
  * dipole vector, and the legacy energy data structures.
+ *
+ * The EnergyData owns an object of type EnergyData::Element,
+ * which takes part in the simulation loop, allowing to record
+ * and output energies during the simulation.
  */
-class EnergyElement final :
-    public ISimulatorElement,
-    public ITrajectoryWriterClient,
-    public ITrajectorySignallerClient,
-    public IEnergySignallerClient,
-    public ICheckpointHelperClient
+class EnergyData final
 {
 public:
     //! Constructor
-    EnergyElement(StatePropagatorData*           statePropagatorData,
-                  FreeEnergyPerturbationElement* freeEnergyPerturbationElement,
-                  const gmx_mtop_t*              globalTopology,
-                  const t_inputrec*              inputrec,
-                  const MDAtoms*                 mdAtoms,
-                  gmx_enerdata_t*                enerd,
-                  gmx_ekindata_t*                ekind,
-                  const Constraints*             constr,
-                  FILE*                          fplog,
-                  t_fcdata*                      fcd,
-                  const MdModulesNotifier&       mdModulesNotifier,
-                  bool                           isMasterRank,
-                  ObservablesHistory*            observablesHistory,
-                  StartingBehavior               startingBehavior);
-
-    /*! \brief Register run function for step / time
-     *
-     * This needs to be called when the energies are at a full time step.
-     * Positioning this element is the responsibility of the programmer.
-     *
-     * This is also the place at which the current state becomes the previous
-     * state.
-     *
-     * @param step                 The step number
-     * @param time                 The time
-     * @param registerRunFunction  Function allowing to register a run function
-     */
-    void scheduleTask(Step step, Time time, const RegisterRunFunctionPtr& registerRunFunction) override;
-
-    //! No element setup needed
-    void elementSetup() override {}
+    EnergyData(StatePropagatorData*           statePropagatorData,
+               FreeEnergyPerturbationElement* freeEnergyPerturbationElement,
+               const gmx_mtop_t*              globalTopology,
+               const t_inputrec*              inputrec,
+               const MDAtoms*                 mdAtoms,
+               gmx_enerdata_t*                enerd,
+               gmx_ekindata_t*                ekind,
+               const Constraints*             constr,
+               FILE*                          fplog,
+               t_fcdata*                      fcd,
+               const MdModulesNotifier&       mdModulesNotifier,
+               bool                           isMasterRank,
+               ObservablesHistory*            observablesHistory,
+               StartingBehavior               startingBehavior);
 
     /*! \brief Final output
      *
-     * Prints the averages to log.
+     * Prints the averages to log. This is called from ModularSimulatorAlgorithm.
+     *
+     * \see ModularSimulatorAlgorithm::teardown
      */
-    void elementTeardown() override;
+    void teardown();
 
     /*! \brief Add contribution to force virial
      *
@@ -225,25 +202,19 @@ public:
                                         ObservablesHistory* observablesHistory,
                                         EnergyOutput*       energyOutput);
 
+    //! The element taking part in the simulator loop
+    class Element;
+    //! Get pointer to element (whose lifetime is managed by this)
+    Element* element();
+
 private:
     /*! \brief Setup (needs file pointer)
-     *
-     * ITrajectoryWriterClient implementation.
      *
      * Initializes the EnergyOutput object, and does some logging output.
      *
      * @param mdoutf  File pointer
      */
-    void trajectoryWriterSetup(gmx_mdoutf* mdoutf) override;
-    //! No trajectory writer teardown needed
-    void trajectoryWriterTeardown(gmx_mdoutf gmx_unused* outf) override {}
-
-    //! ITrajectoryWriterClient implementation.
-    SignallerCallbackPtr registerTrajectorySignallerCallback(TrajectoryEvent event) override;
-    //! ITrajectorySignallerClient implementation
-    ITrajectoryWriterCallbackPtr registerTrajectoryWriterCallback(TrajectoryEvent event) override;
-    //! IEnergySignallerClient implementation
-    SignallerCallbackPtr registerEnergyCallback(EnergySignallerEvent event) override;
+    void setup(gmx_mdoutf* mdoutf);
 
     /*! \brief Save data at energy steps
      *
@@ -259,23 +230,16 @@ private:
      */
     void write(gmx_mdoutf* outf, Step step, Time time, bool writeTrajectory, bool writeLog);
 
-    //! ICheckpointHelperClient implementation
-    void writeCheckpoint(t_state* localState, t_state* globalState) override;
-
     /*
-     * Data owned by EnergyElement
+     * Data owned by EnergyData
      */
+    //! The element
+    std::unique_ptr<Element> element_;
     //! The energy output object
     std::unique_ptr<EnergyOutput> energyOutput_;
 
     //! Whether this is the master rank
     const bool isMasterRank_;
-    //! The next communicated energy writing step
-    Step energyWritingStep_;
-    //! The next communicated energy calculation step
-    Step energyCalculationStep_;
-    //! The next communicated free energy calculation step
-    Step freeEnergyCalculationStep_;
 
     //! The force virial tensor
     tensor forceVirial_;
@@ -309,6 +273,7 @@ private:
     /*
      * Pointers to Simulator data
      */
+    // TODO: Clarify relationship to data objects and find a more robust alternative to raw pointers (#3583)
     //! Pointer to the state propagator data
     StatePropagatorData* statePropagatorData_;
     //! Pointer to the free energy perturbation element
@@ -341,6 +306,87 @@ private:
     ObservablesHistory* observablesHistory_;
 };
 
+/*! \internal
+ * \ingroup module_modularsimulator
+ * \brief Element for EnergyData
+ *
+ * This member class allows EnergyData to take part in the simulator
+ * loop.
+ *
+ * It subscribes to the trajectory signaller, the energy signaller,
+ * and the logging signaller to know when an energy calculation is
+ * needed and when a non-recording step is enough. The simulator
+ * builder is responsible to place the element in a location at
+ * which a valid energy state is available. The EnergyData::Element is
+ * also a subscriber to the trajectory writer element, as it is
+ * responsible to write energy data to trajectory.
+ */
+class EnergyData::Element final :
+    public ISimulatorElement,
+    public ITrajectoryWriterClient,
+    public ITrajectorySignallerClient,
+    public IEnergySignallerClient,
+    public ICheckpointHelperClient
+{
+public:
+    //! Constructor
+    Element(EnergyData* energyData, bool isMasterRank);
+
+    /*! \brief Register run function for step / time
+     *
+     * This needs to be called when the energies are at a full time step.
+     * Positioning this element is the responsibility of the programmer.
+     *
+     * This is also the place at which the current state becomes the previous
+     * state.
+     *
+     * @param step                 The step number
+     * @param time                 The time
+     * @param registerRunFunction  Function allowing to register a run function
+     */
+    void scheduleTask(Step step, Time time, const RegisterRunFunctionPtr& registerRunFunction) override;
+
+    //! No element setup needed
+    void elementSetup() override {}
+
+    //! No element teardown needed
+    void elementTeardown() override {}
+
+private:
+    EnergyData* energyData_;
+
+    /*! \brief Setup (needs file pointer)
+     *
+     * ITrajectoryWriterClient implementation.
+     *
+     * Initializes the EnergyOutput object, and does some logging output.
+     *
+     * @param mdoutf  File pointer
+     */
+    void trajectoryWriterSetup(gmx_mdoutf* mdoutf) override;
+    //! No trajectory writer teardown needed
+    void trajectoryWriterTeardown(gmx_mdoutf gmx_unused* outf) override {}
+
+    //! ITrajectoryWriterClient implementation.
+    SignallerCallbackPtr registerTrajectorySignallerCallback(TrajectoryEvent event) override;
+    //! ITrajectorySignallerClient implementation
+    ITrajectoryWriterCallbackPtr registerTrajectoryWriterCallback(TrajectoryEvent event) override;
+    //! IEnergySignallerClient implementation
+    SignallerCallbackPtr registerEnergyCallback(EnergySignallerEvent event) override;
+
+    //! ICheckpointHelperClient implementation
+    void writeCheckpoint(t_state* localState, t_state* globalState) override;
+
+    //! Whether this is the master rank
+    const bool isMasterRank_;
+    //! The next communicated energy writing step
+    Step energyWritingStep_;
+    //! The next communicated energy calculation step
+    Step energyCalculationStep_;
+    //! The next communicated free energy calculation step
+    Step freeEnergyCalculationStep_;
+};
+
 } // namespace gmx
 
 #endif // GMX_ENERGYELEMENT_MICROSTATE_H
index 86122ccf1280d0b87ea269e33d2e543c63a327d3..531686f26a12577b7cf6e161e59a0719cd8d5ec1 100644 (file)
@@ -54,7 +54,7 @@
 #include "gromacs/mdtypes/mdatom.h"
 #include "gromacs/pbcutil/pbc.h"
 
-#include "energyelement.h"
+#include "energydata.h"
 #include "freeenergyperturbationelement.h"
 #include "statepropagatordata.h"
 
@@ -66,7 +66,7 @@ class history_t;
 namespace gmx
 {
 ForceElement::ForceElement(StatePropagatorData*           statePropagatorData,
-                           EnergyElement*                 energyElement,
+                           EnergyData*                    energyData,
                            FreeEnergyPerturbationElement* freeEnergyPerturbationElement,
                            bool                           isVerbose,
                            bool                           isDynamicBox,
@@ -95,7 +95,7 @@ ForceElement::ForceElement(StatePropagatorData*           statePropagatorData,
     nextVirialCalculationStep_(-1),
     nextFreeEnergyCalculationStep_(-1),
     statePropagatorData_(statePropagatorData),
-    energyElement_(energyElement),
+    energyData_(energyData),
     freeEnergyPerturbationElement_(freeEnergyPerturbationElement),
     localTopology_(nullptr),
     isDynamicBox_(isDynamicBox),
@@ -190,9 +190,9 @@ void ForceElement::run(Step step, Time time, unsigned int flags)
         relax_shell_flexcon(
                 fplog_, cr_, ms, isVerbose_, enforcedRotation_, step, inputrec_, imdSession_,
                 pull_work_, step == nextNSStep_, static_cast<int>(flags), localTopology_, constr_,
-                energyElement_->enerdata(), statePropagatorData_->localNumAtoms(), x, v, box,
-                lambda, hist, forces, force_vir, mdAtoms_->mdatoms(), nrnb_, wcycle_, shellfc_, fr_,
-                runScheduleWork_, time, energyElement_->muTot(), vsite_, ddBalanceRegionHandler_);
+                energyData_->enerdata(), statePropagatorData_->localNumAtoms(), x, v, box, lambda,
+                hist, forces, force_vir, mdAtoms_->mdatoms(), nrnb_, wcycle_, shellfc_, fr_,
+                runScheduleWork_, time, energyData_->muTot(), vsite_, ddBalanceRegionHandler_);
         nShellRelaxationSteps_++;
     }
     else
@@ -202,11 +202,11 @@ void ForceElement::run(Step step, Time time, unsigned int flags)
         gmx_edsam* ed  = nullptr;
 
         do_force(fplog_, cr_, ms, inputrec_, awh, enforcedRotation_, imdSession_, pull_work_, step,
-                 nrnb_, wcycle_, localTopology_, box, x, hist, forces, force_vir, mdAtoms_->mdatoms(),
-                 energyElement_->enerdata(), lambda, fr_, runScheduleWork_, vsite_,
-                 energyElement_->muTot(), time, ed, static_cast<int>(flags), ddBalanceRegionHandler_);
+                 nrnb_, wcycle_, localTopology_, box, x, hist, forces, force_vir,
+                 mdAtoms_->mdatoms(), energyData_->enerdata(), lambda, fr_, runScheduleWork_, vsite_,
+                 energyData_->muTot(), time, ed, static_cast<int>(flags), ddBalanceRegionHandler_);
     }
-    energyElement_->addToForceVirial(force_vir, step);
+    energyData_->addToForceVirial(force_vir, step);
 }
 
 void ForceElement::elementTeardown()
index d2e9387ff5fb1dcca91ab2a79fba5ee129f203ca..561ce8829ba642a464e44d11d651b392d409606e 100644 (file)
@@ -65,7 +65,7 @@ struct t_nrnb;
 namespace gmx
 {
 class Awh;
-class EnergyElement;
+class EnergyData;
 class FreeEnergyPerturbationElement;
 class ImdSession;
 class MDAtoms;
@@ -89,7 +89,7 @@ class ForceElement final :
 public:
     //! Constructor
     ForceElement(StatePropagatorData*           statePropagatorData,
-                 EnergyElement*                 energyElement,
+                 EnergyData*                    energyData,
                  FreeEnergyPerturbationElement* freeEnergyPerturbationElement,
                  bool                           isVerbose,
                  bool                           isDynamicBox,
@@ -146,10 +146,11 @@ private:
     //! The next free energy calculation step
     Step nextFreeEnergyCalculationStep_;
 
+    // TODO: Clarify relationship to data objects and find a more robust alternative to raw pointers (#3583)
     //! Pointer to the micro state
     StatePropagatorData* statePropagatorData_;
-    //! Pointer to the energy element
-    EnergyElement* energyElement_;
+    //! Pointer to the energy data
+    EnergyData* energyData_;
     //! Pointer to the free energy perturbation element
     FreeEnergyPerturbationElement* freeEnergyPerturbationElement_;
 
index 68c6dc08ee162ad971566ccc8c6555c036483419..0f48566ce4f1477e30b8a97c431669e9581c6bb5 100644 (file)
@@ -77,7 +77,7 @@
 #include "compositesimulatorelement.h"
 #include "computeglobalselement.h"
 #include "constraintelement.h"
-#include "energyelement.h"
+#include "energydata.h"
 #include "forceelement.h"
 #include "freeenergyperturbationelement.h"
 #include "parrinellorahmanbarostat.h"
@@ -108,7 +108,7 @@ std::unique_ptr<ISimulatorElement> ModularSimulatorAlgorithmBuilder::buildForces
         SignallerBuilder<NeighborSearchSignaller>* neighborSearchSignallerBuilder,
         SignallerBuilder<EnergySignaller>*         energySignallerBuilder,
         StatePropagatorData*                       statePropagatorDataPtr,
-        EnergyElement*                             energyElementPtr,
+        EnergyData*                                energyDataPtr,
         FreeEnergyPerturbationElement*             freeEnergyPerturbationElement,
         TopologyHolder*                            topologyHolder)
 {
@@ -116,7 +116,7 @@ std::unique_ptr<ISimulatorElement> ModularSimulatorAlgorithmBuilder::buildForces
     const bool isDynamicBox = inputrecDynamicBox(inputrec);
 
     auto forceElement = std::make_unique<ForceElement>(
-            statePropagatorDataPtr, energyElementPtr, freeEnergyPerturbationElement, isVerbose,
+            statePropagatorDataPtr, energyDataPtr, freeEnergyPerturbationElement, isVerbose,
             isDynamicBox, fplog, cr, inputrec, mdAtoms, nrnb, fr, wcycle, runScheduleWork, vsite,
             imdSession, pull_work, constr, top_global, enforcedRotation);
     topologyHolder->registerClient(forceElement.get());
@@ -137,14 +137,14 @@ std::unique_ptr<ISimulatorElement> ModularSimulatorAlgorithmBuilder::buildIntegr
         std::vector<ICheckpointHelperClient*>*     checkpointClients,
         CheckBondedInteractionsCallbackPtr*        checkBondedInteractionsCallback,
         compat::not_null<StatePropagatorData*>     statePropagatorDataPtr,
-        compat::not_null<EnergyElement*>           energyElementPtr,
+        compat::not_null<EnergyData*>              energyDataPtr,
         FreeEnergyPerturbationElement*             freeEnergyPerturbationElementPtr,
         bool                                       hasReadEkinState,
         TopologyHolder*                            topologyHolder,
         SimulationSignals*                         signals)
 {
     auto forceElement = buildForces(neighborSearchSignallerBuilder, energySignallerBuilder,
-                                    statePropagatorDataPtr, energyElementPtr,
+                                    statePropagatorDataPtr, energyDataPtr,
                                     freeEnergyPerturbationElementPtr, topologyHolder);
 
     // list of elements owned by the simulator composite object
@@ -157,7 +157,7 @@ std::unique_ptr<ISimulatorElement> ModularSimulatorAlgorithmBuilder::buildIntegr
     {
         auto computeGlobalsElement =
                 std::make_unique<ComputeGlobalsElement<ComputeGlobalsAlgorithm::LeapFrog>>(
-                        statePropagatorDataPtr, energyElementPtr, freeEnergyPerturbationElementPtr,
+                        statePropagatorDataPtr, energyDataPtr, freeEnergyPerturbationElementPtr,
                         signals, nstglobalcomm_, fplog, mdlog, cr, inputrec, mdAtoms, nrnb, wcycle,
                         fr, top_global, constr, hasReadEkinState);
         topologyHolder->registerClient(computeGlobalsElement.get());
@@ -190,10 +190,10 @@ std::unique_ptr<ISimulatorElement> ModularSimulatorAlgorithmBuilder::buildIntegr
             auto thermostat = std::make_unique<VRescaleThermostat>(
                     inputrec->nsttcouple, -1, false, inputrec->ld_seed, inputrec->opts.ngtc,
                     inputrec->delta_t * inputrec->nsttcouple, inputrec->opts.ref_t, inputrec->opts.tau_t,
-                    inputrec->opts.nrdf, energyElementPtr, propagator->viewOnVelocityScaling(),
+                    inputrec->opts.nrdf, energyDataPtr, propagator->viewOnVelocityScaling(),
                     propagator->velocityScalingCallback(), state_global, cr, inputrec->bContinuation);
             checkpointClients->emplace_back(thermostat.get());
-            energyElementPtr->setVRescaleThermostat(thermostat.get());
+            energyDataPtr->setVRescaleThermostat(thermostat.get());
             addToCallListAndMove(std::move(thermostat), elementCallList, elementsOwnershipList);
         }
 
@@ -205,16 +205,16 @@ std::unique_ptr<ISimulatorElement> ModularSimulatorAlgorithmBuilder::buildIntegr
             prBarostat = std::make_unique<ParrinelloRahmanBarostat>(
                     inputrec->nstpcouple, -1, inputrec->delta_t * inputrec->nstpcouple,
                     inputrec->init_step, propagator->viewOnPRScalingMatrix(),
-                    propagator->prScalingCallback(), statePropagatorDataPtr, energyElementPtr,
-                    fplog, inputrec, mdAtoms, state_global, cr, inputrec->bContinuation);
-            energyElementPtr->setParrinelloRahamnBarostat(prBarostat.get());
+                    propagator->prScalingCallback(), statePropagatorDataPtr, energyDataPtr, fplog,
+                    inputrec, mdAtoms, state_global, cr, inputrec->bContinuation);
+            energyDataPtr->setParrinelloRahamnBarostat(prBarostat.get());
             checkpointClients->emplace_back(prBarostat.get());
         }
         addToCallListAndMove(std::move(propagator), elementCallList, elementsOwnershipList);
         if (constr)
         {
             auto constraintElement = std::make_unique<ConstraintsElement<ConstraintVariable::Positions>>(
-                    constr, statePropagatorDataPtr, energyElementPtr, freeEnergyPerturbationElementPtr,
+                    constr, statePropagatorDataPtr, energyDataPtr, freeEnergyPerturbationElementPtr,
                     MASTER(cr), fplog, inputrec, mdAtoms->mdatoms());
             auto constraintElementPtr = compat::make_not_null(constraintElement.get());
             energySignallerBuilder->registerSignallerClient(constraintElementPtr);
@@ -225,7 +225,13 @@ std::unique_ptr<ISimulatorElement> ModularSimulatorAlgorithmBuilder::buildIntegr
         }
 
         addToCallListAndMove(std::move(computeGlobalsElement), elementCallList, elementsOwnershipList);
-        addToCallList(energyElementPtr, elementCallList); // we have the energies at time t here!
+        auto energyElement = compat::make_not_null(energyDataPtr->element());
+        trajectoryElementBuilder->registerWriterClient(energyElement);
+        trajectorySignallerBuilder->registerSignallerClient(energyElement);
+        energySignallerBuilder->registerSignallerClient(energyElement);
+        checkpointClients->emplace_back(energyElement);
+        // we have the energies at time t here!
+        addToCallList(energyElement, elementCallList);
         if (prBarostat)
         {
             addToCallListAndMove(std::move(prBarostat), elementCallList, elementsOwnershipList);
@@ -235,7 +241,7 @@ std::unique_ptr<ISimulatorElement> ModularSimulatorAlgorithmBuilder::buildIntegr
     {
         auto computeGlobalsElement =
                 std::make_unique<ComputeGlobalsElement<ComputeGlobalsAlgorithm::VelocityVerlet>>(
-                        statePropagatorDataPtr, energyElementPtr, freeEnergyPerturbationElementPtr,
+                        statePropagatorDataPtr, energyDataPtr, freeEnergyPerturbationElementPtr,
                         signals, nstglobalcomm_, fplog, mdlog, cr, inputrec, mdAtoms, nrnb, wcycle,
                         fr, &topologyHolder->globalTopology(), constr, hasReadEkinState);
         topologyHolder->registerClient(computeGlobalsElement.get());
@@ -262,16 +268,16 @@ std::unique_ptr<ISimulatorElement> ModularSimulatorAlgorithmBuilder::buildIntegr
             prBarostat = std::make_unique<ParrinelloRahmanBarostat>(
                     inputrec->nstpcouple, -1, inputrec->delta_t * inputrec->nstpcouple,
                     inputrec->init_step, propagatorVelocities->viewOnPRScalingMatrix(),
-                    propagatorVelocities->prScalingCallback(), statePropagatorDataPtr, energyElementPtr,
+                    propagatorVelocities->prScalingCallback(), statePropagatorDataPtr, energyDataPtr,
                     fplog, inputrec, mdAtoms, state_global, cr, inputrec->bContinuation);
-            energyElementPtr->setParrinelloRahamnBarostat(prBarostat.get());
+            energyDataPtr->setParrinelloRahamnBarostat(prBarostat.get());
             checkpointClients->emplace_back(prBarostat.get());
         }
         addToCallListAndMove(std::move(propagatorVelocities), elementCallList, elementsOwnershipList);
         if (constr)
         {
             auto constraintElement = std::make_unique<ConstraintsElement<ConstraintVariable::Velocities>>(
-                    constr, statePropagatorDataPtr, energyElementPtr, freeEnergyPerturbationElementPtr,
+                    constr, statePropagatorDataPtr, energyDataPtr, freeEnergyPerturbationElementPtr,
                     MASTER(cr), fplog, inputrec, mdAtoms->mdatoms());
             energySignallerBuilder->registerSignallerClient(compat::make_not_null(constraintElement.get()));
             trajectorySignallerBuilder->registerSignallerClient(
@@ -300,12 +306,12 @@ std::unique_ptr<ISimulatorElement> ModularSimulatorAlgorithmBuilder::buildIntegr
             auto thermostat = std::make_unique<VRescaleThermostat>(
                     inputrec->nsttcouple, 0, true, inputrec->ld_seed, inputrec->opts.ngtc,
                     inputrec->delta_t * inputrec->nsttcouple, inputrec->opts.ref_t,
-                    inputrec->opts.tau_t, inputrec->opts.nrdf, energyElementPtr,
+                    inputrec->opts.tau_t, inputrec->opts.nrdf, energyDataPtr,
                     propagatorVelocitiesAndPositions->viewOnVelocityScaling(),
                     propagatorVelocitiesAndPositions->velocityScalingCallback(), state_global, cr,
                     inputrec->bContinuation);
             checkpointClients->emplace_back(thermostat.get());
-            energyElementPtr->setVRescaleThermostat(thermostat.get());
+            energyDataPtr->setVRescaleThermostat(thermostat.get());
             addToCallListAndMove(std::move(thermostat), elementCallList, elementsOwnershipList);
         }
         addToCallListAndMove(std::move(propagatorVelocitiesAndPositions), elementCallList,
@@ -313,7 +319,7 @@ std::unique_ptr<ISimulatorElement> ModularSimulatorAlgorithmBuilder::buildIntegr
         if (constr)
         {
             auto constraintElement = std::make_unique<ConstraintsElement<ConstraintVariable::Positions>>(
-                    constr, statePropagatorDataPtr, energyElementPtr, freeEnergyPerturbationElementPtr,
+                    constr, statePropagatorDataPtr, energyDataPtr, freeEnergyPerturbationElementPtr,
                     MASTER(cr), fplog, inputrec, mdAtoms->mdatoms());
             energySignallerBuilder->registerSignallerClient(compat::make_not_null(constraintElement.get()));
             trajectorySignallerBuilder->registerSignallerClient(
@@ -324,7 +330,13 @@ std::unique_ptr<ISimulatorElement> ModularSimulatorAlgorithmBuilder::buildIntegr
             addToCallListAndMove(std::move(constraintElement), elementCallList, elementsOwnershipList);
         }
         addToCallListAndMove(std::move(computeGlobalsElement), elementCallList, elementsOwnershipList);
-        addToCallList(energyElementPtr, elementCallList); // we have the energies at time t here!
+        auto energyElement = compat::make_not_null(energyDataPtr->element());
+        trajectoryElementBuilder->registerWriterClient(energyElement);
+        trajectorySignallerBuilder->registerSignallerClient(energyElement);
+        energySignallerBuilder->registerSignallerClient(energyElement);
+        checkpointClients->emplace_back(energyElement);
+        // we have the energies at time t here!
+        addToCallList(energyElement, elementCallList);
         if (prBarostat)
         {
             addToCallListAndMove(std::move(prBarostat), elementCallList, elementsOwnershipList);
index e1dda4b99cbb6acec535ca857d3b98ec2155cc3e..902e87ea5ac45e7c81a34f9751b4ff3415ab26d7 100644 (file)
@@ -54,7 +54,7 @@
 #include "gromacs/mdtypes/state.h"
 #include "gromacs/pbcutil/boxutilities.h"
 
-#include "energyelement.h"
+#include "energydata.h"
 #include "statepropagatordata.h"
 
 namespace gmx
@@ -67,7 +67,7 @@ ParrinelloRahmanBarostat::ParrinelloRahmanBarostat(int                   nstpcou
                                                    ArrayRef<rvec>        scalingTensor,
                                                    PropagatorCallbackPtr propagatorCallback,
                                                    StatePropagatorData*  statePropagatorData,
-                                                   EnergyElement*        energyElement,
+                                                   EnergyData*           energyData,
                                                    FILE*                 fplog,
                                                    const t_inputrec*     inputrec,
                                                    const MDAtoms*        mdAtoms,
@@ -81,7 +81,7 @@ ParrinelloRahmanBarostat::ParrinelloRahmanBarostat(int                   nstpcou
     scalingTensor_(scalingTensor),
     propagatorCallback_(std::move(propagatorCallback)),
     statePropagatorData_(statePropagatorData),
-    energyElement_(energyElement),
+    energyData_(energyData),
     fplog_(fplog),
     inputrec_(inputrec),
     mdAtoms_(mdAtoms)
@@ -131,7 +131,7 @@ void ParrinelloRahmanBarostat::scheduleTask(gmx::Step step,
 void ParrinelloRahmanBarostat::integrateBoxVelocityEquations(Step step)
 {
     auto box = statePropagatorData_->constBox();
-    parrinellorahman_pcoupl(fplog_, step, inputrec_, couplingTimeStep_, energyElement_->pressure(step),
+    parrinellorahman_pcoupl(fplog_, step, inputrec_, couplingTimeStep_, energyData_->pressure(step),
                             box, boxRel_, boxVelocity_, scalingTensor_.data(), mu_, false);
     // multiply matrix by the coupling time step to avoid having the propagator needing to know about that
     msmul(scalingTensor_.data(), couplingTimeStep_, scalingTensor_.data());
index 4423bf87a19ca7bf2a44868e5a5d10b48e5373e5..c4a189a5a023bac5c362dff071eaa0d323f31740 100644 (file)
@@ -54,7 +54,7 @@ struct t_commrec;
 
 namespace gmx
 {
-class EnergyElement;
+class EnergyData;
 class MDAtoms;
 class StatePropagatorData;
 
@@ -79,7 +79,7 @@ public:
                              ArrayRef<rvec>        scalingTensor,
                              PropagatorCallbackPtr propagatorCallback,
                              StatePropagatorData*  statePropagatorData,
-                             EnergyElement*        energyElement,
+                             EnergyData*           energyData,
                              FILE*                 fplog,
                              const t_inputrec*     inputrec,
                              const MDAtoms*        mdAtoms,
@@ -125,10 +125,11 @@ private:
     //! Box velocity
     tensor boxVelocity_;
 
+    // TODO: Clarify relationship to data objects and find a more robust alternative to raw pointers (#3583)
     //! Pointer to the micro state
     StatePropagatorData* statePropagatorData_;
-    //! Pointer to the energy element
-    EnergyElement* energyElement_;
+    //! Pointer to the energy data
+    EnergyData* energyData_;
 
     //! Integrate the PR box vector equations of motion - does not alter state
     void integrateBoxVelocityEquations(Step step);
index cbb78424dfa4402270fdfaa89d44c98771b18274..e71008b9876e6bc87a417587e93007e9c0ec229d 100644 (file)
@@ -115,6 +115,7 @@ private:
     //! Whether PME load balancing printing is active \todo Check this!
     bool bPMETunePrinting_;
 
+    // TODO: Clarify relationship to data objects and find a more robust alternative to raw pointers (#3583)
     //! Pointer to the micro state
     StatePropagatorData* statePropagatorData_;
 
index 3528fbd322925910d8ebcd8ca55d4f3da52b04ae..1175c611b318593230762e36797525dc429fa681 100644 (file)
@@ -161,6 +161,7 @@ private:
     //! The time step
     const real timestep_;
 
+    // TODO: Clarify relationship to data objects and find a more robust alternative to raw pointers (#3583)
     //! Pointer to the micro state
     StatePropagatorData* statePropagatorData_;
 
index 76ba0b658d477f11344372c0e55144db4e08411f..b0e27018cb0161876e4d4378ada8dc3e216b95ec 100644 (file)
@@ -98,6 +98,7 @@ ModularSimulatorAlgorithm::ModularSimulatorAlgorithm(std::string              to
                                                      gmx_walltime_accounting* walltime_accounting) :
     taskIterator_(taskQueue_.end()),
     statePropagatorData_(nullptr),
+    energyData_(nullptr),
     step_(-1),
     runFinished_(false),
     topologyName_(std::move(topologyName)),
@@ -171,6 +172,7 @@ void ModularSimulatorAlgorithm::teardown()
     {
         element->elementTeardown();
     }
+    energyData_->teardown();
     if (pmeLoadBalanceHelper_)
     {
         pmeLoadBalanceHelper_->teardown();
@@ -428,11 +430,11 @@ ModularSimulatorAlgorithm ModularSimulatorAlgorithmBuilder::constructElementsAnd
             top_global->natoms, cr, state_global, fr->nbv->useGpu(), inputrec, mdAtoms->mdatoms());
     auto statePropagatorDataPtr = compat::make_not_null(algorithm.statePropagatorData_.get());
 
-    auto energyElement = std::make_unique<EnergyElement>(
+    algorithm.energyData_ = std::make_unique<EnergyData>(
             statePropagatorDataPtr, freeEnergyPerturbationElementPtr, top_global, inputrec, mdAtoms,
             enerd, ekind, constr, fplog, &fr->listedForces->fcdata(), mdModulesNotifier, MASTER(cr),
             observablesHistory, startingBehavior);
-    auto energyElementPtr = compat::make_not_null(energyElement.get());
+    auto energyDataPtr = compat::make_not_null(algorithm.energyData_.get());
 
     /*
      * Build stop handler
@@ -454,14 +456,6 @@ ModularSimulatorAlgorithm ModularSimulatorAlgorithmBuilder::constructElementsAnd
     SignallerBuilder<TrajectorySignaller>     trajectorySignallerBuilder;
     TrajectoryElementBuilder                  trajectoryElementBuilder;
 
-    /*
-     * Register data structures to signallers
-     */
-
-    trajectoryElementBuilder.registerWriterClient(energyElementPtr);
-    trajectorySignallerBuilder.registerSignallerClient(energyElementPtr);
-    energySignallerBuilder.registerSignallerClient(energyElementPtr);
-
     // Register the simulator itself to the neighbor search / last step signaller
     neighborSearchSignallerBuilder.registerSignallerClient(
             compat::make_not_null(algorithm.signalHelper_.get()));
@@ -473,14 +467,13 @@ ModularSimulatorAlgorithm ModularSimulatorAlgorithmBuilder::constructElementsAnd
      * have a full timestep state.
      */
     // TODO: Make a CheckpointHelperBuilder
-    std::vector<ICheckpointHelperClient*> checkpointClients               = { energyElementPtr,
-                                                                freeEnergyPerturbationElementPtr };
+    std::vector<ICheckpointHelperClient*> checkpointClients = { freeEnergyPerturbationElementPtr };
     CheckBondedInteractionsCallbackPtr    checkBondedInteractionsCallback = nullptr;
     auto                                  integrator                      = buildIntegrator(
             &neighborSearchSignallerBuilder, &lastStepSignallerBuilder, &energySignallerBuilder,
             &loggingSignallerBuilder, &trajectorySignallerBuilder, &trajectoryElementBuilder,
             &checkpointClients, &checkBondedInteractionsCallback, statePropagatorDataPtr,
-            energyElementPtr, freeEnergyPerturbationElementPtr, hasReadEkinState,
+            energyDataPtr, freeEnergyPerturbationElementPtr, hasReadEkinState,
             algorithm.topologyHolder_.get(), &algorithm.signals_);
 
     /*
@@ -585,7 +578,6 @@ ModularSimulatorAlgorithm ModularSimulatorAlgorithmBuilder::constructElementsAnd
     addToCallListAndMove(std::move(integrator), algorithm.elementCallList_, algorithm.elementsOwnershipList_);
     addToCallListAndMove(std::move(trajectoryElement), algorithm.elementCallList_,
                          algorithm.elementsOwnershipList_);
-    algorithm.elementsOwnershipList_.emplace_back(std::move(energyElement));
 
     return algorithm;
 }
index b1c57a3f133e603a9e7302a4035c961d47ccf701..74e7e63d3a570ee857da34b857d433086da26cef 100644 (file)
@@ -60,7 +60,7 @@
 
 namespace gmx
 {
-class EnergyElement;
+class EnergyData;
 class EnergySignaller;
 class FreeEnergyPerturbationElement;
 class LoggingSignaller;
@@ -199,6 +199,8 @@ private:
     // Data structures
     //! The state propagator data
     std::unique_ptr<StatePropagatorData> statePropagatorData_;
+    //! The energy data
+    std::unique_ptr<EnergyData> energyData_;
 
     //! The current step
     Step step_;
@@ -306,7 +308,7 @@ private:
                     std::vector<ICheckpointHelperClient*>*     checkpointClients,
                     CheckBondedInteractionsCallbackPtr*        checkBondedInteractionsCallback,
                     compat::not_null<StatePropagatorData*>     statePropagatorDataPtr,
-                    compat::not_null<EnergyElement*>           energyElementPtr,
+                    compat::not_null<EnergyData*>              energyDataPtr,
                     FreeEnergyPerturbationElement*             freeEnergyPerturbationElementPtr,
                     bool                                       hasReadEkinState,
                     TopologyHolder*                            topologyHolder,
@@ -317,7 +319,7 @@ private:
     buildForces(SignallerBuilder<NeighborSearchSignaller>* neighborSearchSignallerBuilder,
                 SignallerBuilder<EnergySignaller>*         energySignallerBuilder,
                 StatePropagatorData*                       statePropagatorDataPtr,
-                EnergyElement*                             energyElementPtr,
+                EnergyData*                                energyDataPtr,
                 FreeEnergyPerturbationElement*             freeEnergyPerturbationElement,
                 TopologyHolder*                            topologyHolder);
 
index 35c7bbd8b35ebe422792724a6d4aafae99d91694..8d5560dd260b3732c82bdb0a93ead74da51d46c7 100644 (file)
@@ -314,6 +314,7 @@ private:
     //! Callback writing the state to file
     void write(gmx_mdoutf* outf, Step step, Time time);
 
+    // TODO: Clarify relationship to data objects and find a more robust alternative to raw pointers (#3583)
     //! Pointer to the free energy perturbation element (for trajectory writing only)
     FreeEnergyPerturbationElement* freeEnergyPerturbationElement_;
 
index 9aa43be58d511893af2176088d1a86ad9093ae79..d437db21769b05490809e39b9a969c95aec613a9 100644 (file)
@@ -65,7 +65,7 @@ VRescaleThermostat::VRescaleThermostat(int                   nstcouple,
                                        const real*           referenceTemperature,
                                        const real*           couplingTime,
                                        const real*           numDegreesOfFreedom,
-                                       EnergyElement*        energyElement,
+                                       EnergyData*           energyData,
                                        ArrayRef<real>        lambdaView,
                                        PropagatorCallbackPtr propagatorCallback,
                                        const t_state*        globalState,
@@ -81,7 +81,7 @@ VRescaleThermostat::VRescaleThermostat(int                   nstcouple,
     couplingTime_(couplingTime, couplingTime + numTemperatureGroups),
     numDegreesOfFreedom_(numDegreesOfFreedom, numDegreesOfFreedom + numTemperatureGroups),
     thermostatIntegral_(numTemperatureGroups, 0.0),
-    energyElement_(energyElement),
+    energyData_(energyData),
     lambda_(lambdaView),
     propagatorCallback_(std::move(propagatorCallback))
 {
@@ -128,7 +128,7 @@ void VRescaleThermostat::setLambda(Step step)
 {
     real currentKineticEnergy, referenceKineticEnergy, newKineticEnergy;
 
-    auto ekind = energyElement_->ekindata();
+    auto ekind = energyData_->ekindata();
 
     for (int i = 0; (i < numTemperatureGroups_); i++)
     {
index 49f3c737bfcdb94d80d8da8c1f417c1c6efd1e1f..9c5906eb76de6d3bf7bc6e1e68e565feb4746a4d 100644 (file)
@@ -46,7 +46,7 @@
 
 #include "gromacs/utility/arrayref.h"
 
-#include "energyelement.h"
+#include "energydata.h"
 #include "modularsimulatorinterfaces.h"
 #include "propagator.h"
 
@@ -75,7 +75,7 @@ public:
                        const real*           referenceTemperature,
                        const real*           couplingTime,
                        const real*           numDegreesOfFreedom,
-                       EnergyElement*        energyElement,
+                       EnergyData*           energyData,
                        ArrayRef<real>        lambdaView,
                        PropagatorCallbackPtr propagatorCallback,
                        const t_state*        globalState,
@@ -121,8 +121,9 @@ private:
     //! Work exerted by thermostat
     std::vector<double> thermostatIntegral_;
 
-    //! Pointer to the energy element (for ekindata)
-    EnergyElement* energyElement_;
+    // TODO: Clarify relationship to data objects and find a more robust alternative to raw pointers (#3583)
+    //! Pointer to the energy data (for ekindata)
+    EnergyData* energyData_;
 
     //! View on the scaling factor of the propagator
     ArrayRef<real> lambda_;