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.

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

Initializer list and insert/update chaining #1359

Merged
merged 8 commits into from
Dec 31, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
7 changes: 7 additions & 0 deletions gtsam/discrete/Assignment.h
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,13 @@ class Assignment : public std::map<L, size_t> {
public:
using std::map<L, size_t>::operator=;

// Define the implicit default constructor.
Assignment() = default;

// Construct from initializer list.
Assignment(std::initializer_list<std::pair<const L, size_t>> init)
: std::map<L, size_t>{init} {}

void print(const std::string& s = "Assignment: ",
const std::function<std::string(L)>& labelFormatter =
&DefaultFormatter) const {
Expand Down
42 changes: 42 additions & 0 deletions gtsam/discrete/DiscreteValues.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

#include <gtsam/discrete/DiscreteValues.h>

#include <boost/range/combine.hpp>
#include <sstream>

using std::cout;
Expand All @@ -26,6 +27,7 @@ using std::stringstream;

namespace gtsam {

/* ************************************************************************ */
void DiscreteValues::print(const string& s,
const KeyFormatter& keyFormatter) const {
cout << s << ": ";
Expand All @@ -34,6 +36,44 @@ void DiscreteValues::print(const string& s,
cout << endl;
}

/* ************************************************************************ */
bool DiscreteValues::equals(const DiscreteValues& x, double tol) const {
if (this->size() != x.size()) return false;
for (const auto values : boost::combine(*this, x)) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Seems odd to use boost here if you want to eventually remove it.

if (values.get<0>() != values.get<1>()) return false;
}
return true;
}

/* ************************************************************************ */
DiscreteValues& DiscreteValues::insert(const DiscreteValues& values) {
dellaert marked this conversation as resolved.
Show resolved Hide resolved
for (const auto& kv : values) {
if (count(kv.first)) {
throw std::out_of_range(
"Requested to insert a DiscreteValues into another DiscreteValues "
"that already contains one or more of its keys.");
} else {
this->emplace(kv);
}
}
return *this;
}

/* ************************************************************************ */
DiscreteValues& DiscreteValues::update(const DiscreteValues& values) {
for (const auto& kv : values) {
if (!count(kv.first)) {
throw std::out_of_range(
"Requested to update a DiscreteValues with another DiscreteValues "
"that contains keys not present in the first.");
} else {
(*this)[kv.first] = kv.second;
}
}
return *this;
}

/* ************************************************************************ */
string DiscreteValues::Translate(const Names& names, Key key, size_t index) {
if (names.empty()) {
stringstream ss;
Expand All @@ -60,6 +100,7 @@ string DiscreteValues::markdown(const KeyFormatter& keyFormatter,
return ss.str();
}

/* ************************************************************************ */
string DiscreteValues::html(const KeyFormatter& keyFormatter,
const Names& names) const {
stringstream ss;
Expand All @@ -84,6 +125,7 @@ string DiscreteValues::html(const KeyFormatter& keyFormatter,
return ss.str();
}

/* ************************************************************************ */
string markdown(const DiscreteValues& values, const KeyFormatter& keyFormatter,
const DiscreteValues::Names& names) {
return values.markdown(keyFormatter, names);
Expand Down
48 changes: 39 additions & 9 deletions gtsam/discrete/DiscreteValues.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,21 +27,16 @@

namespace gtsam {

/** A map from keys to values
* TODO(dellaert): Do we need this? Should we just use gtsam::DiscreteValues?
* We just need another special DiscreteValue to represent labels,
* However, all other Lie's operators are undefined in this class.
* The good thing is we can have a Hybrid graph of discrete/continuous variables
* together..
* Another good thing is we don't need to have the special DiscreteKey which
* stores cardinality of a Discrete variable. It should be handled naturally in
* the new class DiscreteValue, as the variable's type (domain)
/**
* A map from keys to values
Copy link
Collaborator

Choose a reason for hiding this comment

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

A map from gtsam::Keys to a discrete value each.

* @ingroup discrete
*/
class GTSAM_EXPORT DiscreteValues : public Assignment<Key> {
public:
using Base = Assignment<Key>; // base class

/// @name Standard Constructors
/// @{
using Assignment::Assignment; // all constructors

// Define the implicit default constructor.
Expand All @@ -50,14 +45,49 @@ class GTSAM_EXPORT DiscreteValues : public Assignment<Key> {
// Construct from assignment.
explicit DiscreteValues(const Base& a) : Base(a) {}

// Construct from initializer list.
DiscreteValues(std::initializer_list<std::pair<const Key, size_t>> init)
: Assignment<Key>{init} {}

/// @}
/// @name Testable
/// @{

/// print required by Testable.
Copy link
Collaborator

Choose a reason for hiding this comment

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

/// GTSAM-style print method

void print(const std::string& s = "",
const KeyFormatter& keyFormatter = DefaultKeyFormatter) const;

/// equals required by Testable for unit testing.
Copy link
Collaborator

Choose a reason for hiding this comment

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

/// GTSAM-style test method

bool equals(const DiscreteValues& x, double tol = 1e-9) const;

/// @}
/// @name Standard Interface
/// @{

// insert in base class;
std::pair<iterator, bool> insert( const value_type& value ){
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can't we do using Base::insert here?

Copy link
Member Author

Choose a reason for hiding this comment

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

No. I tried.

return Base::insert(value);
}

/** Insert all values from \c values. Throws an invalid_argument exception if
* any keys to be inserted are already used. */
DiscreteValues& insert(const DiscreteValues& values);

/** For all key/value pairs in \c values, replace values with corresponding
* keys in this object with those in \c values. Throws std::out_of_range if
* any keys in \c values are not present in this object. */
DiscreteValues& update(const DiscreteValues& values);
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'll add insert_or_assign to this later since I need to do it for HybridValues too.


/**
* @brief Return a vector of DiscreteValues, one for each possible
* combination of values.
*/
static std::vector<DiscreteValues> CartesianProduct(
const DiscreteKeys& keys) {
return Base::CartesianProduct<DiscreteValues>(keys);
}

/// @}
/// @name Wrapper support
/// @{

Expand Down
3 changes: 0 additions & 3 deletions gtsam/discrete/tests/testAlgebraicDecisionTree.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,7 @@
#include <gtsam/discrete/DecisionTree-inl.h> // for convert only
#define DISABLE_TIMING

#include <boost/assign/std/map.hpp>
#include <boost/assign/std/vector.hpp>
#include <boost/tokenizer.hpp>
using namespace boost::assign;

#include <CppUnitLite/TestHarness.h>
#include <gtsam/base/timing.h>
Expand Down
3 changes: 0 additions & 3 deletions gtsam/discrete/tests/testDecisionTree.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,6 @@

#include <CppUnitLite/TestHarness.h>

#include <boost/assign/std/vector.hpp>
using namespace boost::assign;

using namespace std;
using namespace gtsam;

Expand Down
3 changes: 0 additions & 3 deletions gtsam/discrete/tests/testDecisionTreeFactor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,6 @@
#include <gtsam/discrete/DiscreteDistribution.h>
#include <gtsam/discrete/Signature.h>

#include <boost/assign/std/map.hpp>
using namespace boost::assign;

using namespace std;
using namespace gtsam;

Expand Down
13 changes: 4 additions & 9 deletions gtsam/discrete/tests/testDiscreteBayesNet.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,6 @@
#include <CppUnitLite/TestHarness.h>


#include <boost/assign/list_inserter.hpp>
#include <boost/assign/std/map.hpp>

using namespace boost::assign;

#include <iostream>
#include <string>
#include <vector>
Expand Down Expand Up @@ -115,11 +110,11 @@ TEST(DiscreteBayesNet, Asia) {
EXPECT(assert_equal(expected2, *chordal->back()));

// now sample from it
DiscreteValues expectedSample;
DiscreteValues expectedSample{{Asia.first, 1}, {Dyspnea.first, 1},
dellaert marked this conversation as resolved.
Show resolved Hide resolved
{XRay.first, 1}, {Tuberculosis.first, 0},
{Smoking.first, 1}, {Either.first, 1},
{LungCancer.first, 1}, {Bronchitis.first, 0}};
SETDEBUG("DiscreteConditional::sample", false);
insert(expectedSample)(Asia.first, 1)(Dyspnea.first, 1)(XRay.first, 1)(
Tuberculosis.first, 0)(Smoking.first, 1)(Either.first, 1)(
LungCancer.first, 1)(Bronchitis.first, 0);
auto actualSample = chordal2->sample();
EXPECT(assert_equal(expectedSample, actualSample));
}
Expand Down
3 changes: 0 additions & 3 deletions gtsam/discrete/tests/testDiscreteBayesTree.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,6 @@
#include <gtsam/discrete/DiscreteFactorGraph.h>
#include <gtsam/inference/BayesNet.h>

#include <boost/assign/std/vector.hpp>
using namespace boost::assign;

#include <CppUnitLite/TestHarness.h>

#include <iostream>
Expand Down
3 changes: 0 additions & 3 deletions gtsam/discrete/tests/testDiscreteConditional.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,7 @@
* @date Feb 14, 2011
*/

#include <boost/assign/std/map.hpp>
#include <boost/assign/std/vector.hpp>
#include <boost/make_shared.hpp>
using namespace boost::assign;

#include <CppUnitLite/TestHarness.h>
#include <gtsam/discrete/DecisionTreeFactor.h>
Expand Down
3 changes: 0 additions & 3 deletions gtsam/discrete/tests/testDiscreteFactor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,6 @@
#include <gtsam/base/serializationTestHelpers.h>
#include <gtsam/discrete/DiscreteFactor.h>

#include <boost/assign/std/map.hpp>
using namespace boost::assign;

using namespace std;
using namespace gtsam;
using namespace gtsam::serializationTestHelpers;
Expand Down
Loading