From 2ef41cdb2e15715a09464e50affa063fa4eb1544 Mon Sep 17 00:00:00 2001 From: ejjordan Date: Wed, 22 Sep 2021 18:52:37 +0200 Subject: [PATCH] Remove useless top level nblib force calculator The top level nblib force calculator was a place holder in anticipation of a unification of CPU and GPU force calculator backends, which did not occur. As such it is removed since it only adds maintenance burden and not utility. Also removed is a useless class holding the non-bonded force calculator setup director, which is now a free function. --- api/nblib/CMakeLists.txt | 3 +- api/nblib/forcecalculator.cpp | 80 ------------- api/nblib/forcecalculator.h | 113 ------------------ api/nblib/gmxsetup.cpp | 23 ++-- api/nblib/gmxsetup.h | 22 +--- api/nblib/nblib.h | 3 +- .../samples/argon-forces-integration.cpp | 5 +- .../samples/methane-water-integration.cpp | 5 +- api/nblib/tests/gmxcalculator.cpp | 5 +- api/nblib/tests/nbkernelsystem.cpp | 36 +++--- 10 files changed, 50 insertions(+), 245 deletions(-) delete mode 100644 api/nblib/forcecalculator.cpp delete mode 100644 api/nblib/forcecalculator.h diff --git a/api/nblib/CMakeLists.txt b/api/nblib/CMakeLists.txt index 2b1b0b70fb..31eca82e77 100644 --- a/api/nblib/CMakeLists.txt +++ b/api/nblib/CMakeLists.txt @@ -89,7 +89,6 @@ endif() target_sources(nblib PRIVATE box.cpp - forcecalculator.cpp gmxcalculator.cpp gmxsetup.cpp integrator.cpp @@ -132,7 +131,7 @@ if(GMX_INSTALL_NBLIB_API) basicdefinitions.h box.h exception.h - forcecalculator.h + gmxcalculator.h integrator.h interactions.h molecules.h diff --git a/api/nblib/forcecalculator.cpp b/api/nblib/forcecalculator.cpp deleted file mode 100644 index 06a52872b4..0000000000 --- a/api/nblib/forcecalculator.cpp +++ /dev/null @@ -1,80 +0,0 @@ -/* - * This file is part of the GROMACS molecular simulation package. - * - * Copyright (c) 2020,2021, by the GROMACS development team, led by - * Mark Abraham, David van der Spoel, Berk Hess, and Erik Lindahl, - * and including many others, as listed in the AUTHORS file in the - * top-level source directory and at http://www.gromacs.org. - * - * GROMACS is free software; you can redistribute it and/or - * modify it under the terms of the GNU Lesser General Public License - * as published by the Free Software Foundation; either version 2.1 - * of the License, or (at your option) any later version. - * - * GROMACS is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - * Lesser General Public License for more details. - * - * You should have received a copy of the GNU Lesser General Public - * License along with GROMACS; if not, see - * http://www.gnu.org/licenses, or write to the Free Software Foundation, - * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. - * - * If you want to redistribute modifications to GROMACS, please - * consider that scientific software is very special. Version - * control is crucial - bugs must be traceable. We will be happy to - * consider code for inclusion in the official distribution, but - * derived work must not be called official GROMACS. Details are found - * in the README & COPYING files - if they are missing, get the - * official version at http://www.gromacs.org. - * - * To help us fund GROMACS development, we humbly ask that you cite - * the research papers on the package. Check out http://www.gromacs.org. - */ -/*! \internal \file - * \brief Implements nblib ForceCalculator - * - * \author Victor Holanda - * \author Joe Jordan - * \author Prashanth Kanduri - * \author Sebastian Keller - */ -#include "nblib/exception.h" -#include "nblib/forcecalculator.h" -#include "nblib/gmxcalculator.h" -#include "nblib/gmxsetup.h" -#include "gromacs/utility/arrayref.h" - -namespace nblib -{ - -ForceCalculator::~ForceCalculator() = default; - -ForceCalculator::ForceCalculator(const SimulationState& system, const NBKernelOptions& options) -{ - if (options.useGpu) - { - throw InputException("GPUs are not supported for force calculations yet."); - } - gmxForceCalculator_ = GmxSetupDirector::setupGmxForceCalculator(system, options); -} - -void ForceCalculator::compute(gmx::ArrayRef coordinates, gmx::ArrayRef forces) -{ - if (coordinates.size() != forces.size()) - { - throw InputException("Coordinates array and force buffer size mismatch"); - } - - gmxForceCalculator_->compute(coordinates, forces); -} - -void ForceCalculator::updatePairList(gmx::ArrayRef particleInfoAllVdW, - gmx::ArrayRef coordinates, - const Box& box) -{ - gmxForceCalculator_->setParticlesOnGrid(particleInfoAllVdW, coordinates, box); -} - -} // namespace nblib diff --git a/api/nblib/forcecalculator.h b/api/nblib/forcecalculator.h deleted file mode 100644 index 8a16c945e2..0000000000 --- a/api/nblib/forcecalculator.h +++ /dev/null @@ -1,113 +0,0 @@ -/* - * This file is part of the GROMACS molecular simulation package. - * - * Copyright (c) 2020,2021, by the GROMACS development team, led by - * Mark Abraham, David van der Spoel, Berk Hess, and Erik Lindahl, - * and including many others, as listed in the AUTHORS file in the - * top-level source directory and at http://www.gromacs.org. - * - * GROMACS is free software; you can redistribute it and/or - * modify it under the terms of the GNU Lesser General Public License - * as published by the Free Software Foundation; either version 2.1 - * of the License, or (at your option) any later version. - * - * GROMACS is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - * Lesser General Public License for more details. - * - * You should have received a copy of the GNU Lesser General Public - * License along with GROMACS; if not, see - * http://www.gnu.org/licenses, or write to the Free Software Foundation, - * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. - * - * If you want to redistribute modifications to GROMACS, please - * consider that scientific software is very special. Version - * control is crucial - bugs must be traceable. We will be happy to - * consider code for inclusion in the official distribution, but - * derived work must not be called official GROMACS. Details are found - * in the README & COPYING files - if they are missing, get the - * official version at http://www.gromacs.org. - * - * To help us fund GROMACS development, we humbly ask that you cite - * the research papers on the package. Check out http://www.gromacs.org. - */ -/*! \inpublicapi \file - * \brief - * Implements nblib ForceCalculator - * - * \author Victor Holanda - * \author Joe Jordan - * \author Prashanth Kanduri - * \author Sebastian Keller - */ -#ifndef NBLIB_FORCECALCULATOR_H -#define NBLIB_FORCECALCULATOR_H - -#include "nblib/interactions.h" -#include "nblib/kerneloptions.h" -#include "nblib/simulationstate.h" - -namespace gmx -{ -template -class ArrayRef; -} // namespace gmx - -namespace nblib -{ -class NbvSetupUtil; -class GmxForceCalculator; - -/*! \brief Setups up and computes forces using gromacs backend. - * - * The ForceCalculator uses the data in the SimulationState and NBKernelOptions to opaquely - * construct all gromacs data structures needed to perform nonbonded force calculations. It is - * costly to create this object since much of the SimulationState and NBKernelOptions has to be - * passed to the gromacs backend. However, once constructed, compute can be called repeatedly only - * paying the cost of the actual nonbonded force calculation. Repeated calls to compute on the same - * coordinates will always return the same forces (within precision), so the user must update the - * positions using the forces generated here to advance a simulation. If the coordinates move - * sufficiently far from their positions at construction time, the efficiency of the calculation - * will suffer. To alleviate this, the user can call updatePairList. - * - */ -class ForceCalculator final -{ -public: - ForceCalculator(const SimulationState& system, const NBKernelOptions& options); - - ~ForceCalculator(); - - /*! \brief Dispatch the nonbonded force kernels and reduce the forces - * - * This function zeroes out all values in the passed in forces buffer, so it can be regarded as - * an output only param. - * - * \param[in] coordinates to be used for the force calculation - * \param[out] forces buffer to store the output forces - */ - void compute(gmx::ArrayRef coordinates, gmx::ArrayRef forces); - - /*! \brief Puts particles on a grid based on bounds specified by the box - * - * As compute is called repeatedly, the particles drift apart and the force computation becomes - * progressively less efficient. Calling this function recomputes the particle-particle pair - * lists so that computation can proceed efficiently. Should be called around every 100 steps. - * - * \param particleInfoAllVdW The types of the particles to be placed on grids - * \param coordinates The coordinates to be placed on grids - * \param[in] box The system simulation box - */ - void updatePairList(gmx::ArrayRef particleInfoAllVdW, - gmx::ArrayRef coordinates, - const Box& box); - -private: - //! GROMACS force calculator to compute forces - std::unique_ptr gmxForceCalculator_; -}; - -} // namespace nblib - -#endif // NBLIB_FORCECALCULATOR_H diff --git a/api/nblib/gmxsetup.cpp b/api/nblib/gmxsetup.cpp index e90cba3910..15ef55915b 100644 --- a/api/nblib/gmxsetup.cpp +++ b/api/nblib/gmxsetup.cpp @@ -131,23 +131,24 @@ void NbvSetupUtil::constructPairList(ExclusionLists exclusionLists) } -std::unique_ptr GmxSetupDirector::setupGmxForceCalculator(const SimulationState& system, - const NBKernelOptions& options) +std::unique_ptr setupGmxForceCalculator(const Topology& topology, + const std::vector& coordinates, + const Box& box, + const NBKernelOptions& options) { NbvSetupUtil nbvSetupUtil; nbvSetupUtil.setExecutionContext(options); - nbvSetupUtil.setNonBondedParameters(system.topology().getParticleTypes(), - system.topology().getNonBondedInteractionMap()); - nbvSetupUtil.setParticleInfoAllVdv(system.topology().numParticles()); + nbvSetupUtil.setNonBondedParameters(topology.getParticleTypes(), + topology.getNonBondedInteractionMap()); + nbvSetupUtil.setParticleInfoAllVdv(topology.numParticles()); nbvSetupUtil.setupInteractionConst(options); nbvSetupUtil.setupStepWorkload(options); - nbvSetupUtil.setupNbnxmInstance(system.topology().getParticleTypes().size(), options); - nbvSetupUtil.setParticlesOnGrid(system.coordinates(), system.box()); - nbvSetupUtil.constructPairList(system.topology().exclusionLists()); - nbvSetupUtil.setAtomProperties(system.topology().getParticleTypeIdOfAllParticles(), - system.topology().getCharges()); - nbvSetupUtil.setupForceRec(system.box().legacyMatrix()); + nbvSetupUtil.setupNbnxmInstance(topology.getParticleTypes().size(), options); + nbvSetupUtil.setParticlesOnGrid(coordinates, box); + nbvSetupUtil.constructPairList(topology.exclusionLists()); + nbvSetupUtil.setAtomProperties(topology.getParticleTypeIdOfAllParticles(), topology.getCharges()); + nbvSetupUtil.setupForceRec(box.legacyMatrix()); return nbvSetupUtil.getGmxForceCalculator(); } diff --git a/api/nblib/gmxsetup.h b/api/nblib/gmxsetup.h index bef3ee740d..4ac7a1dca3 100644 --- a/api/nblib/gmxsetup.h +++ b/api/nblib/gmxsetup.h @@ -124,23 +124,11 @@ private: std::unique_ptr gmxForceCalculator_; }; -/*! \brief Calls the setup utilities needed to initialize a GmxForceCalculator object - * - * The GmxSetupDirector encapsulates the multi-stage setup of the GmxForceCalculator which - * is done using the public functions of the NbvSetupUtil. This separation ensures that the - * NbvSetupUtil object is temporary in scope. The function definition makes it easy for the - * developers to follow the sequence of calls and the dataflow involved in setting up - * the non-bonded force calculation backend. This is the only function needed to be called - * from the ForceCalculator during construction. - * - */ -class GmxSetupDirector -{ -public: - //! Sets up and returns a GmxForceCalculator - static std::unique_ptr setupGmxForceCalculator(const SimulationState& system, - const NBKernelOptions& options); -}; +//! Sets up and returns a GmxForceCalculator +std::unique_ptr setupGmxForceCalculator(const Topology& topology, + const std::vector& coordinates, + const Box& box, + const NBKernelOptions& options); } // namespace nblib #endif // NBLIB_GMXSETUP_H diff --git a/api/nblib/nblib.h b/api/nblib/nblib.h index fb81f60ed7..636c92bc47 100644 --- a/api/nblib/nblib.h +++ b/api/nblib/nblib.h @@ -47,7 +47,8 @@ #include "nblib/basicdefinitions.h" #include "nblib/box.h" -#include "nblib/forcecalculator.h" +#include "nblib/gmxcalculator.h" +#include "nblib/gmxsetup.h" #include "nblib/integrator.h" #include "nblib/interactions.h" #include "nblib/kerneloptions.h" diff --git a/api/nblib/samples/argon-forces-integration.cpp b/api/nblib/samples/argon-forces-integration.cpp index 47895d33da..d979c4944a 100644 --- a/api/nblib/samples/argon-forces-integration.cpp +++ b/api/nblib/samples/argon-forces-integration.cpp @@ -107,14 +107,15 @@ int main() // Some performance flags can be set a run time options.nbnxmSimd = nblib::SimdKernels::SimdNo; // The force calculator contains all the data needed to compute forces. - nblib::ForceCalculator forceCalculator(simState, options); + auto forceCalculator = nblib::setupGmxForceCalculator( + simState.topology(), simState.coordinates(), simState.box(), options); // Integration requires masses, positions, and forces nblib::LeapFrog integrator(simState.topology(), simState.box()); // Print some diagnostic info printf("initial forces on particle 0: x %4f y %4f z %4f\n", forces[0][0], forces[0][1], forces[0][2]); // The forces are computed for the user gmx::ArrayRef userForces(simState.forces()); - forceCalculator.compute(simState.coordinates(), userForces); + forceCalculator->compute(simState.coordinates(), userForces); // Print some diagnostic info printf(" final forces on particle 0: x %4f y %4f z %4f\n", userForces[0][0], diff --git a/api/nblib/samples/methane-water-integration.cpp b/api/nblib/samples/methane-water-integration.cpp index a5a02c325d..4dbf3b46d7 100644 --- a/api/nblib/samples/methane-water-integration.cpp +++ b/api/nblib/samples/methane-water-integration.cpp @@ -174,7 +174,8 @@ int main() SimulationState simulationState(coordinates, velocities, forces, box, topology); // The non-bonded force calculator contains all the data needed to compute forces - ForceCalculator forceCalculator(simulationState, options); + auto forceCalculator = nblib::setupGmxForceCalculator( + simulationState.topology(), simulationState.coordinates(), simulationState.box(), options); // The listed force calculator is also initialized with the required arguments ListedForceCalculator listedForceCalculator( @@ -197,7 +198,7 @@ int main() { zeroCartesianArray(simulationState.forces()); - forceCalculator.compute(simulationState.coordinates(), simulationState.forces()); + forceCalculator->compute(simulationState.coordinates(), simulationState.forces()); listedForceCalculator.compute(simulationState.coordinates(), simulationState.forces()); diff --git a/api/nblib/tests/gmxcalculator.cpp b/api/nblib/tests/gmxcalculator.cpp index ae10568258..141ce59023 100644 --- a/api/nblib/tests/gmxcalculator.cpp +++ b/api/nblib/tests/gmxcalculator.cpp @@ -61,8 +61,9 @@ TEST(NBlibTest, GmxForceCalculatorCanCompute) SimulationState simState = argonSystemBuilder.setupSimulationState(); NBKernelOptions options = NBKernelOptions(); options.nbnxmSimd = SimdKernels::SimdNo; - std::unique_ptr gmxForceCalculator = - nblib::GmxSetupDirector::setupGmxForceCalculator(simState, options); + std::unique_ptr gmxForceCalculator = nblib::setupGmxForceCalculator( + simState.topology(), simState.coordinates(), simState.box(), options); + ; EXPECT_NO_THROW(gmxForceCalculator->compute(simState.coordinates(), simState.forces())); } diff --git a/api/nblib/tests/nbkernelsystem.cpp b/api/nblib/tests/nbkernelsystem.cpp index 022317375d..15163e1384 100644 --- a/api/nblib/tests/nbkernelsystem.cpp +++ b/api/nblib/tests/nbkernelsystem.cpp @@ -45,7 +45,7 @@ #include #include "gromacs/topology/exclusionblocks.h" -#include "nblib/forcecalculator.h" +#include "nblib/gmxcalculator.h" #include "nblib/gmxsetup.h" #include "nblib/integrator.h" #include "nblib/tests/testhelpers.h" @@ -69,10 +69,11 @@ TEST(NBlibTest, SpcMethanolForcesAreCorrect) SpcMethanolSimulationStateBuilder spcMethanolSystemBuilder; auto simState = spcMethanolSystemBuilder.setupSimulationState(); - auto forceCalculator = ForceCalculator(simState, options); + auto forceCalculator = nblib::setupGmxForceCalculator( + simState.topology(), simState.coordinates(), simState.box(), options); gmx::ArrayRef forces(simState.forces()); - ASSERT_NO_THROW(forceCalculator.compute(simState.coordinates(), forces)); + ASSERT_NO_THROW(forceCalculator->compute(simState.coordinates(), forces)); RefDataChecker forcesOutputTest(5e-5); forcesOutputTest.testArrays(forces, "SPC-methanol forces"); @@ -86,10 +87,11 @@ TEST(NBlibTest, ExpectedNumberOfForces) SpcMethanolSimulationStateBuilder spcMethanolSystemBuilder; auto simState = spcMethanolSystemBuilder.setupSimulationState(); - auto forceCalculator = ForceCalculator(simState, options); + auto forceCalculator = nblib::setupGmxForceCalculator( + simState.topology(), simState.coordinates(), simState.box(), options); gmx::ArrayRef forces(simState.forces()); - forceCalculator.compute(simState.coordinates(), forces); + forceCalculator->compute(simState.coordinates(), forces); EXPECT_EQ(simState.topology().numParticles(), forces.size()); } @@ -102,14 +104,15 @@ TEST(NBlibTest, CanIntegrateSystem) SpcMethanolSimulationStateBuilder spcMethanolSystemBuilder; auto simState = spcMethanolSystemBuilder.setupSimulationState(); - auto forceCalculator = ForceCalculator(simState, options); + auto forceCalculator = nblib::setupGmxForceCalculator( + simState.topology(), simState.coordinates(), simState.box(), options); LeapFrog integrator(simState.topology(), simState.box()); for (int iter = 0; iter < options.numIterations; iter++) { gmx::ArrayRef forces(simState.forces()); - forceCalculator.compute(simState.coordinates(), simState.forces()); + forceCalculator->compute(simState.coordinates(), simState.forces()); EXPECT_NO_THROW(integrator.integrate( 1.0, simState.coordinates(), simState.velocities(), simState.forces())); } @@ -137,13 +140,14 @@ TEST(NBlibTest, UpdateChangesForces) SpcMethanolSimulationStateBuilder spcMethanolSystemBuilder; auto simState = spcMethanolSystemBuilder.setupSimulationState(); - auto forceCalculator = ForceCalculator(simState, options); + auto forceCalculator = nblib::setupGmxForceCalculator( + simState.topology(), simState.coordinates(), simState.box(), options); LeapFrog integrator(simState.topology(), simState.box()); // step 1 gmx::ArrayRef forces(simState.forces()); - forceCalculator.compute(simState.coordinates(), simState.forces()); + forceCalculator->compute(simState.coordinates(), simState.forces()); // copy computed forces to another array std::vector forces_1(forces.size()); @@ -153,7 +157,7 @@ TEST(NBlibTest, UpdateChangesForces) zeroCartesianArray(forces); // check if forces change without update step - forceCalculator.compute(simState.coordinates(), forces); + forceCalculator->compute(simState.coordinates(), forces); // check if forces change without update for (size_t i = 0; i < forces_1.size(); i++) @@ -171,7 +175,7 @@ TEST(NBlibTest, UpdateChangesForces) zeroCartesianArray(forces); // step 2 - forceCalculator.compute(simState.coordinates(), forces); + forceCalculator->compute(simState.coordinates(), forces); std::vector forces_2(forces.size()); std::copy(forces.begin(), forces.end(), begin(forces_2)); @@ -194,10 +198,11 @@ TEST(NBlibTest, ArgonOplsaForcesAreCorrect) ArgonSimulationStateBuilder argonSystemBuilder(fftypes::OPLSA); auto simState = argonSystemBuilder.setupSimulationState(); - auto forceCalculator = ForceCalculator(simState, options); + auto forceCalculator = nblib::setupGmxForceCalculator( + simState.topology(), simState.coordinates(), simState.box(), options); gmx::ArrayRef testForces(simState.forces()); - forceCalculator.compute(simState.coordinates(), simState.forces()); + forceCalculator->compute(simState.coordinates(), simState.forces()); RefDataChecker forcesOutputTest(1e-7); forcesOutputTest.testArrays(testForces, "Argon forces"); @@ -212,10 +217,11 @@ TEST(NBlibTest, ArgonGromos43A1ForcesAreCorrect) ArgonSimulationStateBuilder argonSystemBuilder(fftypes::GROMOS43A1); auto simState = argonSystemBuilder.setupSimulationState(); - auto forceCalculator = ForceCalculator(simState, options); + auto forceCalculator = nblib::setupGmxForceCalculator( + simState.topology(), simState.coordinates(), simState.box(), options); gmx::ArrayRef testForces(simState.forces()); - forceCalculator.compute(simState.coordinates(), simState.forces()); + forceCalculator->compute(simState.coordinates(), simState.forces()); RefDataChecker forcesOutputTest(1e-7); forcesOutputTest.testArrays(testForces, "Argon forces"); -- 2.22.0