Skip to content

Commit

Permalink
Fixes to Stringify
Browse files Browse the repository at this point in the history
- 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<T>::max_digits10), not from setprecision(30).
  • Loading branch information
nathanwbrei committed Oct 9, 2023
1 parent abe51bb commit 3e48587
Show file tree
Hide file tree
Showing 2 changed files with 65 additions and 56 deletions.
54 changes: 25 additions & 29 deletions src/libraries/JANA/Services/JParameterManager.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#ifndef _JParameterManager_h_
#define _JParameterManager_h_

#include <limits>
#include <string>
#include <algorithm>
#include <vector>
Expand Down Expand Up @@ -121,12 +122,6 @@ class JParameterManager : public JService {
template<typename T>
static std::string Stringify(const T& value);

template<typename T>
static std::string Stringify(const float value);

template<typename T>
static std::string Stringify(const double value);

template<typename T>
static std::string Stringify(const std::vector<T>& values);

Expand Down Expand Up @@ -369,35 +364,35 @@ inline void JParameterManager::Parse(const std::string& value, std::vector<T> &v
template <typename T>
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 <typename T>
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<float>::max_digits10) << value;
return ss.str();
}

// Template specialization for Stringifying a double
template <typename T>
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<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) {
Expand All @@ -419,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) {
Expand Down
67 changes: 40 additions & 27 deletions src/programs/tests/JParameterManagerTests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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<int>(jpm.Stringify(testVal), temp);
REQUIRE(temp == testVal);
SECTION("Integer") {
const std::string testValString = jpm.Stringify(123);
REQUIRE(testValString == "123");
int testValParsed;
jpm.Parse<int>(testValString, testValParsed);
REQUIRE(testValParsed == 123);
}

SECTION("double Test case: 1") {
SECTION("Double requiring low precision") {
const double testVal = 123;
double temp;
jpm.Parse<double>(jpm.Stringify(testVal), temp);
REQUIRE(temp == testVal);
const std::string testValString = jpm.Stringify(testVal);
REQUIRE(testValString == "123");
double testValParsed;
jpm.Parse<double>(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<double>(jpm.Stringify(testVal), temp);
REQUIRE(temp == testVal);
const std::string testValString = jpm.Stringify(testVal);
REQUIRE(testValString == "123");
double testValParsed;
jpm.Parse<double>(testValString, testValParsed);
REQUIRE(testValParsed == 123.0);
}

SECTION("double Test case: 3") {
const double testVal = 123.00001;
double temp;
jpm.Parse<double>(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<double>(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<float>(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<float>(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<float>(jpm.Stringify(testVal), temp);
REQUIRE(temp == testVal);
const std::string testValString = jpm.Stringify(testVal);
REQUIRE(testValString == "123");
float testValParsed;
jpm.Parse<float>(testValString, testValParsed);
REQUIRE(testValParsed == 123.0f);
}

}
Expand Down

0 comments on commit 3e48587

Please sign in to comment.