Merged in feature/testVariableIndex (pull request #405)

Feature/testVariableIndex

Approved-by: Chris Beall <chrisbeall@gmail.com>

gtsam/inference/VariableIndex.h

@@ -63,7 +63,7 @@ public:
   /// @name Standard Constructors
   /// @{
 
-  /** Default constructor, creates an empty VariableIndex */
+  /// Default constructor, creates an empty VariableIndex
   VariableIndex() : nFactors_(0), nEntries_(0) {}
 
   /**
@@ -77,19 +77,16 @@ public:
   /// @name Standard Interface
   /// @{
 
-  /**
+  /// The number of variable entries.  This is equal to the number of unique variable Keys.
-   * The number of variable entries.  This is one greater than the variable
-   * with the highest index.
-   */
   size_t size() const { return index_.size(); }
 
-  /** The number of factors in the original factor graph */
+  /// The number of factors in the original factor graph
   size_t nFactors() const { return nFactors_; }
 
-  /** The number of nonzero blocks, i.e. the number of variable-factor entries */
+  /// The number of nonzero blocks, i.e. the number of variable-factor entries
   size_t nEntries() const { return nEntries_; }
 
-  /** Access a list of factors by variable */
+  /// Access a list of factors by variable
   const Factors& operator[](Key variable) const {
     KeyMap::const_iterator item = index_.find(variable);
     if(item == index_.end())
@@ -102,10 +99,10 @@ public:
   /// @name Testable
   /// @{
 
-  /** Test for equality (for unit tests and debug assertions). */
+  /// Test for equality (for unit tests and debug assertions).
   bool equals(const VariableIndex& other, double tol=0.0) const;
 
-  /** Print the variable index (for unit tests and debugging). */
+  /// Print the variable index (for unit tests and debugging).
   void print(const std::string& str = "VariableIndex: ",
       const KeyFormatter& keyFormatter = DefaultKeyFormatter) const;
 
@@ -140,17 +137,17 @@ public:
   template<typename ITERATOR, class FG>
   void remove(ITERATOR firstFactor, ITERATOR lastFactor, const FG& factors);
 
-  /** Remove unused empty variables (in debug mode verifies they are empty). */
+  /// Remove unused empty variables (in debug mode verifies they are empty).
   template<typename ITERATOR>
   void removeUnusedVariables(ITERATOR firstKey, ITERATOR lastKey);
 
-  /** Iterator to the first variable entry */
+  /// Iterator to the first variable entry
   const_iterator begin() const { return index_.begin(); }
 
-  /** Iterator to the first variable entry */
+  /// Iterator to the first variable entry
   const_iterator end() const { return index_.end(); }
 
-  /** Find the iterator for the requested variable entry */
+  /// Find the iterator for the requested variable entry
   const_iterator find(Key key) const { return index_.find(key); }
 
 protected:

gtsam/symbolic/tests/testVariableIndex.cpp

@@ -11,60 +11,66 @@
 
 /**
  * @file    testVariableIndex.cpp
- * @brief   
+ * @brief   Unit tests for VariableIndex class
  * @author  Richard Roberts
- * @date Sep 26, 2010
+ * @date    Sep 26, 2010
  */
 
+#include <gtsam/inference/VariableIndex.h>
+#include <gtsam/symbolic/SymbolicFactorGraph.h>
+#include <gtsam/base/TestableAssertions.h>
+
+#include <CppUnitLite/TestHarness.h>
+
 #include <boost/assign/std/list.hpp>
 #include <boost/assign/list_of.hpp>
 using namespace boost::assign;
 
-#include <CppUnitLite/TestHarness.h>
-#include <gtsam/base/TestableAssertions.h>
-
-#include <gtsam/inference/VariableIndex.h>
-#include <gtsam/symbolic/SymbolicFactorGraph.h>
-
 using namespace std;
 using namespace gtsam;
 
 /* ************************************************************************* */
-TEST(VariableIndex, augment) {
+// 2 small symbolic graphs shared by all tests
 
-  SymbolicFactorGraph fg1, fg2;
+SymbolicFactorGraph testGraph1() {
+  SymbolicFactorGraph fg1;
   fg1.push_factor(0, 1);
   fg1.push_factor(0, 2);
   fg1.push_factor(5, 9);
   fg1.push_factor(2, 3);
+  return fg1;
+}
+
+SymbolicFactorGraph testGraph2() {
+  SymbolicFactorGraph fg2;
   fg2.push_factor(1, 3);
   fg2.push_factor(2, 4);
   fg2.push_factor(3, 5);
   fg2.push_factor(5, 6);
+  return fg2;
+}
 
-  SymbolicFactorGraph fgCombined; fgCombined.push_back(fg1); fgCombined.push_back(fg2);
+/* ************************************************************************* */
+TEST(VariableIndex, augment) {
+  auto fg1 = testGraph1(), fg2 = testGraph2();
+  SymbolicFactorGraph fgCombined;
+  fgCombined.push_back(fg1);
+  fgCombined.push_back(fg2);
 
   VariableIndex expected(fgCombined);
   VariableIndex actual(fg1);
   actual.augment(fg2);
 
-  LONGS_EQUAL(16, (long)actual.nEntries());
+  LONGS_EQUAL(8, actual.size());
-  LONGS_EQUAL(8, (long)actual.nFactors());
+  LONGS_EQUAL(16, actual.nEntries());
+  LONGS_EQUAL(8, actual.nFactors());
   EXPECT(assert_equal(expected, actual));
 }
 
 /* ************************************************************************* */
 TEST(VariableIndex, augment2) {
 
-  SymbolicFactorGraph fg1, fg2;
+  auto fg1 = testGraph1(), fg2 = testGraph2();
-  fg1.push_factor(0, 1);
-  fg1.push_factor(0, 2);
-  fg1.push_factor(5, 9);
-  fg1.push_factor(2, 3);
-  fg2.push_factor(1, 3);
-  fg2.push_factor(2, 4);
-  fg2.push_factor(3, 5);
-  fg2.push_factor(5, 6);
 
   SymbolicFactorGraph fgCombined;
   fgCombined.push_back(fg1);
@@ -77,23 +83,16 @@ TEST(VariableIndex, augment2) {
   VariableIndex actual(fg1);
   actual.augment(fg2, newIndices);
 
-  LONGS_EQUAL(16, (long) actual.nEntries());
+  LONGS_EQUAL(8, actual.size());
-  LONGS_EQUAL(9, (long) actual.nFactors());
+  LONGS_EQUAL(16, actual.nEntries());
+  LONGS_EQUAL(9, actual.nFactors());
   EXPECT(assert_equal(expected, actual));
 }
 
 /* ************************************************************************* */
 TEST(VariableIndex, remove) {
 
-  SymbolicFactorGraph fg1, fg2;
+  auto fg1 = testGraph1(), fg2 = testGraph2();
-  fg1.push_factor(0, 1);
-  fg1.push_factor(0, 2);
-  fg1.push_factor(5, 9);
-  fg1.push_factor(2, 3);
-  fg2.push_factor(1, 3);
-  fg2.push_factor(2, 4);
-  fg2.push_factor(3, 5);
-  fg2.push_factor(5, 6);
 
   SymbolicFactorGraph fgCombined; fgCombined.push_back(fg1); fgCombined.push_back(fg2);
 
@@ -118,15 +117,7 @@ TEST(VariableIndex, remove) {
 /* ************************************************************************* */
 TEST(VariableIndex, deep_copy) {
 
-  SymbolicFactorGraph fg1, fg2;
+  auto fg1 = testGraph1(), fg2 = testGraph2();
-  fg1.push_factor(0, 1);
-  fg1.push_factor(0, 2);
-  fg1.push_factor(5, 9);
-  fg1.push_factor(2, 3);
-  fg2.push_factor(1, 3);
-  fg2.push_factor(2, 4);
-  fg2.push_factor(3, 5);
-  fg2.push_factor(5, 6);
 
   // Create original graph and VariableIndex
   SymbolicFactorGraph fgOriginal; fgOriginal.push_back(fg1); fgOriginal.push_back(fg2);