Modular simulator: Separate trajectory element and signaller
authorPascal Merz <pascal.merz@me.com>
Thu, 18 Jun 2020 18:53:59 +0000 (18:53 +0000)
committerPascal Merz <pascal.merz@me.com>
Thu, 18 Jun 2020 18:53:59 +0000 (18:53 +0000)
The trajectory element implemented both the ISimulatorElement and
the ISignaller interface. It was both signalling its clients that a
trajectory writing step was about to occur during task list creation,
and actually performing trajectory writing during the simulation.

In general, the tasks of signallers and elements, their call site in
the code, and their build and ownership procedures are all significantly
different. The dual nature of the trajectory element makes this
differentiation less clear, and requires some workarounds. Additionally,
separating the trajectory element in separate element and a signaller
requires no changes on client side, since all clients of the trajectory
element with integrated signaller already implemented two interfaces,
ITrajectorySignallerClient and ITrajectoryWriterClient.

The current change hence splits the current trajectory element into
a pure element and a pure signaller. This will make subsequent changes,
especially the introduction of a builder approach much easier. It also
removes support for combined elements and signallers: It renames the setup
method of the ISignaller interface from `signallerSetup` to `setup`, as
this differentiation was only used for classes implementing both element
and signaller interfaces. It also simplifies the handling of signallers
in ModularSimulator, as having a separate ownership and call list is not
needed anymore.

Refs #3437

src/gromacs/modularsimulator/modularsimulator.cpp
src/gromacs/modularsimulator/modularsimulator.h
src/gromacs/modularsimulator/modularsimulatorinterfaces.h
src/gromacs/modularsimulator/signallers.cpp
src/gromacs/modularsimulator/signallers.h
src/gromacs/modularsimulator/trajectoryelement.cpp
src/gromacs/modularsimulator/trajectoryelement.h

index fa05c5f55031a5d23297d84b5dc0502d56cf8f11..a64b56e03a4af0b55e1f588d810ffcd2fd40370d 100644 (file)
@@ -99,9 +99,9 @@ void ModularSimulator::run()
     GMX_LOG(mdlog.info).asParagraph().appendText("Using the modular simulator.");
     constructElementsAndSignallers();
     simulatorSetup();
-    for (auto& signaller : signallerCallList_)
+    for (auto& signaller : signallerList_)
     {
-        signaller->signallerSetup();
+        signaller->setup();
     }
     if (domDecHelper_)
     {
@@ -281,7 +281,7 @@ void ModularSimulator::populateTaskQueue()
     Time time      = startTime + step_ * timeStep;
 
     // Run an initial call to the signallers
-    for (auto& signaller : signallerCallList_)
+    for (auto& signaller : signallerList_)
     {
         signaller->signal(step_, time);
     }
@@ -321,7 +321,7 @@ void ModularSimulator::populateTaskQueue()
         // prepare next step
         step_++;
         time = startTime + step_ * timeStep;
-        for (auto& signaller : signallerCallList_)
+        for (auto& signaller : signallerList_)
         {
             signaller->signal(step_, time);
         }
@@ -402,17 +402,18 @@ void ModularSimulator::constructElementsAndSignallers()
     SignallerBuilder<LastStepSignaller>       lastStepSignallerBuilder;
     SignallerBuilder<LoggingSignaller>        loggingSignallerBuilder;
     SignallerBuilder<EnergySignaller>         energySignallerBuilder;
+    SignallerBuilder<TrajectorySignaller>     trajectorySignallerBuilder;
     TrajectoryElementBuilder                  trajectoryElementBuilder;
 
     /*
      * Register data structures to signallers
      */
     trajectoryElementBuilder.registerWriterClient(statePropagatorDataPtr);
-    trajectoryElementBuilder.registerSignallerClient(statePropagatorDataPtr);
+    trajectorySignallerBuilder.registerSignallerClient(statePropagatorDataPtr);
     lastStepSignallerBuilder.registerSignallerClient(statePropagatorDataPtr);
 
     trajectoryElementBuilder.registerWriterClient(energyElementPtr);
-    trajectoryElementBuilder.registerSignallerClient(energyElementPtr);
+    trajectorySignallerBuilder.registerSignallerClient(energyElementPtr);
     energySignallerBuilder.registerSignallerClient(energyElementPtr);
 
     // Register the simulator itself to the neighbor search / last step signaller
@@ -428,11 +429,11 @@ void ModularSimulator::constructElementsAndSignallers()
     std::vector<ICheckpointHelperClient*> checkpointClients = { statePropagatorDataPtr, energyElementPtr,
                                                                 freeEnergyPerturbationElementPtr };
     CheckBondedInteractionsCallbackPtr checkBondedInteractionsCallback = nullptr;
-    auto                               integrator =
-            buildIntegrator(&neighborSearchSignallerBuilder, &energySignallerBuilder,
-                            &loggingSignallerBuilder, &trajectoryElementBuilder, &checkpointClients,
-                            &checkBondedInteractionsCallback, statePropagatorDataPtr,
-                            energyElementPtr, freeEnergyPerturbationElementPtr, hasReadEkinState);
+    auto integrator = buildIntegrator(&neighborSearchSignallerBuilder, &energySignallerBuilder,
+                                      &loggingSignallerBuilder, &trajectorySignallerBuilder,
+                                      &checkpointClients, &checkBondedInteractionsCallback,
+                                      statePropagatorDataPtr, energyElementPtr,
+                                      freeEnergyPerturbationElementPtr, hasReadEkinState);
 
     /*
      * Build infrastructure elements
@@ -474,12 +475,18 @@ void ModularSimulator::constructElementsAndSignallers()
      */
     auto energySignaller = energySignallerBuilder.build(
             inputrec->nstcalcenergy, inputrec->fepvals->nstdhdl, inputrec->nstpcouple);
-    trajectoryElementBuilder.registerSignallerClient(compat::make_not_null(energySignaller.get()));
+    trajectorySignallerBuilder.registerSignallerClient(compat::make_not_null(energySignaller.get()));
     loggingSignallerBuilder.registerSignallerClient(compat::make_not_null(energySignaller.get()));
     auto trajectoryElement = trajectoryElementBuilder.build(
             fplog, nfile, fnm, mdrunOptions, cr, outputProvider, mdModulesNotifier, inputrec,
             top_global, oenv, wcycle, startingBehavior, simulationsShareState);
     loggingSignallerBuilder.registerSignallerClient(compat::make_not_null(trajectoryElement.get()));
+    trajectorySignallerBuilder.registerSignallerClient(compat::make_not_null(trajectoryElement.get()));
+    auto trajectorySignaller = trajectorySignallerBuilder.build(
+            inputrec->nstxout, inputrec->nstvout, inputrec->nstfout, inputrec->nstxout_compressed,
+            trajectoryElement->tngBoxOut(), trajectoryElement->tngLambdaOut(),
+            trajectoryElement->tngBoxOutCompressed(), trajectoryElement->tngLambdaOutCompressed(),
+            inputrec->nstenergy);
 
     // Add checkpoint helper here since we need a pointer to the trajectory element and
     // need to register it with the lastStepSignallerBuilder
@@ -493,7 +500,7 @@ void ModularSimulator::constructElementsAndSignallers()
             walltime_accounting, state_global, mdrunOptions.writeConfout);
     lastStepSignallerBuilder.registerSignallerClient(compat::make_not_null(checkpointHelper_.get()));
 
-    lastStepSignallerBuilder.registerSignallerClient(compat::make_not_null(trajectoryElement.get()));
+    lastStepSignallerBuilder.registerSignallerClient(compat::make_not_null(trajectorySignaller.get()));
     auto loggingSignaller =
             loggingSignallerBuilder.build(inputrec->nstlog, inputrec->init_step, inputrec->init_t);
     lastStepSignallerBuilder.registerSignallerClient(compat::make_not_null(loggingSignaller.get()));
@@ -503,11 +510,11 @@ void ModularSimulator::constructElementsAndSignallers()
     auto neighborSearchSignaller = neighborSearchSignallerBuilder.build(
             inputrec->nstlist, inputrec->init_step, inputrec->init_t);
 
-    addToCallListAndMove(std::move(neighborSearchSignaller), signallerCallList_, signallersOwnershipList_);
-    addToCallListAndMove(std::move(lastStepSignaller), signallerCallList_, signallersOwnershipList_);
-    addToCallListAndMove(std::move(loggingSignaller), signallerCallList_, signallersOwnershipList_);
-    addToCallList(trajectoryElement, signallerCallList_);
-    addToCallListAndMove(std::move(energySignaller), signallerCallList_, signallersOwnershipList_);
+    signallerList_.emplace_back(std::move(neighborSearchSignaller));
+    signallerList_.emplace_back(std::move(lastStepSignaller));
+    signallerList_.emplace_back(std::move(loggingSignaller));
+    signallerList_.emplace_back(std::move(trajectorySignaller));
+    signallerList_.emplace_back(std::move(energySignaller));
 
     /*
      * Build the element list
@@ -559,7 +566,7 @@ std::unique_ptr<ISimulatorElement> ModularSimulator::buildIntegrator(
         SignallerBuilder<NeighborSearchSignaller>* neighborSearchSignallerBuilder,
         SignallerBuilder<EnergySignaller>*         energySignallerBuilder,
         SignallerBuilder<LoggingSignaller>*        loggingSignallerBuilder,
-        TrajectoryElementBuilder*                  trajectoryElementBuilder,
+        SignallerBuilder<TrajectorySignaller>*     trajectorySignallerBuilder,
         std::vector<ICheckpointHelperClient*>*     checkpointClients,
         CheckBondedInteractionsCallbackPtr*        checkBondedInteractionsCallback,
         compat::not_null<StatePropagatorData*>     statePropagatorDataPtr,
@@ -586,7 +593,7 @@ std::unique_ptr<ISimulatorElement> ModularSimulator::buildIntegrator(
                         wcycle, fr, &topologyHolder_->globalTopology(), constr, hasReadEkinState);
         topologyHolder_->registerClient(computeGlobalsElement.get());
         energySignallerBuilder->registerSignallerClient(compat::make_not_null(computeGlobalsElement.get()));
-        trajectoryElementBuilder->registerSignallerClient(
+        trajectorySignallerBuilder->registerSignallerClient(
                 compat::make_not_null(computeGlobalsElement.get()));
 
         *checkBondedInteractionsCallback =
@@ -633,7 +640,7 @@ std::unique_ptr<ISimulatorElement> ModularSimulator::buildIntegrator(
                     MASTER(cr), fplog, inputrec, mdAtoms->mdatoms());
             auto constraintElementPtr = compat::make_not_null(constraintElement.get());
             energySignallerBuilder->registerSignallerClient(constraintElementPtr);
-            trajectoryElementBuilder->registerSignallerClient(constraintElementPtr);
+            trajectorySignallerBuilder->registerSignallerClient(constraintElementPtr);
             loggingSignallerBuilder->registerSignallerClient(constraintElementPtr);
 
             addToCallListAndMove(std::move(constraintElement), elementCallList, elementsOwnershipList);
@@ -655,7 +662,7 @@ std::unique_ptr<ISimulatorElement> ModularSimulator::buildIntegrator(
                         wcycle, fr, &topologyHolder_->globalTopology(), constr, hasReadEkinState);
         topologyHolder_->registerClient(computeGlobalsElement.get());
         energySignallerBuilder->registerSignallerClient(compat::make_not_null(computeGlobalsElement.get()));
-        trajectoryElementBuilder->registerSignallerClient(
+        trajectorySignallerBuilder->registerSignallerClient(
                 compat::make_not_null(computeGlobalsElement.get()));
 
         *checkBondedInteractionsCallback =
@@ -689,7 +696,7 @@ std::unique_ptr<ISimulatorElement> ModularSimulator::buildIntegrator(
                     constr, statePropagatorDataPtr, energyElementPtr, freeEnergyPerturbationElementPtr,
                     MASTER(cr), fplog, inputrec, mdAtoms->mdatoms());
             energySignallerBuilder->registerSignallerClient(compat::make_not_null(constraintElement.get()));
-            trajectoryElementBuilder->registerSignallerClient(
+            trajectorySignallerBuilder->registerSignallerClient(
                     compat::make_not_null(constraintElement.get()));
             loggingSignallerBuilder->registerSignallerClient(
                     compat::make_not_null(constraintElement.get()));
@@ -722,7 +729,7 @@ std::unique_ptr<ISimulatorElement> ModularSimulator::buildIntegrator(
                     constr, statePropagatorDataPtr, energyElementPtr, freeEnergyPerturbationElementPtr,
                     MASTER(cr), fplog, inputrec, mdAtoms->mdatoms());
             energySignallerBuilder->registerSignallerClient(compat::make_not_null(constraintElement.get()));
-            trajectoryElementBuilder->registerSignallerClient(
+            trajectorySignallerBuilder->registerSignallerClient(
                     compat::make_not_null(constraintElement.get()));
             loggingSignallerBuilder->registerSignallerClient(
                     compat::make_not_null(constraintElement.get()));
index a9c0058d2a608a1a4ddf87e495bee5e01021a8c0..f5f6bb4e99a3769c13238f766992adb6d4b07c03 100644 (file)
@@ -152,7 +152,7 @@ private:
     buildIntegrator(SignallerBuilder<NeighborSearchSignaller>* neighborSearchSignallerBuilder,
                     SignallerBuilder<EnergySignaller>*         energySignallerBuilder,
                     SignallerBuilder<LoggingSignaller>*        loggingSignallerBuilder,
-                    TrajectoryElementBuilder*                  trajectoryElementBuilder,
+                    SignallerBuilder<TrajectorySignaller>*     trajectorySignallerBuilder,
                     std::vector<ICheckpointHelperClient*>*     checkpointClients,
                     CheckBondedInteractionsCallbackPtr*        checkBondedInteractionsCallback,
                     compat::not_null<StatePropagatorData*>     statePropagatorDataPtr,
@@ -193,27 +193,17 @@ private:
     std::queue<SimulatorRunFunctionPtr> taskQueue_;
 
     /* Note that the Simulator is owning the signallers and elements.
-     * The ownership list and the call list are kept separate, however,
-     * to allow to have elements more than once in the call lists -
-     * either as signaller AND element (such as the TrajectoryElement),
-     * or to have an element twice in the scheduling sequence (currently
-     * not used).
+     * The ownership list and the call list of the elements are kept
+     * separate, to allow to have elements more than once in the call
+     * lists - for example, using velocity verlet, the compute globals
+     * element needs to be scheduled more than once per step. For the
+     * signallers, no distinction between ownership and call list is
+     * made, all signallers are called exactly once per scheduling step.
      *
-     * For the elements, the setup and teardown is applied on the
-     * elementsOwnershipList_, to ensure that it is called only once per
-     * element. For the signallers, the setup is applied on the
-     * signallerCallList_ - this makes sure that both the elementSetup()
-     * and signallerSetup() of an object being both an element and a
-     * signaller is called. It is also not expected to run have a signaller
-     * more than once in the signallerCallList_, so we don't have to worry
-     * about calling the setup method twice. Consequently, this means that
-     * objects being both a signaller and an element should be stored in
-     * the elementsOwnershipList_.
+     * Objects being both a signaller and an element are not supported.
      */
-    //! List of signalers (ownership)
-    std::vector<std::unique_ptr<ISignaller>> signallersOwnershipList_;
-    //! List of signalers (calling sequence)
-    std::vector<compat::not_null<ISignaller*>> signallerCallList_;
+    //! List of signalers (ownership and calling sequence)
+    std::vector<std::unique_ptr<ISignaller>> signallerList_;
     //! List of schedulerElements (ownership)
     std::vector<std::unique_ptr<ISimulatorElement>> elementsOwnershipList_;
     //! List of schedulerElements (calling sequence)
index de1e152718d425fe24c408291018af4c7908c2d6..6f0e0cce49e362dc8b382b02b7af92734f7b9f73 100644 (file)
@@ -70,6 +70,7 @@ class SignallerBuilder;
 class NeighborSearchSignaller;
 class LastStepSignaller;
 class LoggingSignaller;
+class TrajectorySignaller;
 class EnergySignaller;
 
 //! \addtogroup module_modularsimulator
@@ -142,7 +143,7 @@ public:
     //! Function run before every step of scheduling
     virtual void signal(Step, Time) = 0;
     //! Method guaranteed to be called after construction, before simulator run
-    virtual void signallerSetup() = 0;
+    virtual void setup() = 0;
     //! Standard virtual destructor
     virtual ~ISignaller() = default;
 };
@@ -266,8 +267,8 @@ class ITrajectorySignallerClient
 public:
     //! @cond
     // (doxygen doesn't like these...)
-    //! Allow builder of TrajectoryElement to ask for callback registration
-    friend class TrajectoryElementBuilder;
+    //! Allow builder of TrajectorySignaller to ask for callback registration
+    friend class SignallerBuilder<TrajectorySignaller>;
     //! @endcond
     //! Standard virtual destructor
     virtual ~ITrajectorySignallerClient() = default;
index 8e1a1a0575e5752b9d038b6e036f85bf72bbd2e5..26d687ed6102ea2f36767fecfb3c8df553ba473d 100644 (file)
@@ -1,7 +1,7 @@
 /*
  * This file is part of the GROMACS molecular simulation package.
  *
- * Copyright (c) 2019, by the GROMACS development team, led by
+ * Copyright (c) 2019,2020, 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.
@@ -107,7 +107,7 @@ void LastStepSignaller::signal(Step step, Time time)
     }
 }
 
-void LastStepSignaller::signallerSetup()
+void LastStepSignaller::setup()
 {
     GMX_ASSERT(nsStepRegistrationDone_,
                "LastStepSignaller needs to be registered to NeighborSearchSignaller.");
@@ -141,7 +141,7 @@ void LoggingSignaller::signal(Step step, Time time)
     }
 }
 
-void LoggingSignaller::signallerSetup()
+void LoggingSignaller::setup()
 {
     GMX_ASSERT(lastStepRegistrationDone_,
                "LoggingSignaller needs to be registered to LastStepSignaller.");
@@ -154,6 +154,68 @@ SignallerCallbackPtr LoggingSignaller::registerLastStepCallback()
             [this](Step step, Time gmx_unused time) { this->lastStep_ = step; });
 }
 
+TrajectorySignaller::TrajectorySignaller(std::vector<SignallerCallbackPtr> signalEnergyCallbacks,
+                                         std::vector<SignallerCallbackPtr> signalStateCallbacks,
+                                         int                               nstxout,
+                                         int                               nstvout,
+                                         int                               nstfout,
+                                         int                               nstxoutCompressed,
+                                         int                               tngBoxOut,
+                                         int                               tngLambdaOut,
+                                         int                               tngBoxOutCompressed,
+                                         int                               tngLambdaOutCompressed,
+                                         int                               nstenergy) :
+    nstxout_(nstxout),
+    nstvout_(nstvout),
+    nstfout_(nstfout),
+    nstxoutCompressed_(nstxoutCompressed),
+    tngBoxOut_(tngBoxOut),
+    tngLambdaOut_(tngLambdaOut),
+    tngBoxOutCompressed_(tngBoxOutCompressed),
+    tngLambdaOutCompressed_(tngLambdaOutCompressed),
+    nstenergy_(nstenergy),
+    signalEnergyCallbacks_(std::move(signalEnergyCallbacks)),
+    signalStateCallbacks_(std::move(signalStateCallbacks)),
+    lastStep_(-1),
+    lastStepRegistrationDone_(false)
+{
+}
+
+void TrajectorySignaller::setup()
+{
+    GMX_ASSERT(lastStepRegistrationDone_,
+               "TrajectoryElement needs to be registered to LastStepSignaller.");
+}
+
+void TrajectorySignaller::signal(Step step, Time time)
+{
+    if (do_per_step(step, nstxout_) || do_per_step(step, nstvout_) || do_per_step(step, nstfout_)
+        || do_per_step(step, nstxoutCompressed_) || do_per_step(step, tngBoxOut_)
+        || do_per_step(step, tngLambdaOut_) || do_per_step(step, tngBoxOutCompressed_)
+        || do_per_step(step, tngLambdaOutCompressed_))
+    {
+        for (const auto& callback : signalStateCallbacks_)
+        {
+            (*callback)(step, time);
+        }
+    }
+
+    if (do_per_step(step, nstenergy_) || step == lastStep_)
+    {
+        for (const auto& callback : signalEnergyCallbacks_)
+        {
+            (*callback)(step, time);
+        }
+    }
+}
+
+SignallerCallbackPtr TrajectorySignaller::registerLastStepCallback()
+{
+    lastStepRegistrationDone_ = true;
+    return std::make_unique<SignallerCallback>(
+            [this](Step step, Time gmx_unused time) { this->lastStep_ = step; });
+}
+
 EnergySignaller::EnergySignaller(std::vector<SignallerCallbackPtr> calculateEnergyCallbacks,
                                  std::vector<SignallerCallbackPtr> calculateVirialCallbacks,
                                  std::vector<SignallerCallbackPtr> calculateFreeEnergyCallbacks,
@@ -194,7 +256,7 @@ void EnergySignaller::signal(Step step, Time time)
     }
 }
 
-void EnergySignaller::signallerSetup()
+void EnergySignaller::setup()
 {
     GMX_ASSERT(loggingRegistrationDone_,
                "EnergySignaller needs to be registered to LoggingSignaller.");
index 4fd9fd549f5303e80a3956eab61f47aeb6c56ca0..d3fe7db2f15a6722f0f88e2059e02281d8dcb4c4 100644 (file)
@@ -1,7 +1,7 @@
 /*
  * This file is part of the GROMACS molecular simulation package.
  *
- * Copyright (c) 2019, by the GROMACS development team, led by
+ * Copyright (c) 2019,2020, 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.
@@ -110,7 +110,7 @@ public:
     void signal(Step step, Time time) override;
 
     //! Do nothing at setup time
-    void signallerSetup() override{};
+    void setup() override{};
 
     //! Allow builder to construct
     friend class SignallerBuilder<NeighborSearchSignaller>;
@@ -158,7 +158,7 @@ public:
     void signal(Step step, Time time) override;
 
     //! Check that necessary registration was done
-    void signallerSetup() override;
+    void setup() override;
 
     //! Allow builder to construct
     friend class SignallerBuilder<LastStepSignaller>;
@@ -216,7 +216,7 @@ public:
     void signal(Step step, Time time) override;
 
     //! Check that necessary registration was done
-    void signallerSetup() override;
+    void setup() override;
 
     //! Allow builder to construct
     friend class SignallerBuilder<LoggingSignaller>;
@@ -251,6 +251,82 @@ private:
     bool lastStepRegistrationDone_;
 };
 
+/*! \libinternal
+ * \ingroup module_modularsimulator
+ * \brief Element signalling trajectory writing
+ *
+ * During signalling phase, it checks whether the current step is a writing
+ * step for either the energy or the state (position, velocity, forces)
+ * trajectory. It then notifies the signaller clients of the upcoming step.
+ *
+ * The TrajectorySignaller works in close collaboration with the TrajectoryElement
+ * which does the actual trajectory writing during the simulation step.
+ */
+class TrajectorySignaller final : public ISignaller, public ILastStepSignallerClient
+{
+public:
+    /*! \brief Prepare signaller
+     *
+     * Check that necessary registration was done
+     */
+    void setup() override;
+
+    /*! \brief Run the signaller at a specific step / time
+     *
+     * Informs clients when energy or state will be written.
+     *
+     * @param step           The current time step
+     * @param time           The current time
+     */
+    void signal(Step step, Time time) override;
+
+    //! Allow builder to construct
+    friend class SignallerBuilder<TrajectorySignaller>;
+    //! Define client type
+    typedef ITrajectorySignallerClient Client;
+
+private:
+    //! Constructor
+    TrajectorySignaller(std::vector<SignallerCallbackPtr> signalEnergyCallbacks,
+                        std::vector<SignallerCallbackPtr> signalStateCallbacks,
+                        int                               nstxout,
+                        int                               nstvout,
+                        int                               nstfout,
+                        int                               nstxoutCompressed,
+                        int                               tngBoxOut,
+                        int                               tngLambdaOut,
+                        int                               tngBoxOutCompressed,
+                        int                               tngLambdaOutCompressed,
+                        int                               nstenergy);
+
+    //! Output frequencies
+    //! {
+    const int nstxout_;
+    const int nstvout_;
+    const int nstfout_;
+    const int nstxoutCompressed_;
+    const int tngBoxOut_;
+    const int tngLambdaOut_;
+    const int tngBoxOutCompressed_;
+    const int tngLambdaOutCompressed_;
+    const int nstenergy_;
+    //! }
+
+    //! Callbacks to signal events
+    //! {
+    std::vector<SignallerCallbackPtr> signalEnergyCallbacks_;
+    std::vector<SignallerCallbackPtr> signalStateCallbacks_;
+    //! }
+
+    /*
+     * Last step client
+     */
+    Step lastStep_;
+    bool lastStepRegistrationDone_;
+    //! ILastStepSignallerClient implementation
+    SignallerCallbackPtr registerLastStepCallback() override;
+};
+
 /*! \libinternal
  * \ingroup module_modularsimulator
  * \brief Element signalling energy related special steps
@@ -274,7 +350,7 @@ public:
     void signal(Step step, Time time) override;
 
     //! Check that necessary registration was done
-    void signallerSetup() override;
+    void setup() override;
 
     //! Allow builder to construct
     friend class SignallerBuilder<EnergySignaller>;
@@ -347,6 +423,21 @@ std::unique_ptr<Signaller> SignallerBuilder<Signaller>::build(Args&&... args)
     return std::unique_ptr<Signaller>(new Signaller(std::move(callbacks), std::forward<Args>(args)...));
 }
 
+/*! \brief Build the signaller
+ *
+ * Specialized version - TrajectorySignaller has a different build process
+ */
+template<>
+template<typename... Args>
+std::unique_ptr<TrajectorySignaller> SignallerBuilder<TrajectorySignaller>::build(Args&&... args)
+{
+    auto signalEnergyCallbacks = buildCallbackVector(TrajectoryEvent::EnergyWritingStep);
+    auto signalStateCallbacks  = buildCallbackVector(TrajectoryEvent::StateWritingStep);
+    // NOLINTNEXTLINE(modernize-make-unique): make_unique does not work with private constructor
+    return std::unique_ptr<TrajectorySignaller>(new TrajectorySignaller(
+            std::move(signalEnergyCallbacks), std::move(signalStateCallbacks), std::forward<Args>(args)...));
+}
+
 /*! \brief Build the signaller
  *
  * Specialized version - EnergySignaller has a significantly different build process
@@ -412,6 +503,16 @@ SignallerBuilder<LoggingSignaller>::getSignallerCallback(typename LoggingSignall
     return client->registerLoggingCallback(std::forward<Args>(args)...);
 }
 
+//! Get a callback from a single client - TrajectorySignaller
+template<>
+template<typename... Args>
+SignallerCallbackPtr
+SignallerBuilder<TrajectorySignaller>::getSignallerCallback(typename TrajectorySignaller::Client* client,
+                                                            Args&&... args)
+{
+    return client->registerTrajectorySignallerCallback(std::forward<Args>(args)...);
+}
+
 //! Get a callback from a single client - EnergySignaller
 template<>
 template<typename... Args>
index 1da5d8ccc8a0902203da0129a1667aa323967fc0..f71b2ca62607e092187cc8a0eca78f115e41c3c9 100644 (file)
@@ -49,9 +49,7 @@
 
 namespace gmx
 {
-TrajectoryElement::TrajectoryElement(std::vector<SignallerCallbackPtr>     signalEnergyCallbacks,
-                                     std::vector<SignallerCallbackPtr>     signalStateCallbacks,
-                                     std::vector<ITrajectoryWriterClient*> writerClients,
+TrajectoryElement::TrajectoryElement(std::vector<ITrajectoryWriterClient*> writerClients,
                                      FILE*                                 fplog,
                                      int                                   nfile,
                                      const t_filenm                        fnm[],
@@ -81,51 +79,25 @@ TrajectoryElement::TrajectoryElement(std::vector<SignallerCallbackPtr>     signa
                       startingBehavior,
                       simulationsShareState,
                       nullptr)),
-    nstxout_(inputrec->nstxout),
-    nstvout_(inputrec->nstvout),
-    nstfout_(inputrec->nstfout),
-    nstxoutCompressed_(inputrec->nstxout_compressed),
-    tngBoxOut_(mdoutf_get_tng_box_output_interval(outf_)),
-    tngLambdaOut_(mdoutf_get_tng_lambda_output_interval(outf_)),
-    tngBoxOutCompressed_(mdoutf_get_tng_compressed_box_output_interval(outf_)),
-    tngLambdaOutCompressed_(mdoutf_get_tng_compressed_lambda_output_interval(outf_)),
-    nstenergy_(inputrec->nstenergy),
-    signalEnergyCallbacks_(std::move(signalEnergyCallbacks)),
-    signalStateCallbacks_(std::move(signalStateCallbacks)),
-    lastStep_(-1),
-    lastStepRegistrationDone_(false),
     writerClients_(std::move(writerClients))
 {
 }
 
-void TrajectoryElement::signallerSetup()
+int TrajectoryElement::tngBoxOut() const
 {
-    GMX_ASSERT(lastStepRegistrationDone_,
-               "TrajectoryElement needs to be registered to LastStepSignaller.");
+    return mdoutf_get_tng_box_output_interval(outf_);
 }
-
-void TrajectoryElement::signal(Step step, Time time)
+int TrajectoryElement::tngLambdaOut() const
 {
-    if (do_per_step(step, nstxout_) || do_per_step(step, nstvout_) || do_per_step(step, nstfout_)
-        || do_per_step(step, nstxoutCompressed_) || do_per_step(step, tngBoxOut_)
-        || do_per_step(step, tngLambdaOut_) || do_per_step(step, tngBoxOutCompressed_)
-        || do_per_step(step, tngLambdaOutCompressed_))
-    {
-        writeStateStep_ = step;
-        for (const auto& callback : signalStateCallbacks_)
-        {
-            (*callback)(step, time);
-        }
-    }
-
-    if (do_per_step(step, nstenergy_) || step == lastStep_)
-    {
-        writeEnergyStep_ = step;
-        for (const auto& callback : signalEnergyCallbacks_)
-        {
-            (*callback)(step, time);
-        }
-    }
+    return mdoutf_get_tng_lambda_output_interval(outf_);
+}
+int TrajectoryElement::tngBoxOutCompressed() const
+{
+    return mdoutf_get_tng_compressed_box_output_interval(outf_);
+}
+int TrajectoryElement::tngLambdaOutCompressed() const
+{
+    return mdoutf_get_tng_compressed_lambda_output_interval(outf_);
 }
 
 void TrajectoryElement::elementSetup()
@@ -188,22 +160,25 @@ void TrajectoryElement::write(Step step, Time time, bool writeState, bool writeE
     }
 }
 
-SignallerCallbackPtr TrajectoryElement::registerLastStepCallback()
-{
-    lastStepRegistrationDone_ = true;
-    return std::make_unique<SignallerCallback>(
-            [this](Step step, Time gmx_unused time) { this->lastStep_ = step; });
-}
-
 SignallerCallbackPtr TrajectoryElement::registerLoggingCallback()
 {
     return std::make_unique<SignallerCallback>(
             [this](Step step, Time /*unused*/) { logWritingStep_ = step; });
 }
 
-void TrajectoryElementBuilder::registerSignallerClient(compat::not_null<ITrajectorySignallerClient*> client)
+SignallerCallbackPtr TrajectoryElement::registerTrajectorySignallerCallback(TrajectoryEvent event)
 {
-    signallerClients_.emplace_back(client);
+    if (event == TrajectoryEvent::StateWritingStep)
+    {
+        return std::make_unique<SignallerCallback>(
+                [this](Step step, Time /*unused*/) { this->writeStateStep_ = step; });
+    }
+    if (event == TrajectoryEvent::EnergyWritingStep)
+    {
+        return std::make_unique<SignallerCallback>(
+                [this](Step step, Time /*unused*/) { this->writeEnergyStep_ = step; });
+    }
+    return nullptr;
 }
 
 void TrajectoryElementBuilder::registerWriterClient(compat::not_null<ITrajectoryWriterClient*> client)
index be1a7bd7894df9f6d656296cf35b174edd9a0c57..afbe2811c1dea5246cfa761f1f9c9551f4f42cb8 100644 (file)
@@ -77,33 +77,18 @@ enum class StartingBehavior;
  * element only prepares the output struct, and passes it to the clients who
  * write their part of the trajectory.
  */
-class TrajectoryElement final :
-    public ISimulatorElement,
-    public ISignaller,
-    public ILastStepSignallerClient,
-    public ILoggingSignallerClient
+class TrajectoryElement final : public ISimulatorElement, public ILoggingSignallerClient, public ITrajectorySignallerClient
 {
 public:
     friend class TrajectoryElementBuilder;
-
-    /*
-     * Methods for the signaller part of the element
-     */
-
-    /*! \brief Prepare signaller
-     *
-     * Check that necessary registration was done
-     */
-    void signallerSetup() override;
-
-    /*! \brief Run the signaller at a specific step / time
-     *
-     * Informs clients when energy or state will be written.
-     *
-     * @param step           The current time step
-     * @param time           The current time
-     */
-    void signal(Step step, Time time) override;
+    //! Get the box writeout frequency for TNG
+    [[nodiscard]] int tngBoxOut() const;
+    //! Get the lambda writeout frequency for TNG
+    [[nodiscard]] int tngLambdaOut() const;
+    //! Get the compressed box writeout frequency for TNG
+    [[nodiscard]] int tngBoxOutCompressed() const;
+    //! Get the compressed lambda writeout frequency for TNG
+    [[nodiscard]] int tngLambdaOutCompressed() const;
 
     /*
      * Methods for the trajectory writing part of the element
@@ -145,9 +130,7 @@ public:
 
 private:
     //! Constructor
-    TrajectoryElement(std::vector<SignallerCallbackPtr>     signalEnergyCallbacks,
-                      std::vector<SignallerCallbackPtr>     signalStateCallbacks,
-                      std::vector<ITrajectoryWriterClient*> writerClients,
+    TrajectoryElement(std::vector<ITrajectoryWriterClient*> writerClients,
                       FILE*                                 fplog,
                       int                                   nfile,
                       const t_filenm                        fnm[],
@@ -174,36 +157,8 @@ private:
 
     //! ILoggingSignallerClient implementation
     SignallerCallbackPtr registerLoggingCallback() override;
-
-    /*
-     * Signaller
-     */
-    //! Output frequencies
-    //! {
-    const int nstxout_;
-    const int nstvout_;
-    const int nstfout_;
-    const int nstxoutCompressed_;
-    const int tngBoxOut_;
-    const int tngLambdaOut_;
-    const int tngBoxOutCompressed_;
-    const int tngLambdaOutCompressed_;
-    const int nstenergy_;
-    //! }
-
-    //! Callbacks to signal events
-    //! {
-    std::vector<SignallerCallbackPtr> signalEnergyCallbacks_;
-    std::vector<SignallerCallbackPtr> signalStateCallbacks_;
-    //! }
-
-    /*
-     * Last step client
-     */
-    Step lastStep_;
-    bool lastStepRegistrationDone_;
-    //! ILastStepSignallerClient implementation
-    SignallerCallbackPtr registerLastStepCallback() override;
+    //! ITrajectorySignallerClient implementation
+    SignallerCallbackPtr registerTrajectorySignallerCallback(TrajectoryEvent event) override;
 
     /*
      * Trajectory writing
@@ -225,16 +180,12 @@ private:
  * \ingroup module_modularsimulator
  * \brief Build the `TrajectoryElement`
  *
- * This builder allows clients to register with the trajectory element, either
- * as signaller clients or as writer clients. The builder then builds the
- * element.
+ * This builder allows clients to register with the trajectory element
+ * as writer clients. The builder then builds the trajectory element.
  */
 class TrajectoryElementBuilder final
 {
 public:
-    //! Allows clients to register to the signaller
-    void registerSignallerClient(compat::not_null<ITrajectorySignallerClient*> client);
-
     //! Allows clients to register as trajectory writers
     void registerWriterClient(compat::not_null<ITrajectoryWriterClient*> client);
 
@@ -243,8 +194,6 @@ public:
     std::unique_ptr<TrajectoryElement> build(Args&&... args);
 
 private:
-    //! List of signaller clients
-    std::vector<ITrajectorySignallerClient*> signallerClients_;
     //! List of writer clients
     std::vector<ITrajectoryWriterClient*> writerClients_;
 };
@@ -252,27 +201,9 @@ private:
 template<typename... Args>
 std::unique_ptr<TrajectoryElement> TrajectoryElementBuilder::build(Args&&... args)
 {
-    std::vector<SignallerCallbackPtr> signalEnergyCallbacks;
-    std::vector<SignallerCallbackPtr> signalStateCallbacks;
-    // Allow clients to register their callbacks
-    for (auto& client : signallerClients_)
-    {
-        // don't register nullptr
-        if (auto energyCallback =
-                    client->registerTrajectorySignallerCallback(TrajectoryEvent::EnergyWritingStep))
-        {
-            signalEnergyCallbacks.emplace_back(std::move(energyCallback));
-        }
-        if (auto stateCallback =
-                    client->registerTrajectorySignallerCallback(TrajectoryEvent::StateWritingStep))
-        {
-            signalStateCallbacks.emplace_back(std::move(stateCallback));
-        }
-    }
     // NOLINTNEXTLINE(modernize-make-unique): make_unique does not work with private constructor
     return std::unique_ptr<TrajectoryElement>(
-            new TrajectoryElement(std::move(signalEnergyCallbacks), std::move(signalStateCallbacks),
-                                  std::move(writerClients_), std::forward<Args>(args)...));
+            new TrajectoryElement(std::move(writerClients_), std::forward<Args>(args)...));
 }
 
 } // namespace gmx