Merge pull request #251 from JeffersonLab/AyanRoy16_issue_#233

Address issue #233: Resolve floating-point JParameter stringification round-trip problems

src/libraries/JANA/Services/JParameterManager.h

@@ -5,12 +5,14 @@
 #ifndef _JParameterManager_h_
 #define _JParameterManager_h_
 
+#include <limits>
 #include <string>
 #include <algorithm>
 #include <vector>
 #include <array>
 #include <map>
 #include <cmath>
+#include <iomanip>
 
 #include <JANA/JLogger.h>
 #include <JANA/JException.h>
@@ -113,9 +115,13 @@ class JParameterManager : public JService {
     template<typename T, size_t arrSize>
     static inline void Parse(const std::string& value, std::array<T,arrSize>& out); 
 
+    /*
+    Template specialization done for double and float numbers in order to match the 
+    precision of the number with the string
+    */
     template<typename T>
     static std::string Stringify(const T& value);
-
+    
     template<typename T>
     static std::string Stringify(const std::vector<T>& values);
 
@@ -362,6 +368,31 @@ inline std::string JParameterManager::Stringify(const T& value) {
     return ss.str();
 }
 
+template <>
+inline std::string JParameterManager::Stringify(const float& value) {
+    std::stringstream ss;
+    // Use enough precision to make sure that the "float -> string -> float" roundtrip is exact. 
+    // The stringstream << operator is smart enough to give us a clean representation when one is available.
+    ss << std::setprecision(std::numeric_limits<float>::max_digits10) << value;
+    return ss.str();
+}
+template <>
+inline std::string JParameterManager::Stringify(const double& value) {
+    std::stringstream ss;
+    // Use enough precision to make sure that the "double -> string -> double" roundtrip is exact. 
+    // The stringstream << operator is smart enough to give us a clean representation when one is available.
+    ss << std::setprecision(std::numeric_limits<double>::max_digits10) << value;
+    return ss.str();
+}
+template <>
+inline std::string JParameterManager::Stringify(const long double& value) {
+    std::stringstream ss;
+    // Use enough precision to make sure that the "long double -> string -> long double" roundtrip is exact. 
+    // The stringstream << operator is smart enough to give us a clean representation when one is available.
+    ss << std::setprecision(std::numeric_limits<long double>::max_digits10) << value;
+    return ss.str();
+}
+
 // @brief Specialization for strings. The stream operator is not only redundant here, but it also splits the string (see Issue #191)
 template <>
 inline std::string JParameterManager::Stringify(const std::string& value) {
@@ -383,6 +414,7 @@ inline std::string JParameterManager::Stringify(const std::vector<T> &values) {
     return ss.str();
 }
 
+
 // @brief Template for generically stringifying an array
 template <typename T, size_t N>
 inline std::string JParameterManager::Stringify(const std::array<T,N> &values) {
@@ -398,20 +430,14 @@ inline std::string JParameterManager::Stringify(const std::array<T,N> &values) {
     return ss.str();
 }
 
+// @brief Equals() is called internally by SetDefaultParameter. This allows the user to define equivalence relations on custom data types so that
+// they can silence any spurious "loses equality with itself after stringification" warnings. Generally you should try specializing Stringify and Parse
+// first to normalize your data representation.
 template <typename T>
 inline bool JParameterManager::Equals(const T& lhs, const T& rhs) {
     return lhs == rhs;
 }
 
-template <>
-inline bool JParameterManager::Equals(const float& lhs, const float& rhs) {
-    return (std::abs(lhs-rhs) < std::abs(lhs)*std::numeric_limits<float>::epsilon());
-}
-
-template <>
-inline bool JParameterManager::Equals(const double& lhs, const double& rhs) {
-    return (std::abs(lhs-rhs) < std::abs(lhs)*std::numeric_limits<double>::epsilon());
-}
 
 #endif // _JParameterManager_h_
 

src/programs/tests/JParameterManagerTests.cc

@@ -317,6 +317,81 @@ TEST_CASE("JParameterManager_ArrayParams") {
     }
 }
 
+TEST_CASE("JParameterManagerFloatingPointRoundTrip") {
+    JParameterManager jpm;
+
+    SECTION("Integer") {
+        const std::string testValString = jpm.Stringify(123);
+        REQUIRE(testValString == "123");
+        int testValParsed;
+        jpm.Parse<int>(testValString, testValParsed);
+        REQUIRE(testValParsed == 123);
+    }
+
+    SECTION("Double requiring low precision") {
+        const double testVal = 123;
+        const std::string testValString = jpm.Stringify(testVal);
+        REQUIRE(testValString == "123");
+        double testValParsed;
+        jpm.Parse<double>(testValString, testValParsed);
+        REQUIRE(testValParsed == 123.0);
+    }
+
+    SECTION("Double requiring low precision, with extra zeros") {
+        const double testVal = 123.0;
+        const std::string testValString = jpm.Stringify(testVal);
+        REQUIRE(testValString == "123");
+        double testValParsed;
+        jpm.Parse<double>(testValString, testValParsed);
+        REQUIRE(testValParsed == 123.0);
+    }
+
+    SECTION("Double requiring high precision") {
+        const double testVal = 123.0001;
+        const std::string testValString = jpm.Stringify(testVal);
+        std::cout << "High-precision double stringified to " << testValString << std::endl;
+        // REQUIRE(testValString == "123.0001");
+        double testValParsed;
+        jpm.Parse<double>(testValString, testValParsed);
+        REQUIRE(testValParsed == 123.0001);
+    }
+
+    SECTION("Float requiring high precision") {
+        const float testVal = 123.0001f;
+        const std::string testValString = jpm.Stringify(testVal);
+        std::cout << "High-precision float stringified to " << testValString << std::endl;
+        // REQUIRE(testValString == "123.0001");
+        float testValParsed;
+        jpm.Parse<float>(testValString, testValParsed);
+        REQUIRE(testValParsed == 123.0001f);
+    }
+
+    SECTION("Float requiring low precision") {
+        const float testVal = 123.0f;
+        const std::string testValString = jpm.Stringify(testVal);
+        REQUIRE(testValString == "123");
+        float testValParsed;
+        jpm.Parse<float>(testValString, testValParsed);
+        REQUIRE(testValParsed == 123.0f);
+    }
+
+}
+
+
+TEST_CASE("JParameterManagerIssue233") {
+    JParameterManager jpm;
+    double x = 0.0;
+    jpm.SetDefaultParameter("x", x, "Description");
+    // This should NOT print out a warning about losing equality with itself after stringification
+
+    // We reproduce the logic inside SetDefaultParameter here so that CI can catch regressions
+    std::string x_stringified = JParameterManager::Stringify(x);
+    double x_roundtrip;
+    JParameterManager::Parse(x_stringified, x_roundtrip);
+    REQUIRE(JParameterManager::Equals(x_roundtrip, x));
+}
+
+
 TEST_CASE("JParameterManager_Issue217StringsWithWhitespace") {
     JParameterManager jpm;
     SECTION("Reading a array of strings") {