Bug fix for Index::size() when points are removed.

mariusmuja · mariusmuja · commit 97598a64fb66 · 2012-12-22T03:49:20.000-08:00
Index::size() should not include the number of points removed from the
index.
diff --git a/src/cpp/flann/algorithms/nn_index.h b/src/cpp/flann/algorithms/nn_index.h
@@ -76,12 +76,13 @@ class NNIndex : public IndexBase
     typedef typename Distance::ElementType ElementType;
     typedef typename Distance::ResultType DistanceType;
 
-	NNIndex(Distance d) : distance_(d), last_id_(0), size_(0), veclen_(0), data_ptr_(NULL), removed_(false)
+	NNIndex(Distance d) : distance_(d), last_id_(0), size_(0), veclen_(0),
+			removed_(false), removed_count_(0), data_ptr_(NULL)
 	{
 	}
 
 	NNIndex(const IndexParams& params, Distance d) : distance_(d), last_id_(0), size_(0), veclen_(0),
-			index_params_(params), data_ptr_(NULL), removed_(false)
+			index_params_(params), removed_(false), removed_count_(0), data_ptr_(NULL)
 	{
 	}
 
@@ -91,11 +92,12 @@ class NNIndex : public IndexBase
 		size_(other.size_),
 		veclen_(other.veclen_),
 		index_params_(other.index_params_),
+		removed_(other.removed_),
 		removed_points_(other.removed_points_),
+		removed_count_(other.removed_count_),
 		ids_(other.ids_),
 		points_(other.points_),
-		data_ptr_(NULL),
-		removed_(other.removed_)
+		data_ptr_(NULL)
 	{
 		if (other.data_ptr_) {
 			data_ptr_ = new ElementType[size_*veclen_];
@@ -155,13 +157,14 @@ class NNIndex : public IndexBase
     		removed_points_.resize(size_);
     		removed_points_.reset();
     		last_id_ = size_;
+        	removed_ = true;
     	}
 
     	size_t point_index = id_to_index(id);
-    	if (point_index!=size_t(-1)) {
+    	if (point_index!=size_t(-1) && !removed_points_.test(point_index)) {
     		removed_points_.set(point_index);
+    		removed_count_++;
     	}
-    	removed_ = true;
     }
 
 
@@ -186,7 +189,7 @@ class NNIndex : public IndexBase
      */
     inline size_t size() const
     {
-        return size_;
+    	return size_ - removed_count_;
     }
 
     /**
@@ -500,7 +503,7 @@ class NNIndex : public IndexBase
     	}
     	else {
     		// explicitly indicated to use unbounded radius result set
-    		// or we know there'll be enough room for resulting indices and dists
+    		// and we know there'll be enough room for resulting indices and dists
     		if (params.max_neighbors<0 && (num_neighbors>=size())) {
 #pragma omp parallel num_threads(params.cores)
     			{
@@ -694,7 +697,7 @@ class NNIndex : public IndexBase
     	else {
     		// binary search
     		size_t start = 0;
-    		size_t end = size();
+    		size_t end = ids_.size();
 
     		while (start<end) {
     			size_t mid = (start+end)/2;
@@ -732,6 +735,7 @@ class NNIndex : public IndexBase
     	ids_.clear();
     	removed_points_.clear();
     	removed_ = false;
+    	removed_count_ = 0;
 
     	points_.resize(size_);
     	for (size_t i=0;i<size_;++i) {
@@ -775,6 +779,7 @@ class NNIndex : public IndexBase
     	ids_.resize(last_idx);
     	removed_points_.resize(last_idx);
     	size_ = last_idx;
+    	removed_count_ = 0;
     }
 
     void swap(NNIndex& other)
@@ -784,11 +789,12 @@ class NNIndex : public IndexBase
     	std::swap(size_, other.size_);
     	std::swap(veclen_, other.veclen_);
     	std::swap(index_params_, other.index_params_);
+    	std::swap(removed_, other.removed_);
     	std::swap(removed_points_, other.removed_points_);
+    	std::swap(removed_count_, other.removed_count_);
     	std::swap(ids_, other.ids_);
     	std::swap(points_, other.points_);
     	std::swap(data_ptr_, other.data_ptr_);
-    	std::swap(removed_, other.removed_);
     }
 
 protected:
@@ -821,11 +827,21 @@ class NNIndex : public IndexBase
      */
     IndexParams index_params_;
 
+    /**
+     * Flag indicating if at least a point was removed from the index
+     */
+    bool removed_;
+
     /**
      * Array used to mark points removed from the index
      */
     DynamicBitset removed_points_;
 
+    /**
+     * Number of points removed from the index
+     */
+    size_t removed_count_;
+
     /**
      * Array of point IDs, returned by nearest-neighbour operations
      */
@@ -841,10 +857,6 @@ class NNIndex : public IndexBase
      */
     ElementType* data_ptr_;
 
-    /**
-     * Flag indicating if at least a point was removed from the index
-     */
-    bool removed_;
 
 };
 
diff --git a/test/flann_hierarchical_test.cpp b/test/flann_hierarchical_test.cpp
@@ -142,15 +142,27 @@ TEST_F(HierarchicalIndex_Brief100K, TestRemove)
 			}
 		}
 	}
-	for (std::set<int>::iterator it = neighbors.begin(); it!=neighbors.end();++it) {
-		index.removePoint(*it);
-	}
 
-	// also remove 10% of the initial points
-	size_t offset = data.rows/10;
-	for (size_t i=0;i<offset;++i) {
-		index.removePoint(i);
-	}
+    flann::DynamicBitset removed(data.rows);
+
+    for (std::set<int>::iterator it = neighbors.begin(); it!=neighbors.end();++it) {
+    	index.removePoint(*it);
+    	removed.set(*it);
+    }
+
+    // also remove 10% of the initial points
+    size_t offset = data.rows/10;
+    for (size_t i=0;i<offset;++i) {
+    	index.removePoint(i);
+    	removed.set(i);
+    }
+
+    size_t new_size = 0;
+    for (size_t i=0;i<removed.size();++i) {
+    	if (!removed.test(i)) ++new_size;
+    }
+
+    EXPECT_EQ(index.size(), new_size);
 
 	start_timer("Searching KNN after remove points...");
 	index.knnSearch(query, indices, dists, k_nn_, flann::SearchParams(2000) );
@@ -166,6 +178,8 @@ TEST_F(HierarchicalIndex_Brief100K, TestRemove)
 	// rebuild index
 	index.buildIndex();
 
+	EXPECT_EQ(index.size(), new_size);
+
 	start_timer("Searching KNN after remove points and rebuild index...");
 	index.knnSearch(query, indices, dists, k_nn_, flann::SearchParams(2000) );
 	printf("done (%g seconds)\n", stop_timer());
diff --git a/test/flann_kdtree_single_test.cpp b/test/flann_kdtree_single_test.cpp
@@ -95,9 +95,20 @@ TEST_F(KDTreeSingle, TestRemove)
 			}
 		}
 	}
-	for (std::set<int>::iterator it = neighbors.begin(); it!=neighbors.end();++it) {
-		index.removePoint(*it);
-	}
+    flann::DynamicBitset removed(data.rows);
+
+    for (std::set<int>::iterator it = neighbors.begin(); it!=neighbors.end();++it) {
+    	index.removePoint(*it);
+    	removed.set(*it);
+    }
+
+    size_t new_size = 0;
+    for (size_t i=0;i<removed.size();++i) {
+    	if (!removed.test(i)) ++new_size;
+    }
+
+    EXPECT_EQ(index.size(), new_size);
+
 
 	start_timer("Searching KNN after remove points...");
 	index.knnSearch(query, indices, dists, knn, flann::SearchParams(-1) );
@@ -112,6 +123,8 @@ TEST_F(KDTreeSingle, TestRemove)
 	// rebuild index
 	index.buildIndex();
 
+    EXPECT_EQ(index.size(), new_size);
+
 	start_timer("Searching KNN after remove points and rebuild index...");
 	index.knnSearch(query, indices, dists, knn, flann::SearchParams(-1) );
 	printf("done (%g seconds)\n", stop_timer());
diff --git a/test/flann_kdtree_test.cpp b/test/flann_kdtree_test.cpp
@@ -110,16 +110,28 @@ TEST_F(KDTree_SIFT10K, TestRemove)
         	}
         }
     }
+
+    flann::DynamicBitset removed(data.rows);
+
     for (std::set<int>::iterator it = neighbors.begin(); it!=neighbors.end();++it) {
     	index.removePoint(*it);
+    	removed.set(*it);
     }
 
     // also remove 10% of the initial points
     size_t offset = data.rows/10;
     for (size_t i=0;i<offset;++i) {
     	index.removePoint(i);
+    	removed.set(i);
+    }
+
+    size_t new_size = 0;
+    for (size_t i=0;i<removed.size();++i) {
+    	if (!removed.test(i)) ++new_size;
     }
 
+    EXPECT_EQ(index.size(), new_size);
+
     start_timer("Searching KNN after remove points...");
     index.knnSearch(query, indices, dists, knn, flann::SearchParams(128) );
     printf("done (%g seconds)\n", stop_timer());
@@ -135,6 +147,8 @@ TEST_F(KDTree_SIFT10K, TestRemove)
 	// rebuild index
 	index.buildIndex();
 
+    EXPECT_EQ(index.size(), new_size);
+
 	start_timer("Searching KNN after remove points and rebuild index...");
 	index.knnSearch(query, indices, dists, knn, flann::SearchParams(128) );
 	printf("done (%g seconds)\n", stop_timer());
diff --git a/test/flann_kmeans_test.cpp b/test/flann_kmeans_test.cpp
@@ -98,16 +98,27 @@ TEST_F(KMeans_SIFT10K, TestRemove)
         	}
         }
     }
+    flann::DynamicBitset removed(data.rows);
+
     for (std::set<int>::iterator it = neighbors.begin(); it!=neighbors.end();++it) {
     	index.removePoint(*it);
+    	removed.set(*it);
     }
 
     // also remove 10% of the initial points
     size_t offset = data.rows/10;
     for (size_t i=0;i<offset;++i) {
     	index.removePoint(i);
+    	removed.set(i);
+    }
+
+    size_t new_size = 0;
+    for (size_t i=0;i<removed.size();++i) {
+    	if (!removed.test(i)) ++new_size;
     }
 
+    EXPECT_EQ(index.size(), new_size);
+
     start_timer("Searching KNN after remove points...");
     index.knnSearch(query, indices, dists, knn, flann::SearchParams(128) );
     printf("done (%g seconds)\n", stop_timer());
@@ -122,6 +133,8 @@ TEST_F(KMeans_SIFT10K, TestRemove)
 	// rebuild index
 	index.buildIndex();
 
+    EXPECT_EQ(index.size(), new_size);
+
 	start_timer("Searching KNN after remove points and rebuild index...");
 	index.knnSearch(query, indices, dists, knn, flann::SearchParams(128) );
 	printf("done (%g seconds)\n", stop_timer());
diff --git a/test/flann_linear_test.cpp b/test/flann_linear_test.cpp
@@ -55,16 +55,27 @@ TEST_F(Linear_SIFT10K, TestRemove)
         	}
         }
     }
+    flann::DynamicBitset removed(data.rows);
+
     for (std::set<int>::iterator it = neighbors.begin(); it!=neighbors.end();++it) {
     	index.removePoint(*it);
+    	removed.set(*it);
     }
 
     // also remove 10% of the initial points
     size_t offset = data.rows/10;
     for (size_t i=0;i<offset;++i) {
     	index.removePoint(i);
+    	removed.set(i);
+    }
+
+    size_t new_size = 0;
+    for (size_t i=0;i<removed.size();++i) {
+    	if (!removed.test(i)) ++new_size;
     }
 
+    EXPECT_EQ(index.size(), new_size);
+
     start_timer("Searching KNN after remove points...");
     index.knnSearch(query, indices, dists, 5, flann::SearchParams(128) );
     printf("done (%g seconds)\n", stop_timer());
@@ -79,6 +90,8 @@ TEST_F(Linear_SIFT10K, TestRemove)
 	// rebuild index
 	index.buildIndex();
 
+    EXPECT_EQ(index.size(), new_size);
+
 	start_timer("Searching KNN after remove points and rebuild index...");
 	index.knnSearch(query, indices, dists, knn, flann::SearchParams(128) );
 	printf("done (%g seconds)\n", stop_timer());
diff --git a/test/flann_lsh_test.cpp b/test/flann_lsh_test.cpp
@@ -145,15 +145,26 @@ TEST_F(LshIndex_Brief100K, TestRemove)
 			}
 		}
 	}
-	for (std::set<int>::iterator it = neighbors.begin(); it!=neighbors.end();++it) {
-		index.removePoint(*it);
-	}
+    flann::DynamicBitset removed(data.rows);
 
-	// also remove 10% of the initial points
-	size_t offset = data.rows/10;
-	for (size_t i=0;i<offset;++i) {
-		index.removePoint(i);
-	}
+    for (std::set<int>::iterator it = neighbors.begin(); it!=neighbors.end();++it) {
+    	index.removePoint(*it);
+    	removed.set(*it);
+    }
+
+    // also remove 10% of the initial points
+    size_t offset = data.rows/10;
+    for (size_t i=0;i<offset;++i) {
+    	index.removePoint(i);
+    	removed.set(i);
+    }
+
+    size_t new_size = 0;
+    for (size_t i=0;i<removed.size();++i) {
+    	if (!removed.test(i)) ++new_size;
+    }
+
+    EXPECT_EQ(index.size(), new_size);
 
 	start_timer("Searching KNN after remove points...");
 	index.knnSearch(query, indices, dists, k_nn_, flann::SearchParams(-1) );
@@ -169,6 +180,8 @@ TEST_F(LshIndex_Brief100K, TestRemove)
 	// rebuild index
 	index.buildIndex();
 
+	EXPECT_EQ(index.size(), new_size);
+
 	start_timer("Searching KNN after remove points and rebuild index...");
 	index.knnSearch(query, indices, dists, k_nn_, flann::SearchParams(128) );
 	printf("done (%g seconds)\n", stop_timer());
