new names find/merge, made it clear DFSVector operations are expensive

gtsam/base/DSFVector.cpp

@@ -42,18 +42,18 @@ DSFBase::DSFBase(const boost::shared_ptr<V>& v_in) {
 }
 
 /* ************************************************************************* */
-size_t DSFBase::findSet(size_t key) const {
+size_t DSFBase::find(size_t key) const {
   // follow parent pointers until we reach set representative
   size_t parent = (*v_)[key];
   if (parent != key)
-    parent = findSet(parent); // recursive call
+    parent = find(parent); // recursive call
   (*v_)[key] = parent; // path compression
   return parent;
 }
 
 /* ************************************************************************* */
-void DSFBase::makeUnionInPlace(const size_t& i1, const size_t& i2) {
+void DSFBase::merge(const size_t& i1, const size_t& i2) {
-  (*v_)[findSet(i2)] = findSet(i1);
+  (*v_)[find(i2)] = find(i1);
 }
 
 /* ************************************************************************* */
@@ -80,7 +80,7 @@ DSFVector::DSFVector(const boost::shared_ptr<V>& v_in,
 bool DSFVector::isSingleton(const size_t& label) const {
   bool result = false;
   BOOST_FOREACH(size_t key,keys_) {
-    if (findSet(key) == label) {
+    if (find(key) == label) {
       if (!result) // find the first occurrence
         result = true;
       else
@@ -94,7 +94,7 @@ bool DSFVector::isSingleton(const size_t& label) const {
 std::set<size_t> DSFVector::set(const size_t& label) const {
   std::set < size_t > set;
   BOOST_FOREACH(size_t key,keys_)
-    if (findSet(key) == label)
+    if (find(key) == label)
       set.insert(key);
   return set;
 }
@@ -103,7 +103,7 @@ std::set<size_t> DSFVector::set(const size_t& label) const {
 std::map<size_t, std::set<size_t> > DSFVector::sets() const {
   std::map<size_t, std::set<size_t> > sets;
   BOOST_FOREACH(size_t key,keys_)
-    sets[findSet(key)].insert(key);
+    sets[find(key)].insert(key);
   return sets;
 }
 
@@ -111,7 +111,7 @@ std::map<size_t, std::set<size_t> > DSFVector::sets() const {
 std::map<size_t, std::vector<size_t> > DSFVector::arrays() const {
   std::map<size_t, std::vector<size_t> > arrays;
   BOOST_FOREACH(size_t key,keys_)
-    arrays[findSet(key)].push_back(key);
+    arrays[find(key)].push_back(key);
   return arrays;
 }
 

gtsam/base/DSFVector.h

@@ -29,6 +29,7 @@ namespace gtsam {
 /**
  * A fast implementation of disjoint set forests that uses vector as underly data structure.
  * This is the absolute minimal DSF data structure, and only allows size_t keys
+ * Uses rank compression but not union by rank :-(
  * @addtogroup base
  */
 class GTSAM_EXPORT DSFBase {
@@ -47,20 +48,26 @@ public:
   DSFBase(const boost::shared_ptr<V>& v_in);
 
   /// find the label of the set in which {key} lives
-  size_t findSet(size_t key) const;
+  size_t find(size_t key) const;
 
-  /// the in-place version of makeUnion
+  /// Merge two sets
-  void makeUnionInPlace(const size_t& i1, const size_t& i2);
+  void merge(const size_t& i1, const size_t& i2);
+
+  /// @deprecated old name
+  inline size_t findSet(size_t key) const {return find(key);}
+
+  /// @deprecated old name
+  inline void makeUnionInPlace(const size_t& i1, const size_t& i2) {return merge(i1,i2);}
 };
 
 /**
- * A fast implementation of disjoint set forests that uses vector as underly data structure.
+ * DSFVector additionaly keeps a vector of keys to support more expensive operations
  * @addtogroup base
  */
 class GTSAM_EXPORT DSFVector: public DSFBase {
 
 private:
-  std::vector<size_t> keys_;///< stores keys to support more expensive operations
+  std::vector<size_t> keys_; ///< stores keys to support more expensive operations
 
 public:
   /// constructor that allocate new memory, uses sequential keys 0...numNodes-1
@@ -72,6 +79,8 @@ public:
   /// constructor that uses the existing memory
   DSFVector(const boost::shared_ptr<V>& v_in, const std::vector<size_t>& keys);
 
+  // all operations below loop over all keys and hence are *at least* O(n)
+
   /// find whether there is one and only one occurrence for the given {label}
   bool isSingleton(const size_t& label) const;
 

gtsam/base/tests/testDSFVector.cpp

@@ -33,114 +33,114 @@ using namespace std;
 using namespace gtsam;
 
 /* ************************************************************************* */
-TEST(DSFVectorVector, findSet) {
+TEST(DSFVectorVector, find) {
-  DSFVector dsf(3);
+  DSFBase dsf(3);
-  EXPECT(dsf.findSet(0) != dsf.findSet(2));
+  EXPECT(dsf.find(0) != dsf.find(2));
 }
 
 /* ************************************************************************* */
-TEST(DSFVectorVector, makeUnionInPlace) {
+TEST(DSFVectorVector, merge) {
-  DSFVector dsf(3);
+  DSFBase dsf(3);
-  dsf.makeUnionInPlace(0,2);
+  dsf.merge(0,2);
-  EXPECT(dsf.findSet(0) == dsf.findSet(2));
+  EXPECT(dsf.find(0) == dsf.find(2));
 }
 
 /* ************************************************************************* */
-TEST(DSFVectorVector, makeUnionInPlace2) {
+TEST(DSFBase, makeUnion2) {
-  boost::shared_ptr<DSFVector::V> v = boost::make_shared<DSFVector::V>(5);
+  DSFBase dsf(3);
+  dsf.merge(2,0);
+  EXPECT(dsf.find(0) == dsf.find(2));
+}
+
+/* ************************************************************************* */
+TEST(DSFBase, makeUnion3) {
+  DSFBase dsf(3);
+  dsf.merge(0,1);
+  dsf.merge(1,2);
+  EXPECT(dsf.find(0) == dsf.find(2));
+}
+
+/* ************************************************************************* */
+TEST(DSFVector, merge2) {
+  boost::shared_ptr<DSFBase::V> v = boost::make_shared<DSFBase::V>(5);
   std::vector<size_t> keys; keys += 1, 3;
   DSFVector dsf(v, keys);
-  dsf.makeUnionInPlace(1,3);
+  dsf.merge(1,3);
-  EXPECT(dsf.findSet(1) == dsf.findSet(3));
+  EXPECT(dsf.find(1) == dsf.find(3));
-}
-
-/* ************************************************************************* */
-TEST(DSFVector, makeUnion2) {
-  DSFVector dsf(3);
-  dsf.makeUnionInPlace(2,0);
-  EXPECT(dsf.findSet(0) == dsf.findSet(2));
-}
-
-/* ************************************************************************* */
-TEST(DSFVector, makeUnion3) {
-  DSFVector dsf(3);
-  dsf.makeUnionInPlace(0,1);
-  dsf.makeUnionInPlace(1,2);
-  EXPECT(dsf.findSet(0) == dsf.findSet(2));
 }
 
 /* ************************************************************************* */
 TEST(DSFVector, sets) {
   DSFVector dsf(2);
-  dsf.makeUnionInPlace(0,1);
+  dsf.merge(0,1);
   map<size_t, set<size_t> > sets = dsf.sets();
   LONGS_EQUAL(1, sets.size());
 
   set<size_t> expected; expected += 0, 1;
-  EXPECT(expected == sets[dsf.findSet(0)]);
+  EXPECT(expected == sets[dsf.find(0)]);
 }
 
 /* ************************************************************************* */
 TEST(DSFVector, arrays) {
   DSFVector dsf(2);
-  dsf.makeUnionInPlace(0,1);
+  dsf.merge(0,1);
   map<size_t, vector<size_t> > arrays = dsf.arrays();
   LONGS_EQUAL(1, arrays.size());
 
   vector<size_t> expected; expected += 0, 1;
-  EXPECT(expected == arrays[dsf.findSet(0)]);
+  EXPECT(expected == arrays[dsf.find(0)]);
 }
 
 /* ************************************************************************* */
 TEST(DSFVector, sets2) {
   DSFVector dsf(3);
-  dsf.makeUnionInPlace(0,1);
+  dsf.merge(0,1);
-  dsf.makeUnionInPlace(1,2);
+  dsf.merge(1,2);
   map<size_t, set<size_t> > sets = dsf.sets();
   LONGS_EQUAL(1, sets.size());
 
   set<size_t> expected; expected += 0, 1, 2;
-  EXPECT(expected == sets[dsf.findSet(0)]);
+  EXPECT(expected == sets[dsf.find(0)]);
 }
 
 /* ************************************************************************* */
 TEST(DSFVector, arrays2) {
   DSFVector dsf(3);
-  dsf.makeUnionInPlace(0,1);
+  dsf.merge(0,1);
-  dsf.makeUnionInPlace(1,2);
+  dsf.merge(1,2);
   map<size_t, vector<size_t> > arrays = dsf.arrays();
   LONGS_EQUAL(1, arrays.size());
 
   vector<size_t> expected; expected += 0, 1, 2;
-  EXPECT(expected == arrays[dsf.findSet(0)]);
+  EXPECT(expected == arrays[dsf.find(0)]);
 }
 
 /* ************************************************************************* */
 TEST(DSFVector, sets3) {
   DSFVector dsf(3);
-  dsf.makeUnionInPlace(0,1);
+  dsf.merge(0,1);
   map<size_t, set<size_t> > sets = dsf.sets();
   LONGS_EQUAL(2, sets.size());
 
   set<size_t> expected; expected += 0, 1;
-  EXPECT(expected == sets[dsf.findSet(0)]);
+  EXPECT(expected == sets[dsf.find(0)]);
 }
 
 /* ************************************************************************* */
 TEST(DSFVector, arrays3) {
   DSFVector dsf(3);
-  dsf.makeUnionInPlace(0,1);
+  dsf.merge(0,1);
   map<size_t, vector<size_t> > arrays = dsf.arrays();
   LONGS_EQUAL(2, arrays.size());
 
   vector<size_t> expected; expected += 0, 1;
-  EXPECT(expected == arrays[dsf.findSet(0)]);
+  EXPECT(expected == arrays[dsf.find(0)]);
 }
 
 /* ************************************************************************* */
 TEST(DSFVector, set) {
   DSFVector dsf(3);
-  dsf.makeUnionInPlace(0,1);
+  dsf.merge(0,1);
   set<size_t> set = dsf.set(0);
   LONGS_EQUAL(2, set.size());
 
@@ -151,8 +151,8 @@ TEST(DSFVector, set) {
 /* ************************************************************************* */
 TEST(DSFVector, set2) {
   DSFVector dsf(3);
-  dsf.makeUnionInPlace(0,1);
+  dsf.merge(0,1);
-  dsf.makeUnionInPlace(1,2);
+  dsf.merge(1,2);
   set<size_t> set = dsf.set(0);
   LONGS_EQUAL(3, set.size());
 
@@ -163,7 +163,7 @@ TEST(DSFVector, set2) {
 /* ************************************************************************* */
 TEST(DSFVector, isSingleton) {
   DSFVector dsf(3);
-  dsf.makeUnionInPlace(0,1);
+  dsf.merge(0,1);
   EXPECT(!dsf.isSingleton(0));
   EXPECT(!dsf.isSingleton(1));
   EXPECT( dsf.isSingleton(2));
@@ -184,16 +184,16 @@ TEST(DSFVector, mergePairwiseMatches) {
   // Merge matches
   DSFVector dsf(keys);
   BOOST_FOREACH(const Match& m, matches)
-    dsf.makeUnionInPlace(m.first,m.second);
+    dsf.merge(m.first,m.second);
 
   // Each point is now associated with a set, represented by one of its members
   size_t rep1 = 1, rep2 = 4;
-  EXPECT_LONGS_EQUAL(rep1,dsf.findSet(1));
+  EXPECT_LONGS_EQUAL(rep1,dsf.find(1));
-  EXPECT_LONGS_EQUAL(rep1,dsf.findSet(2));
+  EXPECT_LONGS_EQUAL(rep1,dsf.find(2));
-  EXPECT_LONGS_EQUAL(rep1,dsf.findSet(3));
+  EXPECT_LONGS_EQUAL(rep1,dsf.find(3));
-  EXPECT_LONGS_EQUAL(rep2,dsf.findSet(4));
+  EXPECT_LONGS_EQUAL(rep2,dsf.find(4));
-  EXPECT_LONGS_EQUAL(rep2,dsf.findSet(5));
+  EXPECT_LONGS_EQUAL(rep2,dsf.find(5));
-  EXPECT_LONGS_EQUAL(rep2,dsf.findSet(6));
+  EXPECT_LONGS_EQUAL(rep2,dsf.find(6));
 
   // Check that we have two connected components, 1,2,3 and 4,5,6
   map<size_t, set<size_t> > sets = dsf.sets();