Simplify energy-file comparisons in tests
authorMark Abraham <mark.j.abraham@gmail.com>
Tue, 27 Aug 2019 11:44:24 +0000 (13:44 +0200)
committerMark Abraham <mark.j.abraham@gmail.com>
Tue, 3 Sep 2019 04:49:41 +0000 (06:49 +0200)
Now that we use newer infrastructure, we can use lambdas.

Also made FramePairManager and ContinuationFramePairManager
more consistent.

Change-Id: I8ccc9dd3134e5e188c3ea2b8b6ab335b4f267b10

src/programs/mdrun/tests/energycomparison.h
src/programs/mdrun/tests/exactcontinuation.cpp
src/programs/mdrun/tests/mdruncomparison.h
src/programs/mdrun/tests/simulatorcomparison.cpp

index 6a010646a7222f600be6713d6ab156495c926fb5..3562cfe89602b921cf65be3f9e3f286b858ef09b 100644 (file)
@@ -88,24 +88,6 @@ checkEnergiesAgainstReferenceData(const std::string      &energyFilename,
                                   const EnergyTolerances &energiesToMatch,
                                   TestReferenceChecker   *checker);
 
-/*! \internal
- * \brief Functor for comparing reference and test frames on
- * particular energies to match. */
-class EnergyComparator
-{
-    public:
-        //! Constructor
-        EnergyComparator(const EnergyTolerances &energiesToMatch)
-            : energiesToMatch_(energiesToMatch) {}
-        //! The functor method.
-        void operator()(const EnergyFrame &reference, const EnergyFrame &test)
-        {
-            compareEnergyFrames(reference, test, energiesToMatch_);
-        }
-        //! Container of the energies to match and the tolerance required.
-        const EnergyTolerances &energiesToMatch_;
-};
-
 }  // namespace test
 }  // namespace gmx
 
index 0efdd338f777cb4a214eba2be22803a9a6ba0992..41cbce52c2a0b03b3a0cd9c6632567cab1a23df0 100644 (file)
@@ -79,7 +79,7 @@ namespace
  * one- or two-part run.
  *
  * \todo This class has a similar interface with one in
- * mdcomparison.cpp, but not enough to warrant extracting an interface
+ * mdcomparison.h, but not enough to warrant extracting an interface
  * class. Perhaps parts of this should be cast as a class that returns
  * iterators to successive frames, so that the fancy footwork for
  * pretending a two-part run is one sequence is separate from the
@@ -192,8 +192,8 @@ class ContinuationFramePairManager
         {
             while (shouldContinueComparing())
             {
-                Frame firstFrame  = full_->frame();
-                Frame secondFrame = isFirstPart_ ? firstPart_->frame() : secondPart_->frame();
+                EnergyFrame firstFrame  = full_->frame();
+                EnergyFrame secondFrame = isFirstPart_ ? firstPart_->frame() : secondPart_->frame();
                 SCOPED_TRACE("Comparing frames from two runs '" + firstFrame.frameName() + "' and '" + secondFrame.frameName() + "'");
                 compareTwoFrames(firstFrame, secondFrame);
             }
@@ -298,13 +298,12 @@ void runTest(TestFileManager        *fileManager,
         secondPartRunEdrFileName += ".part0002.edr";
     }
 
-    // Build the functor that will compare reference and test
-    // energy frames on the chosen energy fields.
-    //
-    // TODO It would be less code if we used a lambda for this, but either
-    // clang 3.4 or libstdc++ 5.2.1 have an issue with capturing a
-    // std::unordered_map
-    EnergyComparator energyComparator(energiesToMatch);
+    // Build the functor that will compare energy frames on the chosen
+    // energy fields.
+    auto energyComparator = [&energiesToMatch](const EnergyFrame &fullRun, const EnergyFrame &twoPartRun)
+        {
+            compareEnergyFrames(fullRun, twoPartRun, energiesToMatch);
+        };
     // Build the manager that will present matching pairs of frames to compare.
     //
     // TODO Here is an unnecessary copy of keys (ie. the energy field
index 4c19e72103bfce8ebb253e5b97f28cc5250e6d0a..323822454a8fd638a085f53216e69924c351211e 100644 (file)
@@ -1,7 +1,7 @@
 /*
  * This file is part of the GROMACS molecular simulation package.
  *
- * Copyright (c) 2016,2018, by the GROMACS development team, led by
+ * Copyright (c) 2016,2018,2019, 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,8 +53,15 @@ namespace test
 
 /*! \internal
  * \brief Manages returning a pair of frames from two
- * equivalent simulations that are meaningful to compare. */
-template <class FrameReader, class Frame>
+ * equivalent simulations that are meaningful to compare.
+ *
+ * \todo This is largely duplicated by ContinuationFrameManager. These
+ * could be refactored into components that compare iterators to
+ * frames.
+ *
+ * \tparam FrameReader  Has readNextFrame() and frame() methods
+ *                      useful for returning successive Frame objects */
+template <class FrameReader>
 class FramePairManager
 {
     public:
@@ -101,13 +108,17 @@ class FramePairManager
             return false;
         }
     public:
-        //! Compare all possible pairs of frames using \c compareTwoFrames.
+        /*! \brief Compare all possible pairs of frames using \c compareTwoFrames.
+         *
+         * \tparam Frame  The type of frame used in the comparison (returned
+         *                by FrameReader and used by compareTwoFrames). */
+        template <class Frame>
         void compareAllFramePairs(std::function<void(const Frame &, const Frame &)> compareTwoFrames)
         {
             while (shouldContinueComparing())
             {
-                auto firstFrame  = first_->frame();
-                auto secondFrame = second_->frame();
+                Frame firstFrame  = first_->frame();
+                Frame secondFrame = second_->frame();
                 SCOPED_TRACE("Comparing frames from two runs '" + firstFrame.frameName() + "' and '" + secondFrame.frameName() + "'");
                 compareTwoFrames(firstFrame, secondFrame);
             }
index 72ffcfa2b1cf0c40aab4480e19884fed937fea80..1383bf5546337c5d38f2e6b6eccd57e66f4d0136 100644 (file)
@@ -62,22 +62,6 @@ namespace test
 namespace
 {
 
-//! Functor for comparing reference and test frames on particular energies to match.
-class EnergyComparator
-{
-    public:
-        //! Constructor
-        explicit EnergyComparator(const EnergyTolerances &energiesToMatch)
-            : energiesToMatch_(energiesToMatch) {}
-        //! The functor method.
-        void operator()(const EnergyFrame &reference, const EnergyFrame &test)
-        {
-            compareEnergyFrames(reference, test, energiesToMatch_);
-        }
-        //! Container of the energies to match and the tolerance required.
-        const EnergyTolerances &energiesToMatch_;
-};
-
 //! Run grompp and mdrun for both sets of mdp field values
 template <bool doEnvironmentVariable, bool doRerun>
 void executeSimulatorComparisonTestImpl(
@@ -174,24 +158,23 @@ void executeSimulatorComparisonTestImpl(
         }
     }
 
-    // Build the functor that will compare reference and test
-    // energy frames on the chosen energy fields.
-    //
-    // TODO It would be less code if we used a lambda for this, but either
-    // clang 3.4 or libstdc++ 5.2.1 have an issue with capturing a
-    // std::unordered_map
-    EnergyComparator energyComparator(energiesToMatch);
+    // Build the functor that will compare energy frames on the chosen
+    // energy fields.
+    auto energyComparator = [&energiesToMatch](const EnergyFrame &first, const EnergyFrame &second)
+        {
+            compareEnergyFrames(first, second, energiesToMatch);
+        };
     // Build the manager that will present matching pairs of frames to compare.
     //
     // TODO Here is an unnecessary copy of keys (ie. the energy field
     // names), for convenience. In the future, use a range.
     auto namesOfEnergiesToMatch = getKeys(energiesToMatch);
-    FramePairManager<EnergyFrameReader, EnergyFrame>
+    FramePairManager<EnergyFrameReader>
          energyManager(
             openEnergyFileToReadFields(simulator1EdrFileName, namesOfEnergiesToMatch),
             openEnergyFileToReadFields(simulator2EdrFileName, namesOfEnergiesToMatch));
     // Compare the energy frames.
-    energyManager.compareAllFramePairs(energyComparator);
+    energyManager.compareAllFramePairs<EnergyFrame>(energyComparator);
 
     // Specify how trajectory frame matching must work.
     TrajectoryFrameMatchSettings trajectoryMatchSettings {
@@ -213,11 +196,11 @@ void executeSimulatorComparisonTestImpl(
             compareTrajectoryFrames(reference, test, trajectoryMatchSettings, trajectoryTolerances);
         };
     // Build the manager that will present matching pairs of frames to compare
-    FramePairManager<TrajectoryFrameReader, TrajectoryFrame>
+    FramePairManager<TrajectoryFrameReader>
     trajectoryManager(std::make_unique<TrajectoryFrameReader>(simulator1TrajectoryFileName),
                       std::make_unique<TrajectoryFrameReader>(simulator2TrajectoryFileName));
     // Compare the trajectory frames.
-    trajectoryManager.compareAllFramePairs(trajectoryComparator);
+    trajectoryManager.compareAllFramePairs<TrajectoryFrame>(trajectoryComparator);
 }
 }       // namespace