Merge branch 'develop' into discrete-elimination-refactor

CppUnitLite/Test.h

@@ -129,7 +129,7 @@ protected:
     result_.addFailure(Failure(name_, __FILE__, __LINE__, #expected, #actual)); }
 
 #define CHECK_EQUAL(expected,actual)\
-{ if (!((expected) == (actual))) { result_.addFailure(Failure(name_, __FILE__, __LINE__, std::to_string(expected), std::to_string(actual))); } }
+{ if (!((expected) == (actual))) { result_.addFailure(Failure(name_, __FILE__, __LINE__, std::to_string(expected), std::to_string(actual))); return; } }
 
 #define LONGS_EQUAL(expected,actual)\
 { long actualTemp = actual; \

gtsam/discrete/DecisionTreeFactor.cpp

@@ -195,7 +195,7 @@ namespace gtsam {
     // Construct unordered_map with values
     std::vector<std::pair<DiscreteValues, double>> result;
     for (const auto& assignment : assignments) {
-      result.emplace_back(assignment, operator()(assignment));
+      result.emplace_back(assignment, evaluate(assignment));
     }
     return result;
   }

gtsam/discrete/DecisionTreeFactor.h

@@ -137,10 +137,13 @@ namespace gtsam {
 
     /// Calculate probability for given values, 
     /// is just look up in AlgebraicDecisionTree.
-    double operator()(const Assignment<Key>& values) const override {
+    virtual double evaluate(const Assignment<Key>& values) const override {
       return ADT::operator()(values);
     }
 
+    /// Disambiguate to use DiscreteFactor version. Mainly for wrapper
+    using DiscreteFactor::operator();
+
     /// Calculate error for DiscreteValues `x`, is -log(probability).
     double error(const DiscreteValues& values) const override;
 

gtsam/discrete/DiscreteConditional.h

@@ -169,7 +169,7 @@ class GTSAM_EXPORT DiscreteConditional
   }
 
   using BaseFactor::error;       ///< DiscreteValues version
-  using BaseFactor::evaluate;  // DiscreteValues version
+  using BaseFactor::evaluate;    ///< DiscreteValues version
   using BaseFactor::operator();  ///< DiscreteValues version
 
   /**

gtsam/discrete/DiscreteFactor.h

@@ -107,12 +107,12 @@ class GTSAM_EXPORT DiscreteFactor : public Factor {
    * @param values Discrete assignment.
    * @return double
    */
-  double evaluate(const Assignment<Key>& values) const {
-    return operator()(values);
-  }
+  virtual double evaluate(const Assignment<Key>& values) const = 0;
 
   /// Find value for given assignment of values to variables
-  virtual double operator()(const Assignment<Key>& values) const = 0;
+  double operator()(const DiscreteValues& values) const {
+    return evaluate(values);
+  }
 
   /// Error is just -log(value)
   virtual double error(const DiscreteValues& values) const;

gtsam/discrete/TableFactor.cpp

@@ -133,7 +133,7 @@ bool TableFactor::equals(const DiscreteFactor& other, double tol) const {
 }
 
 /* ************************************************************************ */
-double TableFactor::operator()(const Assignment<Key>& values) const {
+double TableFactor::evaluate(const Assignment<Key>& values) const {
   // a b c d => D * (C * (B * (a) + b) + c) + d
   uint64_t idx = 0, card = 1;
   for (auto it = sorted_dkeys_.rbegin(); it != sorted_dkeys_.rend(); ++it) {

gtsam/discrete/TableFactor.h

@@ -152,7 +152,7 @@ class GTSAM_EXPORT TableFactor : public DiscreteFactor {
   // /// @{
 
   /// Evaluate probability distribution, is just look up in TableFactor.
-  double operator()(const Assignment<Key>& values) const override;
+  double evaluate(const Assignment<Key>& values) const override;
 
   /// Calculate error for DiscreteValues `x`, is -log(probability).
   double error(const DiscreteValues& values) const override;

gtsam_unstable/discrete/AllDiff.cpp

@@ -26,7 +26,7 @@ void AllDiff::print(const std::string& s, const KeyFormatter& formatter) const {
 }
 
 /* ************************************************************************* */
-double AllDiff::operator()(const Assignment<Key>& values) const {
+double AllDiff::evaluate(const Assignment<Key>& values) const {
   std::set<size_t> taken;  // record values taken by keys
   for (Key dkey : keys_) {
     size_t value = values.at(dkey);      // get the value for that key

gtsam_unstable/discrete/AllDiff.h

@@ -45,7 +45,7 @@ class GTSAM_UNSTABLE_EXPORT AllDiff : public Constraint {
   }
 
   /// Calculate value = expensive !
-  double operator()(const Assignment<Key>& values) const override;
+  double evaluate(const Assignment<Key>& values) const override;
 
   /// Convert into a decisiontree, can be *very* expensive !
   DecisionTreeFactor toDecisionTreeFactor() const override;

gtsam_unstable/discrete/BinaryAllDiff.h

@@ -47,7 +47,7 @@ class BinaryAllDiff : public Constraint {
   }
 
   /// Calculate value
-  double operator()(const Assignment<Key>& values) const override {
+  double evaluate(const Assignment<Key>& values) const override {
     return (double)(values.at(keys_[0]) != values.at(keys_[1]));
   }
 

gtsam_unstable/discrete/Domain.cpp

@@ -30,7 +30,7 @@ string Domain::base1Str() const {
 }
 
 /* ************************************************************************* */
-double Domain::operator()(const Assignment<Key>& values) const {
+double Domain::evaluate(const Assignment<Key>& values) const {
   return contains(values.at(key()));
 }
 

gtsam_unstable/discrete/Domain.h

@@ -82,7 +82,7 @@ class GTSAM_UNSTABLE_EXPORT Domain : public Constraint {
   bool contains(size_t value) const { return values_.count(value) > 0; }
 
   /// Calculate value
-  double operator()(const Assignment<Key>& values) const override;
+  double evaluate(const Assignment<Key>& values) const override;
 
   /// Convert into a decisiontree
   DecisionTreeFactor toDecisionTreeFactor() const override;

gtsam_unstable/discrete/SingleValue.cpp

@@ -22,7 +22,7 @@ void SingleValue::print(const string& s, const KeyFormatter& formatter) const {
 }
 
 /* ************************************************************************* */
-double SingleValue::operator()(const Assignment<Key>& values) const {
+double SingleValue::evaluate(const Assignment<Key>& values) const {
   return (double)(values.at(keys_[0]) == value_);
 }
 

gtsam_unstable/discrete/SingleValue.h

@@ -55,7 +55,7 @@ class GTSAM_UNSTABLE_EXPORT SingleValue : public Constraint {
   }
 
   /// Calculate value
-  double operator()(const Assignment<Key>& values) const override;
+  double evaluate(const Assignment<Key>& values) const override;
 
   /// Convert into a decisiontree
   DecisionTreeFactor toDecisionTreeFactor() const override;

python/gtsam/tests/test_DecisionTreeFactor.py

@@ -13,9 +13,10 @@ Author: Frank Dellaert
 
 import unittest
 
+from gtsam.utils.test_case import GtsamTestCase
+
 from gtsam import (DecisionTreeFactor, DiscreteDistribution, DiscreteValues,
                    Ordering)
-from gtsam.utils.test_case import GtsamTestCase
 
 
 class TestDecisionTreeFactor(GtsamTestCase):

python/gtsam/tests/test_DiscreteBayesTree.py

@@ -19,8 +19,8 @@ from gtsam.utils.test_case import GtsamTestCase
 
 import gtsam
 from gtsam import (DiscreteBayesNet, DiscreteBayesTreeClique,
-                   DiscreteConditional, DiscreteFactorGraph,
-                   DiscreteValues, Ordering)
+                   DiscreteConditional, DiscreteFactorGraph, DiscreteValues,
+                   Ordering)
 
 
 class TestDiscreteBayesNet(GtsamTestCase):

python/gtsam/tests/test_DiscreteConditional.py

@@ -13,9 +13,10 @@ Author: Varun Agrawal
 
 import unittest
 
-from gtsam import DecisionTreeFactor, DiscreteConditional, DiscreteKeys
 from gtsam.utils.test_case import GtsamTestCase
 
+from gtsam import DecisionTreeFactor, DiscreteConditional, DiscreteKeys
+
 # Some DiscreteKeys for binary variables:
 A = 0, 2
 B = 1, 2

python/gtsam/tests/test_DiscreteFactorGraph.py

@@ -14,9 +14,12 @@ Author: Frank Dellaert
 import unittest
 
 import numpy as np
-from gtsam import DecisionTreeFactor, DiscreteConditional, DiscreteFactorGraph, DiscreteKeys, DiscreteValues, Ordering, Symbol
 from gtsam.utils.test_case import GtsamTestCase
 
+from gtsam import (DecisionTreeFactor, DiscreteConditional,
+                   DiscreteFactorGraph, DiscreteKeys, DiscreteValues, Ordering,
+                   Symbol)
+
 OrderingType = Ordering.OrderingType