Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature: Add support for std::array parameters (Addresses issue #236) #244

Merged
merged 9 commits into from
Oct 2, 2023
Merged

Feature: Add support for std::array parameters (Addresses issue #236) #244

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
476ef02
Adding array support in JParameterManager (90%) done
AyanRoy16 Sep 26, 2023
494f2d5
Minor test name change
AyanRoy16 Sep 26, 2023
d6cec80
Hopeful solution to template resolution conundrum
nathanwbrei Sep 27, 2023
aae225f
Demonstrate user override of parse(std::string, std::array<double,N>)
nathanwbrei Sep 27, 2023
909b04f
Array of dynamic size (test cases are failing)
AyanRoy16 Oct 2, 2023
b504793
Merge branch 'AyanRoy16-issue-236-Adding-array-in-Jparameter' of http…
AyanRoy16 Oct 2, 2023
c24da69
Changing the test cases to incorporate the parse method
AyanRoy16 Oct 2, 2023
60197c8
Fix linking problem and excise ParseVector, etc
nathanwbrei Oct 2, 2023
3de3221
Fixing the code for handling failed bool test cases
AyanRoy16 Oct 2, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
193 changes: 73 additions & 120 deletions src/libraries/JANA/Services/JParameterManager.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#include <string>
#include <algorithm>
#include <vector>
#include <array>
#include <map>
#include <cmath>

Expand Down Expand Up @@ -104,19 +105,26 @@ class JParameterManager : public JService {
void WriteConfigFile(std::string name);

template<typename T>
static T Parse(const std::string& value);
static inline void Parse(const std::string& value, T& out);

template<typename T>
static std::string Stringify(const T& value);
static inline void Parse(const std::string& value, std::vector<T>& out);

template<typename T, size_t arrSize>
static inline void Parse(const std::string& value, std::array<T,arrSize>& out);

template<typename T>
static bool Equals(const T& lhs, const T& rhs);
static std::string Stringify(const T& value);

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

template<typename T,size_t N>
static std::string Stringify(const std::array<T,N>& values);

template<typename T>
static std::string StringifyVector(const std::vector<T>& values);
static bool Equals(const T& lhs, const T& rhs);


static std::string ToLower(const std::string& name);

Expand Down Expand Up @@ -145,7 +153,7 @@ JParameter* JParameterManager::GetParameter(std::string name, T& val) {
if (result == m_parameters.end()) {
return nullptr;
}
val = Parse<T>(result->second->GetValue());
Parse(result->second->GetValue(),val);
result->second->SetIsUsed(true);
return result->second;
}
Expand All @@ -159,12 +167,14 @@ JParameter* JParameterManager::GetParameter(std::string name, T& val) {
///
template<typename T>
T JParameterManager::GetParameterValue(std::string name) {
T t;
auto result = m_parameters.find(ToLower(name));
if (result == m_parameters.end()) {
throw JException("Unknown parameter \"%s\"", name.c_str());
}
result->second->SetIsUsed(true);
return Parse<T>(result->second->GetValue());
Parse(result->second->GetValue(),t);
return t;
}


Expand Down Expand Up @@ -220,7 +230,7 @@ JParameter* JParameterManager::SetDefaultParameter(std::string name, T& val, std

std::lock_guard<std::mutex> lock(m_mutex);
JParameter* param = nullptr;

T t;
auto result = m_parameters.find(ToLower(name));
if (result != m_parameters.end()) {
// We already have a value stored for this parameter
Expand All @@ -236,7 +246,8 @@ JParameter* JParameterManager::SetDefaultParameter(std::string name, T& val, std
// We tried to set the same default parameter twice. This is fine; this happens all the time
// because we construct lots of copies of JFactories, each of which calls SetDefaultParameter on its own.
// However, we still want to warn the user if the same parameter was declared with different values.
if (!Equals(val, Parse<T>(param->GetDefault()))) {
Parse(param->GetDefault(),t);
if (!Equals(val, t)) {
LOG_WARN(m_logger) << "Parameter '" << name << "' has conflicting defaults: '"
<< Stringify(val) << "' vs '" << param->GetDefault() << "'"
<< LOG_END;
Expand All @@ -256,7 +267,9 @@ JParameter* JParameterManager::SetDefaultParameter(std::string name, T& val, std

// Test whether parameter is one-to-one with its string representation.
// If not, warn the user they may have a problem.
if (!Equals(val, Parse<T>(valstr))) {

Parse(valstr,t);
if (!Equals(val, t)) {
LOG_WARN(m_logger) << "Parameter '" << name << "' with value '" << valstr
<< "' loses equality with itself after stringification" << LOG_END;
}
Expand All @@ -265,7 +278,8 @@ JParameter* JParameterManager::SetDefaultParameter(std::string name, T& val, std

// Always put val through the stringification/parsing cycle to be consistent with
// values passed in from config file, accesses from other threads
val = Parse<T>(param->GetValue());
Parse(param->GetValue(),t);
val = t;
param->SetIsUsed(true);
return param;
}
Expand Down Expand Up @@ -293,89 +307,51 @@ inline T JParameterManager::RegisterParameter(std::string name, const T default_

/// @brief Logic for parsing different types in a generic way
template <typename T>
inline T JParameterManager::Parse(const std::string& value) {
void JParameterManager::Parse(const std::string& value, T& val) {
std::stringstream ss(value);
T val;
ss >> val;
return val;
}

/// @brief Specialization for handling strings that don't need parsing

/// @brief Specialization for string.
/// The stream operator is not only redundant here, but it also splits the string (see Issue #191)
template <>
inline std::string JParameterManager::Parse(const std::string& value) {
return std::string(value);
inline void JParameterManager::Parse(const std::string& value, std::string& out) {
out = value;
}


/// @brief Specialization for bool
template <>
inline bool JParameterManager::Parse(const std::string& value) {
if (value == "0") return false;
if (value == "1") return true;
if (value == "false") return false;
if (value == "true") return true;
if (value == "off") return false;
if (value == "on") return true;
throw JException("'%s' not parseable as bool", value.c_str());
inline void JParameterManager::Parse(const std::string& value, bool& val) {
if (value == "0" || value =="false" || value == "off") val = false;
else if (value == "1" || value == "true" || value == "on") val = true;
else throw JException("'%s' not parseable as bool", value.c_str());
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's probably fine here, but in general I'd advise against rearranging ugly logic unless it is truly necessary, particularly when parsing is involved. It's just so easy to introduce bugs or activate bugs that have been lying dormant. Particularly when our test cases are this light.



/// @brief Specialization for std::vector<std::string>
template<>
inline std::vector<std::string> JParameterManager::Parse(const std::string& value) {
std::stringstream ss(value);
std::vector<std::string> result;
// @brief Template to parse a string and return in an array
template<typename T, size_t N>
inline void JParameterManager::Parse(const std::string& value,std::array<T,N> &val) {
std::string s;
std::stringstream ss(value);
int indx = 0;
while (getline(ss, s, ',')) {
result.push_back(s);
}
return result;
T t;
Parse(s, t);
val[indx++]= t;
}
}

template <typename T>
inline std::vector<T> JParameterManager::ParseVector(const std::string &value) {
/// @brief Specialization for std::vector<std::string>
template<typename T>
inline void JParameterManager::Parse(const std::string& value, std::vector<T> &val) {
std::stringstream ss(value);
std::vector<T> result;
std::string s;
while (getline(ss, s, ',')) {
std::stringstream sss(s);
T t;
sss >> t;
result.push_back(t);
Parse(s, t);
val.push_back(t);
}
return result;
}

template <>
inline std::vector<int> JParameterManager::Parse(const std::string& value) {
return ParseVector<int>(value);
}
template <>
inline std::vector<long int> JParameterManager::Parse(const std::string& value) {
return ParseVector<long int>(value);
}
template <>
inline std::vector<long long int> JParameterManager::Parse(const std::string& value) {
return ParseVector<long long int>(value);
}
template <>
inline std::vector<unsigned int> JParameterManager::Parse(const std::string& value) {
return ParseVector<unsigned int>(value);
}
template <>
inline std::vector<unsigned long int> JParameterManager::Parse(const std::string& value) {
return ParseVector<unsigned long int>(value);
}
template <>
inline std::vector<unsigned long long int> JParameterManager::Parse(const std::string& value) {
return ParseVector<unsigned long long int>(value);
}
template <>
inline std::vector<float> JParameterManager::Parse(const std::string& value) {
return ParseVector<float>(value);
}
template <>
inline std::vector<double> JParameterManager::Parse(const std::string& value) {
return ParseVector<double>(value);
}

/// @brief Logic for stringifying different types in a generic way
Expand All @@ -386,8 +362,15 @@ inline std::string JParameterManager::Stringify(const T& 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) {
return value;
}


template <typename T>
inline std::string JParameterManager::StringifyVector(const std::vector<T> &values) {
inline std::string JParameterManager::Stringify(const std::vector<T> &values) {
std::stringstream ss;
size_t len = values.size();
for (size_t i = 0; i+1 < len; ++i) {
Expand All @@ -400,51 +383,19 @@ inline std::string JParameterManager::StringifyVector(const std::vector<T> &valu
return ss.str();
}

/// Specializations of Stringify

template<>
inline std::string JParameterManager::Stringify(const std::vector<std::string>& values) {
return StringifyVector(values);
}

template<>
inline std::string JParameterManager::Stringify(const std::vector<int>& values) {
return StringifyVector(values);
}

template<>
inline std::string JParameterManager::Stringify(const std::vector<long int>& values) {
return StringifyVector(values);
}

template<>
inline std::string JParameterManager::Stringify(const std::vector<long long int>& values) {
return StringifyVector(values);
}

template<>
inline std::string JParameterManager::Stringify(const std::vector<unsigned int>& values) {
return StringifyVector(values);
}

template<>
inline std::string JParameterManager::Stringify(const std::vector<unsigned long int>& values) {
return StringifyVector(values);
}

template<>
inline std::string JParameterManager::Stringify(const std::vector<unsigned long long int>& values) {
return StringifyVector(values);
}

template<>
inline std::string JParameterManager::Stringify(const std::vector<float>& values) {
return StringifyVector(values);
}

template<>
inline std::string JParameterManager::Stringify(const std::vector<double>& values) {
return StringifyVector(values);
// @brief Template for generically stringifying an array
template <typename T, size_t N>
inline std::string JParameterManager::Stringify(const std::array<T,N> &values) {
std::stringstream ss;
size_t len = values.size();
for (size_t i = 0; i+1 < N; ++i) {
ss << values[i];
ss << ",";
}
if (len != 0) {
ss << values[len-1];
}
return ss.str();
}

template <typename T>
Expand All @@ -464,3 +415,5 @@ inline bool JParameterManager::Equals(const double& lhs, const double& rhs) {

#endif // _JParameterManager_h_



10 changes: 5 additions & 5 deletions src/libraries/JANA/Utils/JInspector.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,12 +76,12 @@ inline std::string JParameterManager::Stringify(const JInspector::Format& value)
}

template <>
inline JInspector::Format JParameterManager::Parse(const std::string& value) {
inline void JParameterManager::Parse(const std::string& value, JInspector::Format& val) {
auto lowered = JParameterManager::ToLower(value);
if (lowered == "table") return JInspector::Format::Table;
if (lowered == "json") return JInspector::Format::Json;
if (lowered == "tsv") return JInspector::Format::Tsv;
else return JInspector::Format::Table;
if (lowered == "table") val = JInspector::Format::Table;
if (lowered == "json") val = JInspector::Format::Json;
if (lowered == "tsv") val = JInspector::Format::Tsv;
else val = JInspector::Format::Table;
}

#endif // _JIntrospection_h_
Loading