Remove useless top level nblib force calculator
authorejjordan <ejjordan@kth.se>
Wed, 22 Sep 2021 16:52:37 +0000 (18:52 +0200)
committerJoe Jordan <ejjordan12@gmail.com>
Thu, 23 Sep 2021 02:18:54 +0000 (02:18 +0000)
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
api/nblib/forcecalculator.cpp [deleted file]
api/nblib/forcecalculator.h [deleted file]
api/nblib/gmxsetup.cpp
api/nblib/gmxsetup.h
api/nblib/nblib.h
api/nblib/samples/argon-forces-integration.cpp
api/nblib/samples/methane-water-integration.cpp
api/nblib/tests/gmxcalculator.cpp
api/nblib/tests/nbkernelsystem.cpp

index 2b1b0b70fb6bba4f1e13f7e84257f015b2db222e..31eca82e77a6ffb630b1c8f03cc5927e18f673fd 100644 (file)
@@ -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 (file)
index 06a5287..0000000
+++ /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 <victor.holanda@cscs.ch>
- * \author Joe Jordan <ejjordan@kth.se>
- * \author Prashanth Kanduri <kanduri@cscs.ch>
- * \author Sebastian Keller <keller@cscs.ch>
- */
-#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<const Vec3> coordinates, gmx::ArrayRef<Vec3> forces)
-{
-    if (coordinates.size() != forces.size())
-    {
-        throw InputException("Coordinates array and force buffer size mismatch");
-    }
-
-    gmxForceCalculator_->compute(coordinates, forces);
-}
-
-void ForceCalculator::updatePairList(gmx::ArrayRef<const int64_t> particleInfoAllVdW,
-                                     gmx::ArrayRef<Vec3>          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 (file)
index 8a16c94..0000000
+++ /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 <victor.holanda@cscs.ch>
- * \author Joe Jordan <ejjordan@kth.se>
- * \author Prashanth Kanduri <kanduri@cscs.ch>
- * \author Sebastian Keller <keller@cscs.ch>
- */
-#ifndef NBLIB_FORCECALCULATOR_H
-#define NBLIB_FORCECALCULATOR_H
-
-#include "nblib/interactions.h"
-#include "nblib/kerneloptions.h"
-#include "nblib/simulationstate.h"
-
-namespace gmx
-{
-template<typename T>
-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<const Vec3> coordinates, gmx::ArrayRef<Vec3> 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<const int64_t> particleInfoAllVdW,
-                        gmx::ArrayRef<Vec3>          coordinates,
-                        const Box&                   box);
-
-private:
-    //! GROMACS force calculator to compute forces
-    std::unique_ptr<GmxForceCalculator> gmxForceCalculator_;
-};
-
-} // namespace nblib
-
-#endif // NBLIB_FORCECALCULATOR_H
index e90cba391027dddcc2b4ef87f145ff9fac6f20aa..15ef55915be19198b685e9f987cfcfe98d1cd203 100644 (file)
@@ -131,23 +131,24 @@ void NbvSetupUtil::constructPairList(ExclusionLists<int> exclusionLists)
 }
 
 
-std::unique_ptr<GmxForceCalculator> GmxSetupDirector::setupGmxForceCalculator(const SimulationState& system,
-                                                                              const NBKernelOptions& options)
+std::unique_ptr<GmxForceCalculator> setupGmxForceCalculator(const Topology&          topology,
+                                                            const std::vector<Vec3>& 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();
 }
index bef3ee740d7b6a7679a73af85acbfed7179a3d2e..4ac7a1dca34628b266a36841dee5f40ff588c24c 100644 (file)
@@ -124,23 +124,11 @@ private:
     std::unique_ptr<GmxForceCalculator> 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<GmxForceCalculator> setupGmxForceCalculator(const SimulationState& system,
-                                                                       const NBKernelOptions& options);
-};
+//! Sets up and returns a GmxForceCalculator
+std::unique_ptr<GmxForceCalculator> setupGmxForceCalculator(const Topology&          topology,
+                                                            const std::vector<Vec3>& coordinates,
+                                                            const Box&               box,
+                                                            const NBKernelOptions&   options);
 
 } // namespace nblib
 #endif // NBLIB_GMXSETUP_H
index fb81f60ed71b9cf970289578cc9624db36892587..636c92bc47bb41ba913a2639d25144be7380f528 100644 (file)
@@ -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"
index 47895d33daaba41005dd2751d2c0207676ab2cd9..d979c4944a399b787bfe2779ac4cf3e873a36415 100644 (file)
@@ -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<nblib::Vec3> 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],
index a5a02c325d05dd93bd26deb46e6de22d9e075023..4dbf3b46d702ab80646d4327324a42d689cb4879 100644 (file)
@@ -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());
 
index ae10568258c8303a0698e284ece35e39cc3241d6..141ce590234b6c155391b0f1a3c64f432c86d34c 100644 (file)
@@ -61,8 +61,9 @@ TEST(NBlibTest, GmxForceCalculatorCanCompute)
     SimulationState             simState = argonSystemBuilder.setupSimulationState();
     NBKernelOptions             options  = NBKernelOptions();
     options.nbnxmSimd                    = SimdKernels::SimdNo;
-    std::unique_ptr<GmxForceCalculator> gmxForceCalculator =
-            nblib::GmxSetupDirector::setupGmxForceCalculator(simState, options);
+    std::unique_ptr<GmxForceCalculator> gmxForceCalculator = nblib::setupGmxForceCalculator(
+            simState.topology(), simState.coordinates(), simState.box(), options);
+    ;
     EXPECT_NO_THROW(gmxForceCalculator->compute(simState.coordinates(), simState.forces()));
 }
 
index 022317375df13ea0027661d0987332eb1e1515d4..15163e1384d60efdbd8c39f67cf64176440a7214 100644 (file)
@@ -45,7 +45,7 @@
 #include <gtest/gtest.h>
 
 #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<Vec3> 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<Vec3>(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<Vec3> 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<Vec3> 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<Vec3> forces(simState.forces());
-    forceCalculator.compute(simState.coordinates(), simState.forces());
+    forceCalculator->compute(simState.coordinates(), simState.forces());
 
     // copy computed forces to another array
     std::vector<Vec3> 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<Vec3> 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<Vec3> testForces(simState.forces());
-    forceCalculator.compute(simState.coordinates(), simState.forces());
+    forceCalculator->compute(simState.coordinates(), simState.forces());
 
     RefDataChecker forcesOutputTest(1e-7);
     forcesOutputTest.testArrays<Vec3>(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<Vec3> testForces(simState.forces());
-    forceCalculator.compute(simState.coordinates(), simState.forces());
+    forceCalculator->compute(simState.coordinates(), simState.forces());
 
     RefDataChecker forcesOutputTest(1e-7);
     forcesOutputTest.testArrays<Vec3>(testForces, "Argon forces");