From 3e485873b1d0b5912e55648a29c382cb63f31edb Mon Sep 17 00:00:00 2001 From: Nathan Brei Date: Mon, 9 Oct 2023 13:26:27 -0400 Subject: [PATCH] Fixes to Stringify - The specializations of Stringify for float and double were not actually specializations. Rather they were a separate base template. - The specializations weren't getting called by the test cases - The specializations contained a bug where the stringstream was not cleared before new data was piped in - The cleanest exact representation of a floating point type T comes from setprecision(std::numerics_limits::max_digits10), not from setprecision(30). --- .../JANA/Services/JParameterManager.h | 54 +++++++-------- src/programs/tests/JParameterManagerTests.cc | 67 +++++++++++-------- 2 files changed, 65 insertions(+), 56 deletions(-) diff --git a/src/libraries/JANA/Services/JParameterManager.h b/src/libraries/JANA/Services/JParameterManager.h index 7047fbbd1..dc85c95e1 100644 --- a/src/libraries/JANA/Services/JParameterManager.h +++ b/src/libraries/JANA/Services/JParameterManager.h @@ -5,6 +5,7 @@ #ifndef _JParameterManager_h_ #define _JParameterManager_h_ +#include #include #include #include @@ -121,12 +122,6 @@ class JParameterManager : public JService { template static std::string Stringify(const T& value); - template - static std::string Stringify(const float value); - - template - static std::string Stringify(const double value); - template static std::string Stringify(const std::vector& values); @@ -369,35 +364,35 @@ inline void JParameterManager::Parse(const std::string& value, std::vector &v template inline std::string JParameterManager::Stringify(const T& value) { std::stringstream ss; - ss << std::setprecision(30) << value ; - return ss.str(); + ss << value; + return ss.str(); } -// Template specialization for Stringifying a float -template -inline std::string JParameterManager::Stringify(const float value) { +template <> +inline std::string JParameterManager::Stringify(const float& value) { std::stringstream ss; - ss << value; - float tempVal; - Parse(ss.str(), tempVal); - if (!(value == tempVal)){ - ss << std::setprecision(30) << tempVal; - } - return ss.str(); + // 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::max_digits10) << value; + return ss.str(); } - -// Template specialization for Stringifying a double -template -inline std::string JParameterManager::Stringify(const double value) { +template <> +inline std::string JParameterManager::Stringify(const double& value) { std::stringstream ss; - ss << value; - double tempVal; - Parse(ss.str(), tempVal); - if (!(value == tempVal)){ - ss << std::setprecision(30) << tempVal; - } - return ss.str(); + // 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::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::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) { @@ -419,6 +414,7 @@ inline std::string JParameterManager::Stringify(const std::vector &values) { return ss.str(); } + // @brief Template for generically stringifying an array template inline std::string JParameterManager::Stringify(const std::array &values) { diff --git a/src/programs/tests/JParameterManagerTests.cc b/src/programs/tests/JParameterManagerTests.cc index addb5027c..7eb475656 100644 --- a/src/programs/tests/JParameterManagerTests.cc +++ b/src/programs/tests/JParameterManagerTests.cc @@ -317,49 +317,62 @@ TEST_CASE("JParameterManager_ArrayParams") { } } -TEST_CASE("JParamter: Issue 233:") { +TEST_CASE("JParameter: Issue 233") { JParameterManager jpm; - SECTION("int Test case: ") { - const int testVal = 123; - int temp; - jpm.Parse(jpm.Stringify(testVal), temp); - REQUIRE(temp == testVal); + SECTION("Integer") { + const std::string testValString = jpm.Stringify(123); + REQUIRE(testValString == "123"); + int testValParsed; + jpm.Parse(testValString, testValParsed); + REQUIRE(testValParsed == 123); } - SECTION("double Test case: 1") { + SECTION("Double requiring low precision") { const double testVal = 123; - double temp; - jpm.Parse(jpm.Stringify(testVal), temp); - REQUIRE(temp == testVal); + const std::string testValString = jpm.Stringify(testVal); + REQUIRE(testValString == "123"); + double testValParsed; + jpm.Parse(testValString, testValParsed); + REQUIRE(testValParsed == 123.0); } - SECTION("double Test case: 2") { + SECTION("Double requiring low precision, with extra zeros") { const double testVal = 123.0; - double temp; - jpm.Parse(jpm.Stringify(testVal), temp); - REQUIRE(temp == testVal); + const std::string testValString = jpm.Stringify(testVal); + REQUIRE(testValString == "123"); + double testValParsed; + jpm.Parse(testValString, testValParsed); + REQUIRE(testValParsed == 123.0); } - SECTION("double Test case: 3") { - const double testVal = 123.00001; - double temp; - jpm.Parse(jpm.Stringify(testVal), temp); - REQUIRE(temp == testVal); + 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(testValString, testValParsed); + REQUIRE(testValParsed == 123.0001); } - SECTION("float Test case: 1") { + SECTION("Float requiring high precision") { const float testVal = 123.0001f; - float temp; - jpm.Parse(jpm.Stringify(testVal), temp); - REQUIRE(temp == testVal); + 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(testValString, testValParsed); + REQUIRE(testValParsed == 123.0001f); } - SECTION("float Test case: 2") { + SECTION("Float requiring low precision") { const float testVal = 123.0f; - float temp; - jpm.Parse(jpm.Stringify(testVal), temp); - REQUIRE(temp == testVal); + const std::string testValString = jpm.Stringify(testVal); + REQUIRE(testValString == "123"); + float testValParsed; + jpm.Parse(testValString, testValParsed); + REQUIRE(testValParsed == 123.0f); } }