Replace FramesToCompare enum by MaxNumFrames
authorPascal Merz <pascal.merz@me.com>
Fri, 25 Sep 2020 16:37:48 +0000 (16:37 +0000)
committerMark Abraham <mark.j.abraham@gmail.com>
Fri, 25 Sep 2020 16:37:48 +0000 (16:37 +0000)
This allows to explicitly chose a maximum number of frames to be
tested, rather than only chosing to compare the first frame or
all frames. It also implements to chose the number of frames for
tests comparing to reference data rather than only when comparing
simulation results.

12 files changed:
src/programs/mdrun/tests/comparison_helpers.h
src/programs/mdrun/tests/energycomparison.cpp
src/programs/mdrun/tests/energycomparison.h
src/programs/mdrun/tests/exactcontinuation.cpp
src/programs/mdrun/tests/periodicactions.cpp
src/programs/mdrun/tests/rerun.cpp
src/programs/mdrun/tests/simulator.cpp
src/programs/mdrun/tests/simulatorcomparison.cpp
src/programs/mdrun/tests/trajectorycomparison.cpp
src/programs/mdrun/tests/trajectorycomparison.h
src/testutils/refdata.cpp
src/testutils/refdata.h

index 0cde6c8dbef5814a5d40bce08e20e7148b55b86f..d55c0a2274b812c7a1446b6b854fd867199fba90 100644 (file)
@@ -49,12 +49,25 @@ namespace gmx
 namespace test
 {
 
-//! Enumeration controlling which trajectory frames are compared
-enum class FramesToCompare : int
+/*! \internal
+ * \brief Named struct indicating the max number of frames to be compared */
+struct MaxNumFrames
 {
-    AllFrames,
-    OnlyFirstFrame, // e.g. because step-0 quantities are most useful to compare
-    Count
+    //! Explicit constructor
+    explicit MaxNumFrames(unsigned int maxFrame) : maxFrame_(maxFrame) {}
+
+    //! Implicit conversion to int - struct can be used like underlying type
+    operator unsigned int() const { return maxFrame_; }
+
+    //! Return a MaxNumFrames that will try to compare all frames
+    [[nodiscard]] static MaxNumFrames compareAllFrames()
+    {
+        return MaxNumFrames(std::numeric_limits<decltype(maxFrame_)>::max());
+    }
+
+private:
+    //! Internal value
+    const unsigned int maxFrame_;
 };
 
 } // namespace test
index f32520330c36a1a6c8819054f85c2bcdcc0ceaee..cea5e9d021dd1ace8f69b2d42c6a7976c9acf045 100644 (file)
@@ -78,9 +78,9 @@ EnergyTermsToCompare EnergyComparison::defaultEnergyTermsToCompare()
 };
 
 EnergyComparison::EnergyComparison(const EnergyTermsToCompare& energyTermsToCompare,
-                                   FramesToCompare             framesToCompare) :
+                                   MaxNumFrames                maxNumFrames) :
     energyTermsToCompare_(energyTermsToCompare),
-    framesToCompare_(framesToCompare)
+    maxNumFrames_(maxNumFrames)
 {
 }
 
@@ -97,7 +97,7 @@ std::vector<std::string> EnergyComparison::getEnergyNames() const
 
 void EnergyComparison::operator()(const EnergyFrame& reference, const EnergyFrame& test) const
 {
-    if (framesToCompare_ == FramesToCompare::OnlyFirstFrame && firstFrameHasBeenCompared_)
+    if (numComparedFrames_ >= maxNumFrames_)
     {
         // Nothing should be compared
         return;
@@ -122,18 +122,19 @@ void EnergyComparison::operator()(const EnergyFrame& reference, const EnergyFram
             ADD_FAILURE() << "Could not find energy component from reference frame in test frame";
         }
     }
-    firstFrameHasBeenCompared_ = true;
+    numComparedFrames_++;
 }
 
 void checkEnergiesAgainstReferenceData(const std::string&          energyFilename,
                                        const EnergyTermsToCompare& energyTermsToCompare,
-                                       TestReferenceChecker*       checker)
+                                       TestReferenceChecker*       checker,
+                                       MaxNumFrames                maxNumEnergyFrames)
 {
     const bool thisRankChecks = (gmx_node_rank() == 0);
 
     if (thisRankChecks)
     {
-        EnergyComparison energyComparison(energyTermsToCompare, FramesToCompare::AllFrames);
+        EnergyComparison energyComparison(energyTermsToCompare, maxNumEnergyFrames);
         auto energyReader = openEnergyFileToReadTerms(energyFilename, energyComparison.getEnergyNames());
 
         std::unordered_map<std::string, TestReferenceChecker> checkers;
@@ -150,11 +151,12 @@ void checkEnergiesAgainstReferenceData(const std::string&          energyFilenam
         // frames with the same step number. But we need a unique
         // identifier so we match the intended reference data, so we
         // keep track of the number of the frame read from the file.
-        int frameNumber = 0;
-        while (energyReader->readNextFrame())
+        unsigned int frameNumber = 0;
+        while (frameNumber < maxNumEnergyFrames && energyReader->readNextFrame())
         {
-            const EnergyFrame& frame     = energyReader->frame();
-            const std::string  frameName = frame.frameName() + " in frame " + toString(frameNumber);
+            const EnergyFrame& frame = energyReader->frame();
+            const std::string  frameName =
+                    frame.frameName() + " in frame " + toString(static_cast<int64_t>(frameNumber));
 
             SCOPED_TRACE("Comparing frame " + frameName);
             for (const auto& energyTermToCompare : energyTermsToCompare)
@@ -167,6 +169,11 @@ void checkEnergiesAgainstReferenceData(const std::string&          energyFilenam
             }
             ++frameNumber;
         }
+        if (frameNumber == maxNumEnergyFrames && energyReader->readNextFrame())
+        {
+            // There would have been at least one more frame!
+            checker->disableUnusedEntriesCheck();
+        }
     }
     else
     {
@@ -174,5 +181,13 @@ void checkEnergiesAgainstReferenceData(const std::string&          energyFilenam
     }
 }
 
+void checkEnergiesAgainstReferenceData(const std::string&          energyFilename,
+                                       const EnergyTermsToCompare& energyTermsToCompare,
+                                       TestReferenceChecker*       checker)
+{
+    checkEnergiesAgainstReferenceData(energyFilename, energyTermsToCompare, checker,
+                                      MaxNumFrames::compareAllFrames());
+}
+
 } // namespace test
 } // namespace gmx
index b43d45692d670b4e48e103cbd38c866fcecad942..c33f66331b260d9c52af6506da4f9aefe754598f 100644 (file)
@@ -71,7 +71,7 @@ public:
     //! Defaults for energy comparisons
     static EnergyTermsToCompare defaultEnergyTermsToCompare();
     //! Constructor
-    EnergyComparison(const EnergyTermsToCompare& energyTermsToCompare, FramesToCompare framesToCompare);
+    EnergyComparison(const EnergyTermsToCompare& energyTermsToCompare, MaxNumFrames maxNumFrames);
     /*! \brief Return the names of energies that will be compared
      *
      * This function can be used to provide an input for
@@ -94,15 +94,15 @@ public:
 private:
     //! Energy terms to match with given tolerances.
     EnergyTermsToCompare energyTermsToCompare_;
-    //! Which frames should be compared.
-    FramesToCompare framesToCompare_ = FramesToCompare::AllFrames;
-    /*! \brief Whether the first frame has been compared yet
+    //! How many frames should be compared.
+    MaxNumFrames maxNumFrames_ = MaxNumFrames::compareAllFrames();
+    /*! \brief The number of frames that have been compared until now
      *
      * This field is mutable because the need to update the flag
-     * after the first frame is merely an implementation detail,
+     * when checking frames is merely an implementation detail,
      * rather than a proper change of internal state triggered
      * by the caller. */
-    mutable bool firstFrameHasBeenCompared_ = false;
+    mutable unsigned int numComparedFrames_ = 0;
 };
 
 /*! \brief Check a subset of the energies found in an energy file
@@ -114,10 +114,24 @@ private:
  * \param[in]  energyFilename        The name of an energy file.
  * \param[in]  energyTermsToCompare  Set of energies to match at given tolerances.
  * \param[in]  checker               Root checker for reference data.
+ * \param[in]  maxNumEnergyFrames    The maximum number of frames to check
  *
  * \todo This is quite similar to the functionality used in PmeTest,
  * and we should consider reducing the duplication.
  */
+void checkEnergiesAgainstReferenceData(const std::string&          energyFilename,
+                                       const EnergyTermsToCompare& energyTermsToCompare,
+                                       TestReferenceChecker*       checker,
+                                       MaxNumFrames                maxNumEnergyFrames);
+
+/*!
+ * \brief Check a subset of the energies found in an energy file
+ * against reference data.
+ *
+ * Convenience overload using all frames
+ *
+ * \see checkEnergiesAgainstReferenceData(const std::string&, const EnergyTermsToCompare&, TestReferenceChecker*, MaxNumFrames)
+ */
 void checkEnergiesAgainstReferenceData(const std::string&          energyFilename,
                                        const EnergyTermsToCompare& energyTermsToCompare,
                                        TestReferenceChecker*       checker);
index 737d98a38946e40ba25e8df3b7925cecc229d4f4..ba2abb9b61a423347178e704d286beaa760a96df 100644 (file)
@@ -303,7 +303,7 @@ void runTest(TestFileManager*            fileManager,
 
     // Build the functor that will compare energy frames on the chosen
     // energy terms.
-    EnergyComparison energyComparison(energyTermsToCompare, FramesToCompare::AllFrames);
+    EnergyComparison energyComparison(energyTermsToCompare, MaxNumFrames::compareAllFrames());
 
     // Build the manager that will present matching pairs of frames to compare.
     //
index 4f17a587959ce3f6efded8e74496c2238e1e9c0b..41c69ab839ecdf2ce86b1373695956905fca2b17 100644 (file)
@@ -107,7 +107,7 @@ public:
     ReferenceFileNames referenceFileNames_ = { fileManager_.getTemporaryFilePath("reference.edr") };
     //! Functor for energy comparison
     EnergyComparison energyComparison_{ EnergyComparison::defaultEnergyTermsToCompare(),
-                                        FramesToCompare::AllFrames };
+                                        MaxNumFrames::compareAllFrames() };
     //! Names of energies compared by energyComparison_
     std::vector<std::string> namesOfEnergiesToMatch_ = energyComparison_.getEnergyNames();
 };
index 6f1946cfa3420545085d293f6daf1a0fef1c1e79..e358b9cd1ec12878815e88384df75eeec542f13a 100644 (file)
@@ -106,7 +106,7 @@ const TrajectoryFrameMatchSettings MdrunRerunTest::trajectoryMatchSettings = {
     ComparisonConditions::MustCompare,
     ComparisonConditions::NoComparison,
     ComparisonConditions::MustCompare,
-    FramesToCompare::AllFrames
+    MaxNumFrames::compareAllFrames()
 };
 
 void executeRerunTest(TestFileManager*            fileManager,
index 6ed4f5f3c27531ced441ad871c7d9472b8df4a23..222b4ae41d02c9638ba64995ad81cc53e490d854 100644 (file)
@@ -158,7 +158,7 @@ TEST_P(SimulatorComparisonTest, WithinTolerances)
                                                           ComparisonConditions::MustCompare,
                                                           ComparisonConditions::MustCompare,
                                                           ComparisonConditions::MustCompare,
-                                                          FramesToCompare::AllFrames };
+                                                          MaxNumFrames::compareAllFrames() };
     TrajectoryTolerances trajectoryTolerances = TrajectoryComparison::s_defaultTrajectoryTolerances;
     if (simulationName != "argon12")
     {
index 64a841f16eb175c6292a6b22b9386c61d927fcf1..c832d5ab9f1c267b42b413d6ccc6c0082084fac0 100644 (file)
@@ -90,7 +90,7 @@ void compareEnergies(const std::string&          edr1Name,
 {
     // Build the functor that will compare energy frames on the chosen
     // energy terms.
-    EnergyComparison energyComparison(energyTermsToCompare, FramesToCompare::AllFrames);
+    EnergyComparison energyComparison(energyTermsToCompare, MaxNumFrames::compareAllFrames());
 
     // Build the manager that will present matching pairs of frames to compare.
     //
index b4573bbeca10de29f28e1da3d6f0b4ce72da0ad7..16a8a0980c40ec534efc29944221e65db4e76081 100644 (file)
@@ -252,7 +252,7 @@ TrajectoryComparison::TrajectoryComparison(const TrajectoryFrameMatchSettings& m
 
 void TrajectoryComparison::operator()(const TrajectoryFrame& reference, const TrajectoryFrame& test) const
 {
-    if (matchSettings_.framesToCompare == FramesToCompare::OnlyFirstFrame && firstFrameHasBeenCompared_)
+    if (numComparedFrames_ >= matchSettings_.maxNumTrajectoryFrames)
     {
         // Nothing should be compared
         return;
@@ -265,7 +265,7 @@ void TrajectoryComparison::operator()(const TrajectoryFrame& reference, const Tr
     compareCoordinates(reference, test, matchSettings_, tolerances_.coordinates);
     compareVelocities(reference, test, matchSettings_, tolerances_.velocities);
     compareForces(reference, test, matchSettings_, tolerances_.forces);
-    firstFrameHasBeenCompared_ = true;
+    numComparedFrames_++;
 }
 
 } // namespace test
index 187045baa66fce3870c93042c30c6249ed9bd03f..9e2847f7388589046f595aef4d311df1620b3d7e 100644 (file)
@@ -99,8 +99,8 @@ struct TrajectoryFrameMatchSettings
     ComparisonConditions velocitiesComparison = ComparisonConditions::CompareIfBothFound;
     //! Whether forces must be compared.
     ComparisonConditions forcesComparison = ComparisonConditions::CompareIfBothFound;
-    //! Which frames will be compared
-    FramesToCompare framesToCompare = FramesToCompare::AllFrames;
+    //! How many frames will be compared.
+    MaxNumFrames maxNumTrajectoryFrames = MaxNumFrames::compareAllFrames();
 };
 
 /*! \internal
@@ -134,13 +134,13 @@ private:
     TrajectoryFrameMatchSettings matchSettings_;
     //! Trajectory fields to match with given tolerances.
     TrajectoryTolerances tolerances_;
-    /*! \brief Whether the first frame has been compared yet
+    /*! \brief The number of frames that have been compared until now
      *
      * This field is mutable because the need to update the flag
-     * after the first frame is merely an implementation detail,
+     * when checking frames is merely an implementation detail,
      * rather than a proper change of internal state triggered
      * by the caller. */
-    mutable bool firstFrameHasBeenCompared_ = false;
+    mutable unsigned int numComparedFrames_ = 0;
 };
 
 } // namespace test
index 7686090c7aab8f2af0ae0c1378b66f0aef6de2ac..e2dc6ed6fd027e117de6343e9620d58ef181c1cd 100644 (file)
@@ -756,6 +756,14 @@ void TestReferenceChecker::checkUnusedEntries()
     }
 }
 
+void TestReferenceChecker::disableUnusedEntriesCheck()
+{
+    if (impl_->compareRootEntry_)
+    {
+        impl_->compareRootEntry_->setCheckedIncludingChildren();
+    }
+}
+
 
 bool TestReferenceChecker::checkPresent(bool bPresent, const char* id)
 {
index 3f681c5be350514cf77be90704ea688849415589..21b8a2c96019280e4ee6403fb205ae7385a3f815 100644 (file)
@@ -302,6 +302,12 @@ public:
      */
     void checkUnusedEntries();
 
+    /*! \brief Disables checking for unused entries
+     *
+     * \see checkUnusedEntries()
+     */
+    void disableUnusedEntriesCheck();
+
     /*! \brief
      * Checks whether a data item is present.
      *