From 4ed447ca8e87e82f1df6d2b108a387b94e20839e Mon Sep 17 00:00:00 2001 From: Frank Dellaert Date: Fri, 25 May 2012 14:51:03 +0000 Subject: [PATCH] Fixed CSP, now a DiscreteFactorGraph again, uses dynamic_cast for Constraint-specific functionality :-( --- gtsam_unstable/discrete/AllDiff.cpp | 2 +- gtsam_unstable/discrete/BinaryAllDiff.h | 2 +- gtsam_unstable/discrete/CMakeLists.txt | 3 +- gtsam_unstable/discrete/CSP.cpp | 11 ++-- gtsam_unstable/discrete/CSP.h | 59 ++++++++------------ gtsam_unstable/discrete/SingleValue.cpp | 4 +- gtsam_unstable/discrete/tests/testCSP.cpp | 18 +++--- gtsam_unstable/discrete/tests/testSudoku.cpp | 10 ++-- 8 files changed, 52 insertions(+), 57 deletions(-) diff --git a/gtsam_unstable/discrete/AllDiff.cpp b/gtsam_unstable/discrete/AllDiff.cpp index 46efd4499..0cab961e1 100644 --- a/gtsam_unstable/discrete/AllDiff.cpp +++ b/gtsam_unstable/discrete/AllDiff.cpp @@ -21,7 +21,7 @@ namespace gtsam { /* ************************************************************************* */ void AllDiff::print(const std::string& s) const { - std::cout << s << ": AllDiff on "; + std::cout << s << "AllDiff on "; BOOST_FOREACH (Index dkey, keys_) std::cout << dkey << " "; std::cout << std::endl; diff --git a/gtsam_unstable/discrete/BinaryAllDiff.h b/gtsam_unstable/discrete/BinaryAllDiff.h index 04eeba953..46901fc7d 100644 --- a/gtsam_unstable/discrete/BinaryAllDiff.h +++ b/gtsam_unstable/discrete/BinaryAllDiff.h @@ -34,7 +34,7 @@ namespace gtsam { // print virtual void print(const std::string& s = "") const { - std::cout << s << ": BinaryAllDiff on " << keys_[0] << " and " << keys_[1] + std::cout << s << "BinaryAllDiff on " << keys_[0] << " and " << keys_[1] << std::endl; } diff --git a/gtsam_unstable/discrete/CMakeLists.txt b/gtsam_unstable/discrete/CMakeLists.txt index 3ba34d12b..707a67d84 100644 --- a/gtsam_unstable/discrete/CMakeLists.txt +++ b/gtsam_unstable/discrete/CMakeLists.txt @@ -18,7 +18,8 @@ set (discrete_full_libs # Exclude tests that don't work set (discrete_excluded_tests "${CMAKE_CURRENT_SOURCE_DIR}/tests/testScheduler.cpp" -"${CMAKE_CURRENT_SOURCE_DIR}/tests/testCSP.cpp") +#"${CMAKE_CURRENT_SOURCE_DIR}/tests/testCSP.cpp" +) # Add all tests diff --git a/gtsam_unstable/discrete/CSP.cpp b/gtsam_unstable/discrete/CSP.cpp index 4da2f440a..10c27d2bb 100644 --- a/gtsam_unstable/discrete/CSP.cpp +++ b/gtsam_unstable/discrete/CSP.cpp @@ -49,8 +49,9 @@ namespace gtsam { // if not already a singleton if (!domains[v].isSingleton()) { // get the constraint and call its ensureArcConsistency method - Constraint::shared_ptr factor = (*this)[f]; - changed[v] = factor->ensureArcConsistency(v,domains) || changed[v]; + Constraint::shared_ptr constraint = boost::dynamic_pointer_cast((*this)[f]); + if (!constraint) throw runtime_error("CSP:runArcConsistency: non-constraint factor"); + changed[v] = constraint->ensureArcConsistency(v,domains) || changed[v]; } } // f if (changed[v]) anyChange = true; @@ -84,8 +85,10 @@ namespace gtsam { // TODO: create a new ordering as we go, to ensure a connected graph // KeyOrdering ordering; // vector dkeys; - BOOST_FOREACH(const Constraint::shared_ptr& factor, factors_) { - Constraint::shared_ptr reduced = factor->partiallyApply(domains); + BOOST_FOREACH(const DiscreteFactor::shared_ptr& f, factors_) { + Constraint::shared_ptr constraint = boost::dynamic_pointer_cast(f); + if (!constraint) throw runtime_error("CSP:runArcConsistency: non-constraint factor"); + Constraint::shared_ptr reduced = constraint->partiallyApply(domains); if (print) reduced->print(); } #endif diff --git a/gtsam_unstable/discrete/CSP.h b/gtsam_unstable/discrete/CSP.h index e2e2a2251..ef4bbc17a 100644 --- a/gtsam_unstable/discrete/CSP.h +++ b/gtsam_unstable/discrete/CSP.h @@ -18,7 +18,7 @@ namespace gtsam { * A specialization of a DiscreteFactorGraph. * It knows about CSP-specific constraints and algorithms */ - class CSP: public FactorGraph { + class CSP: public DiscreteFactorGraph { public: /** A map from keys to values */ @@ -27,30 +27,10 @@ namespace gtsam { typedef boost::shared_ptr sharedValues; public: - /// Constructor - CSP() { - } - template - void add(const DiscreteKey& j, SOURCE table) { - DiscreteKeys keys; - keys.push_back(j); - push_back(boost::make_shared(keys, table)); - } - - template - void add(const DiscreteKey& j1, const DiscreteKey& j2, SOURCE table) { - DiscreteKeys keys; - keys.push_back(j1); - keys.push_back(j2); - push_back(boost::make_shared(keys, table)); - } - - /** add shared discreteFactor immediately from arguments */ - template - void add(const DiscreteKeys& keys, SOURCE table) { - push_back(boost::make_shared(keys, table)); - } +// /// Constructor +// CSP() { +// } /// Add a unary constraint, allowing only a single value void addSingleValue(const DiscreteKey& dkey, size_t value) { @@ -71,19 +51,28 @@ namespace gtsam { push_back(factor); } +// /** return product of all factors as a single factor */ +// DecisionTreeFactor product() const { +// DecisionTreeFactor result; +// BOOST_FOREACH(const sharedFactor& factor, *this) +// if (factor) result = (*factor) * result; +// return result; +// } + /// Find the best total assignment - can be expensive sharedValues optimalAssignment() const; - /* - * Perform loopy belief propagation - * True belief propagation would check for each value in domain - * whether any satisfying separator assignment can be found. - * This corresponds to hyper-arc consistency in CSP speak. - * This can be done by creating a mini-factor graph and search. - * For a nine-by-nine Sudoku, the search tree will be 8+6+6=20 levels deep. - * It will be very expensive to exclude values that way. - */ - // void applyBeliefPropagation(size_t nrIterations = 10) const; +// /* +// * Perform loopy belief propagation +// * True belief propagation would check for each value in domain +// * whether any satisfying separator assignment can be found. +// * This corresponds to hyper-arc consistency in CSP speak. +// * This can be done by creating a mini-factor graph and search. +// * For a nine-by-nine Sudoku, the search tree will be 8+6+6=20 levels deep. +// * It will be very expensive to exclude values that way. +// */ +// void applyBeliefPropagation(size_t nrIterations = 10) const; + /* * Apply arc-consistency ~ Approximate loopy belief propagation * We need to give the domains to a constraint, and it returns @@ -92,7 +81,7 @@ namespace gtsam { */ void runArcConsistency(size_t cardinality, size_t nrIterations = 10, bool print = false) const; - }; + }; // CSP } // gtsam diff --git a/gtsam_unstable/discrete/SingleValue.cpp b/gtsam_unstable/discrete/SingleValue.cpp index 855d62353..a626ecf13 100644 --- a/gtsam_unstable/discrete/SingleValue.cpp +++ b/gtsam_unstable/discrete/SingleValue.cpp @@ -17,8 +17,8 @@ namespace gtsam { /* ************************************************************************* */ void SingleValue::print(const string& s) const { - cout << s << ": SingleValue on " << keys_[0] << " (j=" << keys_[0] - << ") with value " << value_ << endl; + cout << s << "SingleValue on " << "j=" << keys_[0] + << " with value " << value_ << endl; } /* ************************************************************************* */ diff --git a/gtsam_unstable/discrete/tests/testCSP.cpp b/gtsam_unstable/discrete/tests/testCSP.cpp index 4d04afb92..c92cbe1f9 100644 --- a/gtsam_unstable/discrete/tests/testCSP.cpp +++ b/gtsam_unstable/discrete/tests/testCSP.cpp @@ -54,17 +54,17 @@ TEST_UNSAFE( CSP, allInOne) invalid[ID.first] = 0; invalid[UT.first] = 0; invalid[AZ.first] = 0; - EXPECT_DOUBLES_EQUAL(0, csp(invalid), 1e-9); // FIXME: fails due to lack of operator() interface + EXPECT_DOUBLES_EQUAL(0, csp(invalid), 1e-9); // Check a valid combination DiscreteFactor::Values valid; valid[ID.first] = 0; valid[UT.first] = 1; valid[AZ.first] = 0; - EXPECT_DOUBLES_EQUAL(1, csp(valid), 1e-9); // FIXME: fails due to lack of operator() interface + EXPECT_DOUBLES_EQUAL(1, csp(valid), 1e-9); // Just for fun, create the product and check it - DecisionTreeFactor product = csp.product(); // FIXME: fails due to lack of product() + DecisionTreeFactor product = csp.product(); // product.dot("product"); DecisionTreeFactor expectedProduct(ID & AZ & UT, "0 1 0 0 0 0 1 0"); EXPECT(assert_equal(expectedProduct,product)); @@ -74,7 +74,7 @@ TEST_UNSAFE( CSP, allInOne) CSP::Values expected; insert(expected)(ID.first, 1)(UT.first, 0)(AZ.first, 1); EXPECT(assert_equal(expected,*mpe)); - EXPECT_DOUBLES_EQUAL(1, csp(*mpe), 1e-9); // FIXME: fails due to lack of operator() interface + EXPECT_DOUBLES_EQUAL(1, csp(*mpe), 1e-9); } /* ************************************************************************* */ @@ -122,7 +122,7 @@ TEST_UNSAFE( CSP, WesternUS) (MT.first,1)(WY.first,0)(NM.first,3)(CO.first,2) (ID.first,2)(UT.first,1)(AZ.first,0); EXPECT(assert_equal(expected,*mpe)); - EXPECT_DOUBLES_EQUAL(1, csp(*mpe), 1e-9); // FIXME: fails due to lack of operator() interface + EXPECT_DOUBLES_EQUAL(1, csp(*mpe), 1e-9); // Write out the dual graph for hmetis #ifdef DUAL @@ -146,7 +146,7 @@ TEST_UNSAFE( CSP, AllDiff) dkeys += ID,UT,AZ; csp.addAllDiff(dkeys); csp.addSingleValue(AZ,2); - //GTSAM_PRINT(csp); +// GTSAM_PRINT(csp); // Check construction and conversion SingleValue s(AZ,2); @@ -167,21 +167,21 @@ TEST_UNSAFE( CSP, AllDiff) invalid[ID.first] = 0; invalid[UT.first] = 1; invalid[AZ.first] = 0; - EXPECT_DOUBLES_EQUAL(0, csp(invalid), 1e-9); // FIXME: fails due to lack of operator() interface + EXPECT_DOUBLES_EQUAL(0, csp(invalid), 1e-9); // Check a valid combination DiscreteFactor::Values valid; valid[ID.first] = 0; valid[UT.first] = 1; valid[AZ.first] = 2; - EXPECT_DOUBLES_EQUAL(1, csp(valid), 1e-9); // FIXME: fails due to lack of operator() interface + EXPECT_DOUBLES_EQUAL(1, csp(valid), 1e-9); // Solve CSP::sharedValues mpe = csp.optimalAssignment(); CSP::Values expected; insert(expected)(ID.first, 1)(UT.first, 0)(AZ.first, 2); EXPECT(assert_equal(expected,*mpe)); - EXPECT_DOUBLES_EQUAL(1, csp(*mpe), 1e-9); // FIXME: fails due to lack of operator() interface + EXPECT_DOUBLES_EQUAL(1, csp(*mpe), 1e-9); // Arc-consistency vector domains; diff --git a/gtsam_unstable/discrete/tests/testSudoku.cpp b/gtsam_unstable/discrete/tests/testSudoku.cpp index 1bbac4777..583364ade 100644 --- a/gtsam_unstable/discrete/tests/testSudoku.cpp +++ b/gtsam_unstable/discrete/tests/testSudoku.cpp @@ -14,6 +14,8 @@ using namespace std; using namespace gtsam; +#define PRINT false + class Sudoku: public CSP { /// sudoku size @@ -119,7 +121,7 @@ TEST_UNSAFE( Sudoku, small) 0,1, 0,0); // Do BP - csp.runArcConsistency(4); + csp.runArcConsistency(4,10,PRINT); // optimize and check CSP::sharedValues solution = csp.optimalAssignment(); @@ -150,7 +152,7 @@ TEST_UNSAFE( Sudoku, easy) 5,0,0, 0,3,0, 7,0,0); // Do BP - sudoku.runArcConsistency(4); + sudoku.runArcConsistency(4,10,PRINT); // sudoku.printSolution(); // don't do it } @@ -172,7 +174,7 @@ TEST_UNSAFE( Sudoku, extreme) 0,0,0, 2,7,5, 9,0,0); // Do BP - sudoku.runArcConsistency(9,10,false); + sudoku.runArcConsistency(9,10,PRINT); #ifdef METIS VariableIndex index(sudoku); @@ -201,7 +203,7 @@ TEST_UNSAFE( Sudoku, AJC_3star_Feb8_2012) 0,0,0, 1,0,0, 0,3,7); // Do BP - sudoku.runArcConsistency(9,10,true); + sudoku.runArcConsistency(9,10,PRINT); //sudoku.printSolution(); // don't do it }