diff --git a/gtsam/discrete/Signature.cpp b/gtsam/discrete/Signature.cpp index 17f3b52d6..b666a2f4b 100644 --- a/gtsam/discrete/Signature.cpp +++ b/gtsam/discrete/Signature.cpp @@ -16,136 +16,129 @@ * @date Feb 27, 2011 */ -#include "gtsam/discrete/SignatureParser.h" #include "Signature.h" -#include #include +#include + +#include "gtsam/discrete/SignatureParser.h" namespace gtsam { - using namespace std; +using namespace std; - ostream& operator <<(ostream &os, const Signature::Row &row) { - os << row[0]; - for (size_t i = 1; i < row.size(); i++) - os << " " << row[i]; - return os; - } +ostream& operator<<(ostream& os, const Signature::Row& row) { + os << row[0]; + for (size_t i = 1; i < row.size(); i++) os << " " << row[i]; + return os; +} - ostream& operator <<(ostream &os, const Signature::Table &table) { - for (size_t i = 0; i < table.size(); i++) - os << table[i] << endl; - return os; - } +ostream& operator<<(ostream& os, const Signature::Table& table) { + for (size_t i = 0; i < table.size(); i++) os << table[i] << endl; + return os; +} - Signature::Signature(const DiscreteKey& key, const DiscreteKeys& parents, - const Table& table) - : key_(key), parents_(parents) { - operator=(table); - } +Signature::Signature(const DiscreteKey& key, const DiscreteKeys& parents, + const Table& table) + : key_(key), parents_(parents) { + operator=(table); +} - Signature::Signature(const DiscreteKey& key, const DiscreteKeys& parents, - const std::string& spec) - : key_(key), parents_(parents) { - operator=(spec); - } +Signature::Signature(const DiscreteKey& key, const DiscreteKeys& parents, + const std::string& spec) + : key_(key), parents_(parents) { + operator=(spec); +} - Signature::Signature(const DiscreteKey& key) : - key_(key) { - } +Signature::Signature(const DiscreteKey& key) : key_(key) {} - DiscreteKeys Signature::discreteKeys() const { - DiscreteKeys keys; - keys.push_back(key_); - for (const DiscreteKey& key : parents_) keys.push_back(key); - return keys; - } +DiscreteKeys Signature::discreteKeys() const { + DiscreteKeys keys; + keys.push_back(key_); + for (const DiscreteKey& key : parents_) keys.push_back(key); + return keys; +} - KeyVector Signature::indices() const { - KeyVector js; - js.push_back(key_.first); - for (const DiscreteKey& key : parents_) js.push_back(key.first); - return js; - } +KeyVector Signature::indices() const { + KeyVector js; + js.push_back(key_.first); + for (const DiscreteKey& key : parents_) js.push_back(key.first); + return js; +} - vector Signature::cpt() const { - vector cpt; - if (table_) { - const size_t nrStates = table_->at(0).size(); - for (size_t j = 0; j < nrStates; j++) { - for (const Row& row : *table_) { - assert(row.size() == nrStates); - cpt.push_back(row[j]); - } +vector Signature::cpt() const { + vector cpt; + if (table_) { + const size_t nrStates = table_->at(0).size(); + for (size_t j = 0; j < nrStates; j++) { + for (const Row& row : *table_) { + assert(row.size() == nrStates); + cpt.push_back(row[j]); } } - return cpt; } + return cpt; +} - Signature& Signature::operator,(const DiscreteKey& parent) { - parents_.push_back(parent); - return *this; +Signature &Signature::operator,(const DiscreteKey& parent) { + parents_.push_back(parent); + return *this; +} + +static void normalize(Signature::Row& row) { + double sum = 0; + for (size_t i = 0; i < row.size(); i++) sum += row[i]; + for (size_t i = 0; i < row.size(); i++) row[i] /= sum; +} + +Signature& Signature::operator=(const string& spec) { + spec_ = spec; + auto table = SignatureParser::Parse(spec); + if (table) { + for (Row& row : *table) normalize(row); + table_ = *table; } + return *this; +} - static void normalize(Signature::Row& row) { - double sum = 0; - for (size_t i = 0; i < row.size(); i++) - sum += row[i]; - for (size_t i = 0; i < row.size(); i++) - row[i] /= sum; +Signature& Signature::operator=(const Table& t) { + Table table = t; + for (Row& row : table) normalize(row); + table_ = table; + return *this; +} + +ostream& operator<<(ostream& os, const Signature& s) { + os << s.key_.first; + if (s.parents_.empty()) { + os << " % "; + } else { + os << " | " << s.parents_[0].first; + for (size_t i = 1; i < s.parents_.size(); i++) + os << " && " << s.parents_[i].first; + os << " = "; } + os << (s.spec_ ? *s.spec_ : "no spec") << endl; + if (s.table_) + os << (*s.table_); + else + os << "spec could not be parsed" << endl; + return os; +} - Signature& Signature::operator=(const string& spec) { - spec_ = spec; - Table table; - bool success = gtsam::SignatureParser::parse(spec, table); - if (success) { - for (Row& row : table) normalize(row); - table_ = table; - } - return *this; - } +Signature operator|(const DiscreteKey& key, const DiscreteKey& parent) { + Signature s(key); + return s, parent; +} - Signature& Signature::operator=(const Table& t) { - Table table = t; - for(Row& row: table) - normalize(row); - table_ = table; - return *this; - } +Signature operator%(const DiscreteKey& key, const string& parent) { + Signature s(key); + return s = parent; +} - ostream& operator <<(ostream &os, const Signature &s) { - os << s.key_.first; - if (s.parents_.empty()) { - os << " % "; - } else { - os << " | " << s.parents_[0].first; - for (size_t i = 1; i < s.parents_.size(); i++) - os << " && " << s.parents_[i].first; - os << " = "; - } - os << (s.spec_ ? *s.spec_ : "no spec") << endl; - if (s.table_) - os << (*s.table_); - else - os << "spec could not be parsed" << endl; - return os; - } +Signature operator%(const DiscreteKey& key, const Signature::Table& parent) { + Signature s(key); + return s = parent; +} - Signature operator|(const DiscreteKey& key, const DiscreteKey& parent) { - Signature s(key); - return s, parent; - } - - Signature operator%(const DiscreteKey& key, const string& parent) { - Signature s(key); - return s = parent; - } - - Signature operator%(const DiscreteKey& key, const Signature::Table& parent) { - Signature s(key); - return s = parent; - } - -} // namespace gtsam +} // namespace gtsam diff --git a/gtsam/discrete/SignatureParser.cpp b/gtsam/discrete/SignatureParser.cpp index 023fea5bf..ab259d0ea 100644 --- a/gtsam/discrete/SignatureParser.cpp +++ b/gtsam/discrete/SignatureParser.cpp @@ -2,42 +2,46 @@ #include #include +#include #include - namespace gtsam { -inline static std::vector ParseTrueRow() { return {0, 1}; } +using Row = std::vector; +using Table = std::vector; -inline static std::vector ParseFalseRow() { return {1, 0}; } +inline static Row ParseTrueRow() { return {0, 1}; } -inline static SignatureParser::Table ParseOr() { +inline static Row ParseFalseRow() { return {1, 0}; } + +inline static Table ParseOr() { return {ParseFalseRow(), ParseTrueRow(), ParseTrueRow(), ParseTrueRow()}; } -inline static SignatureParser::Table ParseAnd() { +inline static Table ParseAnd() { return {ParseFalseRow(), ParseFalseRow(), ParseFalseRow(), ParseTrueRow()}; } -bool static ParseConditional(const std::string& token, - std::vector& row) { +std::optional static ParseConditional(const std::string& token) { // Expect something like a/b/c std::istringstream iss2(token); + Row row; try { - // if the string has no / then return false - if (std::count(token.begin(), token.end(), '/') == 0) return false; + // if the string has no / then return std::nullopt + if (std::count(token.begin(), token.end(), '/') == 0) return std::nullopt; // split the word on the '/' character for (std::string s; std::getline(iss2, s, '/');) { // can throw exception row.push_back(std::stod(s)); } } catch (...) { - return false; + return std::nullopt; } - return true; + return row; } -void static ParseConditionalTable(const std::vector& tokens, - SignatureParser::Table& table) { +std::optional static ParseConditionalTable( + const std::vector& tokens) { + Table table; // loop over the words // for each word, split it into doubles using a stringstream for (const auto& word : tokens) { @@ -48,14 +52,15 @@ void static ParseConditionalTable(const std::vector& tokens, table.push_back(ParseTrueRow()); } else { // Expect something like a/b/c - std::vector row; - if (!ParseConditional(word, row)) { + if (auto row = ParseConditional(word)) { + table.push_back(*row); + } else { // stop parsing if we encounter an error - return; + return std::nullopt; } - table.push_back(row); } } + return table; } std::vector static Tokenize(const std::string& str) { @@ -67,15 +72,15 @@ std::vector static Tokenize(const std::string& str) { return tokens; } -bool SignatureParser::parse(const std::string& str, Table& table) { +std::optional
SignatureParser::Parse(const std::string& str) { // check if string is just whitespace if (std::all_of(str.begin(), str.end(), isspace)) { - return false; + return std::nullopt; } - // return false if the string is empty + // return std::nullopt if the string is empty if (str.empty()) { - return false; + return std::nullopt; } // tokenize the str on whitespace @@ -83,32 +88,30 @@ bool SignatureParser::parse(const std::string& str, Table& table) { // if the first token is "OR", return the OR table if (tokens[0] == "OR") { - // if there are more tokens, return false + // if there are more tokens, return std::nullopt if (tokens.size() > 1) { - return false; + return std::nullopt; } - table = ParseOr(); - return true; + return ParseOr(); } // if the first token is "AND", return the AND table if (tokens[0] == "AND") { - // if there are more tokens, return false + // if there are more tokens, return std::nullopt if (tokens.size() > 1) { - return false; + return std::nullopt; } - table = ParseAnd(); - return true; + return ParseAnd(); } // otherwise then parse the conditional table - ParseConditionalTable(tokens, table); - // return false if the table is empty - if (table.empty()) { - return false; + auto table = ParseConditionalTable(tokens); + // return std::nullopt if the table is empty + if (!table || table->empty()) { + return std::nullopt; } // the boost::phoenix parser did not return an error if we could not fully // parse a string it just returned whatever it could parse - return true; + return table; } } // namespace gtsam diff --git a/gtsam/discrete/SignatureParser.h b/gtsam/discrete/SignatureParser.h index 98904ec51..e6b402e44 100644 --- a/gtsam/discrete/SignatureParser.h +++ b/gtsam/discrete/SignatureParser.h @@ -17,7 +17,8 @@ */ #pragma once -#include + +#include #include #include @@ -38,18 +39,18 @@ namespace gtsam { * "1/2/3 2/3/4 1/2/3 2/3/4 1/2/3 2/3/4 1/2/3 2/3/4 1/2/3" : * {{1,2,3},{2,3,4},{1,2,3},{2,3,4},{1,2,3},{2,3,4},{1,2,3},{2,3,4},{1,2,3}} * - * If the string has un-parsable elements, should parse whatever it can: - * "1/2 sdf" : {{1,2}} + * If the string has un-parsable elements the parser will fail with nullopt: + * "1/2 sdf" : nullopt ! * - * It should return false if the string is empty: - * "": false + * It also fails if the string is empty: + * "": nullopt ! * - * We should return false if the rows are not of the same size. + * Also fails if the rows are not of the same size. */ -namespace SignatureParser { -typedef std::vector Row; -typedef std::vector Table; +struct SignatureParser { + using Row = std::vector; + using Table = std::vector; -bool parse(const std::string& str, Table& table); -}; // namespace SignatureParser + static std::optional
Parse(const std::string& str); +}; } // namespace gtsam diff --git a/gtsam/discrete/tests/testSignatureParser.cpp b/gtsam/discrete/tests/testSignatureParser.cpp index 99acd56be..5ae71442e 100644 --- a/gtsam/discrete/tests/testSignatureParser.cpp +++ b/gtsam/discrete/tests/testSignatureParser.cpp @@ -32,72 +32,62 @@ bool compareTables(const SignatureParser::Table& table1, /* ************************************************************************* */ // Simple test case TEST(SimpleParser, Simple) { - SignatureParser::Table table, expectedTable; - expectedTable = {{1, 1}, {2, 3}, {1, 4}}; - bool ret = SignatureParser::parse("1/1 2/3 1/4", table); - EXPECT(ret); + SignatureParser::Table expectedTable{{1, 1}, {2, 3}, {1, 4}}; + const auto table = SignatureParser::Parse("1/1 2/3 1/4"); + CHECK(table); // compare the tables - EXPECT(compareTables(table, expectedTable)); + EXPECT(compareTables(*table, expectedTable)); } /* ************************************************************************* */ // Test case with each row having 3 elements TEST(SimpleParser, ThreeElements) { - SignatureParser::Table table, expectedTable; - expectedTable = {{1, 1, 1}, {2, 3, 2}, {1, 4, 3}}; - bool ret = SignatureParser::parse("1/1/1 2/3/2 1/4/3", table); - EXPECT(ret); + SignatureParser::Table expectedTable{{1, 1, 1}, {2, 3, 2}, {1, 4, 3}}; + const auto table = SignatureParser::Parse("1/1/1 2/3/2 1/4/3"); + CHECK(table); // compare the tables - EXPECT(compareTables(table, expectedTable)); + EXPECT(compareTables(*table, expectedTable)); } /* ************************************************************************* */ // A test case to check if we can parse a signature with 'T' and 'F' TEST(SimpleParser, TAndF) { - SignatureParser::Table table, expectedTable; - expectedTable = {{1, 0}, {1, 0}, {1, 0}, {0, 1}}; - bool ret = SignatureParser::parse("F F F T", table); - EXPECT(ret); + SignatureParser::Table expectedTable{{1, 0}, {1, 0}, {1, 0}, {0, 1}}; + const auto table = SignatureParser::Parse("F F F T"); + CHECK(table); // compare the tables - EXPECT(compareTables(table, expectedTable)); + EXPECT(compareTables(*table, expectedTable)); } /* ************************************************************************* */ // A test to parse {F F F 1} TEST(SimpleParser, FFF1) { - SignatureParser::Table table, expectedTable; - expectedTable = {{1, 0}, {1, 0}, {1, 0}}; - // should ignore the last 1 - bool ret = SignatureParser::parse("F F F 1", table); - EXPECT(ret); + SignatureParser::Table expectedTable{{1, 0}, {1, 0}, {1, 0}}; + const auto table = SignatureParser::Parse("F F F"); + CHECK(table); // compare the tables - EXPECT(compareTables(table, expectedTable)); + EXPECT(compareTables(*table, expectedTable)); } /* ************************************************************************* */ // Expect false if the string is empty TEST(SimpleParser, emptyString) { - SignatureParser::Table table; - bool ret = SignatureParser::parse("", table); - EXPECT(!ret); + const auto table = SignatureParser::Parse(""); + EXPECT(!table); } /* ************************************************************************* */ // Expect false if gibberish TEST(SimpleParser, Gibberish) { - SignatureParser::Table table; - bool ret = SignatureParser::parse("sdf 22/3", table); - EXPECT(!ret); + const auto table = SignatureParser::Parse("sdf 22/3"); + EXPECT(!table); } -// If Gibberish is in the middle, it should still parse the rest +// If Gibberish is in the middle, it should not parse. TEST(SimpleParser, GibberishInMiddle) { - SignatureParser::Table table, expectedTable; - expectedTable = {{1, 1}, {2, 3}}; - bool ret = SignatureParser::parse("1/1 2/3 sdf 1/4", table); - EXPECT(ret); - // compare the tables - EXPECT(compareTables(table, expectedTable)); + SignatureParser::Table expectedTable{{1, 1}, {2, 3}}; + const auto table = SignatureParser::Parse("1/1 2/3 sdf 1/4"); + EXPECT(!table); } /* ************************************************************************* */