From 5c8a022942806438c736ec7b97581304e6ef0e15 Mon Sep 17 00:00:00 2001 From: Christian Blau Date: Tue, 21 Jan 2020 11:05:03 +0100 Subject: [PATCH] Add function calls to MdModules to sign up for notifications Allow MdModules to sign up to notifications after they know their own options. This makes it possible that modules only get called back when they need to and not by default and skips the if(active) logic in functions that are called back in favour of not subscribing to the notifications to begin with. refs #3076 Change-Id: Ib95c1f734f919805c661ac6f6451af5da8187eec --- src/gromacs/applied_forces/densityfitting.cpp | 160 +++++++++--------- src/gromacs/applied_forces/densityfitting.h | 2 +- src/gromacs/applied_forces/electricfield.cpp | 5 +- .../applied_forces/tests/densityfitting.cpp | 5 +- src/gromacs/gmxpreprocess/grompp.cpp | 5 + src/gromacs/imd/imd.cpp | 2 + src/gromacs/mdrun/mdmodules.cpp | 14 +- src/gromacs/mdrun/mdmodules.h | 18 +- src/gromacs/mdrun/runner.cpp | 2 + src/gromacs/mdtypes/imdmodule.h | 7 +- src/gromacs/restraint/restraintmdmodule.cpp | 4 + src/gromacs/restraint/restraintmdmodule.h | 8 +- src/gromacs/swap/swapcoords.cpp | 2 + 13 files changed, 147 insertions(+), 87 deletions(-) diff --git a/src/gromacs/applied_forces/densityfitting.cpp b/src/gromacs/applied_forces/densityfitting.cpp index 5a134e4327..2889e61a50 100644 --- a/src/gromacs/applied_forces/densityfitting.cpp +++ b/src/gromacs/applied_forces/densityfitting.cpp @@ -223,8 +223,12 @@ private: */ class DensityFitting final : public IMDModule { + public: - /*! \brief Construct the density fitting module. + //! Construct the density fitting module. + explicit DensityFitting() = default; + + /*! \brief Request to be notified during pre-processing. * * \param[in] notifier allows the module to subscribe to notifications from MdModules. * @@ -234,25 +238,18 @@ public: * - storing its internal parameters in a tpr file by writing to a * key-value-tree during pre-processing by a function taking a * KeyValueTreeObjectBuilder as parameter - * - reading its internal parameters from a key-value-tree during - * simulation setup by taking a const KeyValueTreeObject & parameter - * - constructing local atom sets in the simulation parameter setup - * by taking a LocalAtomSetManager * as parameter - * - the type of periodic boundary conditions that are used - * by taking a PeriodicBoundaryConditionType as parameter - * - the writing of checkpoint data - * by taking a MdModulesWriteCheckpointData as parameter - * - the reading of checkpoint data - * by taking a MdModulesCheckpointReadingDataOnMaster as parameter - * - the broadcasting of checkpoint data - * by taking MdModulesCheckpointReadingBroadcast as parameter */ - explicit DensityFitting(MdModulesNotifier* notifier) + void subscribeToPreProcessingNotifications(MdModulesNotifier* notifier) override { // Callbacks for several kinds of MdModuleNotification are created // and subscribed, and will be dispatched correctly at run time // based on the type of the parameter required by the lambda. + if (!densityFittingOptions_.active()) + { + return; + } + // Setting the atom group indices from index group string const auto setFitGroupIndicesFunction = [this](const IndexGroupsAndNames& indexGroupsAndNames) { densityFittingOptions_.setFitGroupIndices(indexGroupsAndNames); @@ -265,18 +262,41 @@ public: }; notifier->preProcessingNotifications_.subscribe(writeInternalParametersFunction); - // Reading internal parameters during simulation setup - const auto readInternalParametersFunction = [this](const KeyValueTreeObject& tree) { - densityFittingOptions_.readInternalParametersFromKvt(tree); - }; - notifier->simulationSetupNotifications_.subscribe(readInternalParametersFunction); - // Checking for consistency with all .mdp options const auto checkEnergyCaluclationFrequencyFunction = [this](EnergyCalculationFrequencyErrors* energyCalculationFrequencyErrors) { densityFittingOptions_.checkEnergyCaluclationFrequency(energyCalculationFrequencyErrors); }; notifier->preProcessingNotifications_.subscribe(checkEnergyCaluclationFrequencyFunction); + } + + /*! \brief Request to be notified. + * The density fitting code subscribes to these notifications: + * - reading its internal parameters from a key-value-tree during + * simulation setup by taking a const KeyValueTreeObject & parameter + * - constructing local atom sets in the simulation parameter setup + * by taking a LocalAtomSetManager * as parameter + * - the type of periodic boundary conditions that are used + * by taking a PeriodicBoundaryConditionType as parameter + * - the writing of checkpoint data + * by taking a MdModulesWriteCheckpointData as parameter + * - the reading of checkpoint data + * by taking a MdModulesCheckpointReadingDataOnMaster as parameter + * - the broadcasting of checkpoint data + * by taking MdModulesCheckpointReadingBroadcast as parameter + */ + void subscribeToSimulationSetupNotifications(MdModulesNotifier* notifier) override + { + if (!densityFittingOptions_.active()) + { + return; + } + + // Reading internal parameters during simulation setup + const auto readInternalParametersFunction = [this](const KeyValueTreeObject& tree) { + densityFittingOptions_.readInternalParametersFromKvt(tree); + }; + notifier->simulationSetupNotifications_.subscribe(readInternalParametersFunction); // constructing local atom sets during simulation setup const auto setLocalAtomSetFunction = [this](LocalAtomSetManager* localAtomSetManager) { @@ -359,12 +379,8 @@ public: */ void constructLocalAtomSet(LocalAtomSetManager* localAtomSetManager) { - if (densityFittingOptions_.active()) - { - LocalAtomSet atomSet = - localAtomSetManager->add(densityFittingOptions_.buildParameters().indices_); - densityFittingSimulationParameters_.setLocalAtomSet(atomSet); - } + LocalAtomSet atomSet = localAtomSetManager->add(densityFittingOptions_.buildParameters().indices_); + densityFittingSimulationParameters_.setLocalAtomSet(atomSet); } /*! \brief Request energy output to energy file during simulation. @@ -384,21 +400,17 @@ public: */ void writeCheckpointData(MdModulesWriteCheckpointData checkpointWriting) { - if (densityFittingOptions_.active()) - { - const DensityFittingForceProviderState& state = forceProvider_->stateToCheckpoint(); - checkpointWriting.builder_.addValue( - DensityFittingModuleInfo::name_ + "-stepsSinceLastCalculation", - state.stepsSinceLastCalculation_); - checkpointWriting.builder_.addValue( - DensityFittingModuleInfo::name_ + "-adaptiveForceConstantScale", - state.adaptiveForceConstantScale_); - KeyValueTreeObjectBuilder exponentialMovingAverageKvtEntry = - checkpointWriting.builder_.addObject(DensityFittingModuleInfo::name_ - + "-exponentialMovingAverageState"); - exponentialMovingAverageStateAsKeyValueTree(exponentialMovingAverageKvtEntry, - state.exponentialMovingAverageState_); - } + const DensityFittingForceProviderState& state = forceProvider_->stateToCheckpoint(); + checkpointWriting.builder_.addValue( + DensityFittingModuleInfo::name_ + "-stepsSinceLastCalculation", + state.stepsSinceLastCalculation_); + checkpointWriting.builder_.addValue( + DensityFittingModuleInfo::name_ + "-adaptiveForceConstantScale", + state.adaptiveForceConstantScale_); + KeyValueTreeObjectBuilder exponentialMovingAverageKvtEntry = checkpointWriting.builder_.addObject( + DensityFittingModuleInfo::name_ + "-exponentialMovingAverageState"); + exponentialMovingAverageStateAsKeyValueTree(exponentialMovingAverageKvtEntry, + state.exponentialMovingAverageState_); } /*! \brief Read the internal parameters from the checkpoint file on master @@ -406,34 +418,31 @@ public: */ void readCheckpointDataOnMaster(MdModulesCheckpointReadingDataOnMaster checkpointReading) { - if (densityFittingOptions_.active()) + if (checkpointReading.checkpointedData_.keyExists(DensityFittingModuleInfo::name_ + + "-stepsSinceLastCalculation")) { - if (checkpointReading.checkpointedData_.keyExists(DensityFittingModuleInfo::name_ - + "-stepsSinceLastCalculation")) - { - densityFittingState_.stepsSinceLastCalculation_ = - checkpointReading - .checkpointedData_[DensityFittingModuleInfo::name_ - + "-stepsSinceLastCalculation"] - .cast(); - } - if (checkpointReading.checkpointedData_.keyExists(DensityFittingModuleInfo::name_ - + "-adaptiveForceConstantScale")) - { - densityFittingState_.adaptiveForceConstantScale_ = - checkpointReading - .checkpointedData_[DensityFittingModuleInfo::name_ - + "-adaptiveForceConstantScale"] - .cast(); - } - if (checkpointReading.checkpointedData_.keyExists(DensityFittingModuleInfo::name_ - + "-exponentialMovingAverageState")) - { - densityFittingState_.exponentialMovingAverageState_ = exponentialMovingAverageStateFromKeyValueTree( - checkpointReading - .checkpointedData_[DensityFittingModuleInfo::name_ + "-exponentialMovingAverageState"] - .asObject()); - } + densityFittingState_.stepsSinceLastCalculation_ = + checkpointReading + .checkpointedData_[DensityFittingModuleInfo::name_ + + "-stepsSinceLastCalculation"] + .cast(); + } + if (checkpointReading.checkpointedData_.keyExists(DensityFittingModuleInfo::name_ + + "-adaptiveForceConstantScale")) + { + densityFittingState_.adaptiveForceConstantScale_ = + checkpointReading + .checkpointedData_[DensityFittingModuleInfo::name_ + + "-adaptiveForceConstantScale"] + .cast(); + } + if (checkpointReading.checkpointedData_.keyExists(DensityFittingModuleInfo::name_ + + "-exponentialMovingAverageState")) + { + densityFittingState_.exponentialMovingAverageState_ = exponentialMovingAverageStateFromKeyValueTree( + checkpointReading + .checkpointedData_[DensityFittingModuleInfo::name_ + "-exponentialMovingAverageState"] + .asObject()); } } @@ -443,14 +452,11 @@ public: */ void broadcastCheckpointData(MdModulesCheckpointReadingBroadcast checkpointBroadcast) { - if (densityFittingOptions_.active()) + if (PAR(&(checkpointBroadcast.cr_))) { - if (PAR(&(checkpointBroadcast.cr_))) - { - block_bc(&(checkpointBroadcast.cr_), densityFittingState_.stepsSinceLastCalculation_); - block_bc(&(checkpointBroadcast.cr_), densityFittingState_.adaptiveForceConstantScale_); - block_bc(&(checkpointBroadcast.cr_), densityFittingState_.exponentialMovingAverageState_); - } + block_bc(&(checkpointBroadcast.cr_), densityFittingState_.stepsSinceLastCalculation_); + block_bc(&(checkpointBroadcast.cr_), densityFittingState_.adaptiveForceConstantScale_); + block_bc(&(checkpointBroadcast.cr_), densityFittingState_.exponentialMovingAverageState_); } } @@ -473,9 +479,9 @@ private: } // namespace -std::unique_ptr DensityFittingModuleInfo::create(MdModulesNotifier* notifier) +std::unique_ptr DensityFittingModuleInfo::create() { - return std::make_unique(notifier); + return std::make_unique(); } const std::string DensityFittingModuleInfo::name_ = "density-guided-simulation"; diff --git a/src/gromacs/applied_forces/densityfitting.h b/src/gromacs/applied_forces/densityfitting.h index 389e4144c5..6e29a7faa3 100644 --- a/src/gromacs/applied_forces/densityfitting.h +++ b/src/gromacs/applied_forces/densityfitting.h @@ -58,7 +58,7 @@ struct DensityFittingModuleInfo * Fitting an all-atom structure into an experimental cryo-EM density map is a * typical application. */ - static std::unique_ptr create(MdModulesNotifier* notifier); + static std::unique_ptr create(); //! The name of the module static const std::string name_; }; diff --git a/src/gromacs/applied_forces/electricfield.cpp b/src/gromacs/applied_forces/electricfield.cpp index 5bc278142d..08c414f5d4 100644 --- a/src/gromacs/applied_forces/electricfield.cpp +++ b/src/gromacs/applied_forces/electricfield.cpp @@ -1,7 +1,7 @@ /* * This file is part of the GROMACS molecular simulation package. * - * Copyright (c) 2015,2016,2017,2018,2019, by the GROMACS development team, led by + * Copyright (c) 2015,2016,2017,2018,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. @@ -171,6 +171,9 @@ public: void calculateForces(const ForceProviderInput& forceProviderInput, ForceProviderOutput* forceProviderOutput) override; + void subscribeToSimulationSetupNotifications(MdModulesNotifier* /* notifier */) override {} + void subscribeToPreProcessingNotifications(MdModulesNotifier* /* notifier */) override {} + private: //! Return whether or not to apply a field bool isActive() const; diff --git a/src/gromacs/applied_forces/tests/densityfitting.cpp b/src/gromacs/applied_forces/tests/densityfitting.cpp index 38f3cb0d4c..bc5ae180d9 100644 --- a/src/gromacs/applied_forces/tests/densityfitting.cpp +++ b/src/gromacs/applied_forces/tests/densityfitting.cpp @@ -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. @@ -94,7 +94,7 @@ public: { KeyValueTreeObject mdpOptionsTree = mdpValueBuilder_.build(); - densityFittingModule_ = DensityFittingModuleInfo::create(¬ifier_); + densityFittingModule_ = DensityFittingModuleInfo::create(); // set up options Options densityFittingModuleOptions; @@ -117,7 +117,6 @@ public: protected: KeyValueTreeBuilder mdpValueBuilder_; - MdModulesNotifier notifier_; ForceProviders densityFittingForces_; std::unique_ptr densityFittingModule_; }; diff --git a/src/gromacs/gmxpreprocess/grompp.cpp b/src/gromacs/gmxpreprocess/grompp.cpp index 66d9dbaea8..f95384d1f8 100644 --- a/src/gromacs/gmxpreprocess/grompp.cpp +++ b/src/gromacs/gmxpreprocess/grompp.cpp @@ -1856,6 +1856,11 @@ int gmx_grompp(int argc, char* argv[]) } GMX_CATCH_ALL_AND_EXIT_WITH_FATAL_ERROR + // Now that the MdModules have their options assigned from get_ir, subscribe + // to eventual notifications during pre-processing their data + mdModules.subscribeToPreProcessingNotifications(); + + if (bVerbose) { GMX_LOG(logger.info) diff --git a/src/gromacs/imd/imd.cpp b/src/gromacs/imd/imd.cpp index 6126e65fee..4f54a989d3 100644 --- a/src/gromacs/imd/imd.cpp +++ b/src/gromacs/imd/imd.cpp @@ -325,6 +325,8 @@ class InteractiveMolecularDynamics final : public IMDModule IMdpOptionProvider* mdpOptionProvider() override { return nullptr; } IMDOutputProvider* outputProvider() override { return nullptr; } void initForceProviders(ForceProviders* /* forceProviders */) override {} + void subscribeToSimulationSetupNotifications(MdModulesNotifier* /* notifier */) override {} + void subscribeToPreProcessingNotifications(MdModulesNotifier* /* notifier */) override {} }; std::unique_ptr createInteractiveMolecularDynamicsModule() diff --git a/src/gromacs/mdrun/mdmodules.cpp b/src/gromacs/mdrun/mdmodules.cpp index 19dfd2869a..a8ff0a0352 100644 --- a/src/gromacs/mdrun/mdmodules.cpp +++ b/src/gromacs/mdrun/mdmodules.cpp @@ -1,7 +1,7 @@ /* * This file is part of the GROMACS molecular simulation package. * - * Copyright (c) 2016,2017,2018,2019, by the GROMACS development team, led by + * Copyright (c) 2016,2017,2018,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. @@ -63,7 +63,7 @@ class MDModules::Impl : public IMDOutputProvider { public: Impl() : - densityFitting_(DensityFittingModuleInfo::create(¬ifier_)), + densityFitting_(DensityFittingModuleInfo::create()), field_(createElectricFieldModule()), imd_(createInteractiveMolecularDynamicsModule()), swapCoordinates_(createSwapCoordinatesModule()) @@ -174,6 +174,16 @@ ForceProviders* MDModules::initForceProviders() return impl_->forceProviders_.get(); } +void MDModules::subscribeToPreProcessingNotifications() +{ + impl_->densityFitting_->subscribeToPreProcessingNotifications(&impl_->notifier_); +} + +void MDModules::subscribeToSimulationSetupNotifications() +{ + impl_->densityFitting_->subscribeToSimulationSetupNotifications(&impl_->notifier_); +} + void MDModules::add(std::shared_ptr module) { impl_->modules_.emplace_back(std::move(module)); diff --git a/src/gromacs/mdrun/mdmodules.h b/src/gromacs/mdrun/mdmodules.h index 3f956431e5..6fabb761cd 100644 --- a/src/gromacs/mdrun/mdmodules.h +++ b/src/gromacs/mdrun/mdmodules.h @@ -1,7 +1,7 @@ /* * This file is part of the GROMACS molecular simulation package. * - * Copyright (c) 2016,2017,2018,2019, by the GROMACS development team, led by + * Copyright (c) 2016,2017,2018,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. @@ -144,6 +144,22 @@ public: */ ForceProviders* initForceProviders(); + /*! \brief Subscribe MdModules to simulation setup notifications. + * + * Allows MdModules to subscribe to notifications that are called back + * during the set up of an MD simulation, after the options were + * assigned to the modules. + */ + void subscribeToSimulationSetupNotifications(); + + /*! \brief Subscribe MdModules to notifications during pre-processing. + * + * Allows MdModules to subscribe to notifications that are called back + * during pre processing an MD simulation, after the options were + * assigned to the modules. + */ + void subscribeToPreProcessingNotifications(); + /*! * \brief Add a module to the container. * diff --git a/src/gromacs/mdrun/runner.cpp b/src/gromacs/mdrun/runner.cpp index 6827e7e9de..d15f2933d3 100644 --- a/src/gromacs/mdrun/runner.cpp +++ b/src/gromacs/mdrun/runner.cpp @@ -910,6 +910,8 @@ int Mdrunner::mdrunner() // TODO: Error handling mdModules_->assignOptionsToModules(*inputrec->params, nullptr); + // now that the MdModules know their options, they know which callbacks to sign up to + mdModules_->subscribeToSimulationSetupNotifications(); const auto& mdModulesNotifier = mdModules_->notifier().simulationSetupNotifications_; if (inputrec->internalParameters != nullptr) diff --git a/src/gromacs/mdtypes/imdmodule.h b/src/gromacs/mdtypes/imdmodule.h index c9b1f59402..83283417b4 100644 --- a/src/gromacs/mdtypes/imdmodule.h +++ b/src/gromacs/mdtypes/imdmodule.h @@ -1,7 +1,7 @@ /* * This file is part of the GROMACS molecular simulation package. * - * Copyright (c) 2017,2019, by the GROMACS development team, led by + * Copyright (c) 2017,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. @@ -51,6 +51,7 @@ namespace gmx class ForceProviders; class IMDOutputProvider; class IMdpOptionProvider; +struct MdModulesNotifier; /*! \libinternal \brief * Extension module for \Gromacs simulations. @@ -73,6 +74,10 @@ public: virtual IMDOutputProvider* outputProvider() = 0; //! Initializes force providers from this module. virtual void initForceProviders(ForceProviders* forceProviders) = 0; + //! Subscribe to simulation setup notifications + virtual void subscribeToSimulationSetupNotifications(MdModulesNotifier* notifier) = 0; + //! Subscribe to pre processing notifications + virtual void subscribeToPreProcessingNotifications(MdModulesNotifier* notifier) = 0; }; } // namespace gmx diff --git a/src/gromacs/restraint/restraintmdmodule.cpp b/src/gromacs/restraint/restraintmdmodule.cpp index a6ba84eeee..cb8091aac4 100644 --- a/src/gromacs/restraint/restraintmdmodule.cpp +++ b/src/gromacs/restraint/restraintmdmodule.cpp @@ -204,6 +204,10 @@ std::unique_ptr RestraintMDModule::create(std::shared_ptr restraint) : impl_{ std::move(restraint) } diff --git a/src/gromacs/restraint/restraintmdmodule.h b/src/gromacs/restraint/restraintmdmodule.h index c6ec4b5615..2a0c75784a 100644 --- a/src/gromacs/restraint/restraintmdmodule.h +++ b/src/gromacs/restraint/restraintmdmodule.h @@ -1,7 +1,7 @@ /* * This file is part of the GROMACS molecular simulation package. * - * Copyright (c) 2018,2019, by the GROMACS development team, led by + * Copyright (c) 2018,2019,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. @@ -53,6 +53,7 @@ namespace gmx // Forward declaration to allow opaque pointer to library internal class. class RestraintMDModuleImpl; +struct MdModulesNotifier; /*! \libinternal \ingroup module_restraint * \brief MDModule wrapper for Restraint implementations. @@ -115,6 +116,11 @@ public: */ void initForceProviders(ForceProviders* forceProviders) override; + //! Subscribe to simulation setup notifications + void subscribeToSimulationSetupNotifications(MdModulesNotifier* notifier) override; + //! Subscribe to pre processing notifications + void subscribeToPreProcessingNotifications(MdModulesNotifier* notifier) override; + private: /*! * \brief Private implementation opaque pointer. diff --git a/src/gromacs/swap/swapcoords.cpp b/src/gromacs/swap/swapcoords.cpp index a9290efe65..37df287b80 100644 --- a/src/gromacs/swap/swapcoords.cpp +++ b/src/gromacs/swap/swapcoords.cpp @@ -126,6 +126,8 @@ class SwapCoordinates final : public IMDModule IMdpOptionProvider* mdpOptionProvider() override { return nullptr; } IMDOutputProvider* outputProvider() override { return nullptr; } void initForceProviders(ForceProviders* /* forceProviders */) override {} + void subscribeToSimulationSetupNotifications(MdModulesNotifier* /* notifier */) override {} + void subscribeToPreProcessingNotifications(MdModulesNotifier* /* notifier */) override {} }; std::unique_ptr createSwapCoordinatesModule() -- 2.22.0