From b70e42cd389dda052ebd72badb6703a9f7cef35b Mon Sep 17 00:00:00 2001 From: Omar Shrit Date: Sun, 3 Dec 2023 15:42:27 +0100 Subject: [PATCH 1/5] Change the template type from arma::mat to MatType in NS Signed-off-by: Omar Shrit --- .../neighbor_search/neighbor_search.hpp | 10 ++++----- .../neighbor_search/neighbor_search_impl.hpp | 22 +++++++++---------- 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/src/mlpack/methods/neighbor_search/neighbor_search.hpp b/src/mlpack/methods/neighbor_search/neighbor_search.hpp index 2e2b9934d8..ad0f1c6d0f 100644 --- a/src/mlpack/methods/neighbor_search/neighbor_search.hpp +++ b/src/mlpack/methods/neighbor_search/neighbor_search.hpp @@ -223,7 +223,7 @@ class NeighborSearch void Search(const MatType& querySet, const size_t k, arma::Mat& neighbors, - arma::mat& distances); + MatType& distances); /** * Given a pre-built query tree, search for the nearest neighbors of each @@ -248,7 +248,7 @@ class NeighborSearch void Search(Tree& queryTree, const size_t k, arma::Mat& neighbors, - arma::mat& distances, + MatType& distances, bool sameSet = false); /** @@ -267,7 +267,7 @@ class NeighborSearch */ void Search(const size_t k, arma::Mat& neighbors, - arma::mat& distances); + MatType& distances); /** * Calculate the average relative error (effective error) between the @@ -284,8 +284,8 @@ class NeighborSearch * query point. * @return Average relative error. */ - static double EffectiveError(arma::mat& foundDistances, - arma::mat& realDistances); + static double EffectiveError(MatType& foundDistances, + MatType& realDistances); /** * Calculate the recall (% of neighbors found) given the list of found diff --git a/src/mlpack/methods/neighbor_search/neighbor_search_impl.hpp b/src/mlpack/methods/neighbor_search/neighbor_search_impl.hpp index 13e42232f2..850bfd23dd 100644 --- a/src/mlpack/methods/neighbor_search/neighbor_search_impl.hpp +++ b/src/mlpack/methods/neighbor_search/neighbor_search_impl.hpp @@ -104,7 +104,7 @@ SingleTreeTraversalType>::NeighborSearch(const NeighborSearchMode mode, // Build the tree on the empty dataset, if necessary. if (mode != NAIVE_MODE) { - referenceTree = BuildTree(std::move(arma::mat()), + referenceTree = BuildTree(std::move(MatType()), oldFromNewReferences); referenceSet = &referenceTree->Dataset(); } @@ -255,7 +255,7 @@ NeighborSearch(std::move(arma::mat()), + other.referenceTree = BuildTree(std::move(MatType()), other.oldFromNewReferences); other.referenceSet = &other.referenceTree->Dataset(); other.searchMode = DUAL_TREE_MODE, @@ -365,7 +365,7 @@ DualTreeTraversalType, SingleTreeTraversalType>::Search( const MatType& querySet, const size_t k, arma::Mat& neighbors, - arma::mat& distances) + MatType& distances) { if (k > referenceSet->n_cols) { @@ -386,14 +386,14 @@ DualTreeTraversalType, SingleTreeTraversalType>::Search( // To avoid an extra copy, we will store the neighbors and distances in a // separate matrix. arma::Mat* neighborPtr = &neighbors; - arma::mat* distancePtr = &distances; + MatType* distancePtr = &distances; // Mapping is only necessary if the tree rearranges points. if (TreeTraits::RearrangesDataset) { if (searchMode == DUAL_TREE_MODE) { - distancePtr = new arma::mat; // Query indices need to be mapped. + distancePtr = new MatType; // Query indices need to be mapped. neighborPtr = new arma::Mat; } else if (!oldFromNewReferences.empty()) @@ -570,7 +570,7 @@ DualTreeTraversalType, SingleTreeTraversalType>::Search( Tree& queryTree, const size_t k, arma::Mat& neighbors, - arma::mat& distances, + MatType& distances, bool sameSet) { if (k > referenceSet->n_cols) @@ -648,7 +648,7 @@ void NeighborSearch::Search( const size_t k, arma::Mat& neighbors, - arma::mat& distances) + MatType& distances) { if (k > referenceSet->n_cols) { @@ -670,12 +670,12 @@ DualTreeTraversalType, SingleTreeTraversalType>::Search( scores = 0; arma::Mat* neighborPtr = &neighbors; - arma::mat* distancePtr = &distances; + MatType* distancePtr = &distances; if (!oldFromNewReferences.empty() && TreeTraits::RearrangesDataset) { // We will always need to rearrange in this case. - distancePtr = new arma::mat; + distancePtr = new MatType; neighborPtr = new arma::Mat; } @@ -825,8 +825,8 @@ template class SingleTreeTraversalType> double NeighborSearch::EffectiveError( - arma::mat& foundDistances, - arma::mat& realDistances) + MatType& foundDistances, + MatType& realDistances) { if (foundDistances.n_rows != realDistances.n_rows || foundDistances.n_cols != realDistances.n_cols) From a3dcfc70ab353ae2fc5d6b83554e44da9fcca31a Mon Sep 17 00:00:00 2001 From: Omar Shrit Date: Sun, 3 Dec 2023 21:10:16 +0100 Subject: [PATCH 2/5] Let us hope that this will pass for sparse matrices Signed-off-by: Omar Shrit --- .../methods/neighbor_search/neighbor_search.hpp | 12 +++++++----- .../neighbor_search/neighbor_search_impl.hpp | 16 ++++++++-------- 2 files changed, 15 insertions(+), 13 deletions(-) diff --git a/src/mlpack/methods/neighbor_search/neighbor_search.hpp b/src/mlpack/methods/neighbor_search/neighbor_search.hpp index ad0f1c6d0f..558a4a300f 100644 --- a/src/mlpack/methods/neighbor_search/neighbor_search.hpp +++ b/src/mlpack/methods/neighbor_search/neighbor_search.hpp @@ -82,6 +82,8 @@ class NeighborSearch public: //! Convenience typedef. typedef TreeType, MatType> Tree; + //! The type of element held in MatType. + typedef typename MatType::elem_type ElemType; /** * Initialize the NeighborSearch object, passing a reference dataset (this is @@ -223,7 +225,7 @@ class NeighborSearch void Search(const MatType& querySet, const size_t k, arma::Mat& neighbors, - MatType& distances); + arma::Mat& distances); /** * Given a pre-built query tree, search for the nearest neighbors of each @@ -248,7 +250,7 @@ class NeighborSearch void Search(Tree& queryTree, const size_t k, arma::Mat& neighbors, - MatType& distances, + arma::Mat& distances, bool sameSet = false); /** @@ -267,7 +269,7 @@ class NeighborSearch */ void Search(const size_t k, arma::Mat& neighbors, - MatType& distances); + arma::Mat& distances); /** * Calculate the average relative error (effective error) between the @@ -284,8 +286,8 @@ class NeighborSearch * query point. * @return Average relative error. */ - static double EffectiveError(MatType& foundDistances, - MatType& realDistances); + static double EffectiveError(arma::Mat& foundDistances, + arma::Mat& realDistances); /** * Calculate the recall (% of neighbors found) given the list of found diff --git a/src/mlpack/methods/neighbor_search/neighbor_search_impl.hpp b/src/mlpack/methods/neighbor_search/neighbor_search_impl.hpp index 850bfd23dd..c528d23f04 100644 --- a/src/mlpack/methods/neighbor_search/neighbor_search_impl.hpp +++ b/src/mlpack/methods/neighbor_search/neighbor_search_impl.hpp @@ -365,7 +365,7 @@ DualTreeTraversalType, SingleTreeTraversalType>::Search( const MatType& querySet, const size_t k, arma::Mat& neighbors, - MatType& distances) + arma::Mat& distances) { if (k > referenceSet->n_cols) { @@ -386,14 +386,14 @@ DualTreeTraversalType, SingleTreeTraversalType>::Search( // To avoid an extra copy, we will store the neighbors and distances in a // separate matrix. arma::Mat* neighborPtr = &neighbors; - MatType* distancePtr = &distances; + arma::Mat* distancePtr = &distances; // Mapping is only necessary if the tree rearranges points. if (TreeTraits::RearrangesDataset) { if (searchMode == DUAL_TREE_MODE) { - distancePtr = new MatType; // Query indices need to be mapped. + distancePtr = new arma::Mat; // Query indices need to be mapped. neighborPtr = new arma::Mat; } else if (!oldFromNewReferences.empty()) @@ -570,7 +570,7 @@ DualTreeTraversalType, SingleTreeTraversalType>::Search( Tree& queryTree, const size_t k, arma::Mat& neighbors, - MatType& distances, + arma::Mat& distances, bool sameSet) { if (k > referenceSet->n_cols) @@ -648,7 +648,7 @@ void NeighborSearch::Search( const size_t k, arma::Mat& neighbors, - MatType& distances) + arma::Mat& distances) { if (k > referenceSet->n_cols) { @@ -670,7 +670,7 @@ DualTreeTraversalType, SingleTreeTraversalType>::Search( scores = 0; arma::Mat* neighborPtr = &neighbors; - MatType* distancePtr = &distances; + arma::Mat* distancePtr = &distances; if (!oldFromNewReferences.empty() && TreeTraits::RearrangesDataset) { @@ -825,8 +825,8 @@ template class SingleTreeTraversalType> double NeighborSearch::EffectiveError( - MatType& foundDistances, - MatType& realDistances) + arma::Mat& foundDistances, + arma::Mat& realDistances) { if (foundDistances.n_rows != realDistances.n_rows || foundDistances.n_cols != realDistances.n_cols) From 2f3fde220b208e7f78ebc08f8d5d862891154cce Mon Sep 17 00:00:00 2001 From: Omar Shrit Date: Thu, 7 Dec 2023 19:57:19 +0100 Subject: [PATCH 3/5] Add a float tests, fix the distances in the rules search Signed-off-by: Omar Shrit --- .../neighbor_search/neighbor_search_rules.hpp | 5 ++- .../neighbor_search_rules_impl.hpp | 2 +- src/mlpack/tests/knn_test.cpp | 38 +++++++++++++++++++ 3 files changed, 43 insertions(+), 2 deletions(-) diff --git a/src/mlpack/methods/neighbor_search/neighbor_search_rules.hpp b/src/mlpack/methods/neighbor_search/neighbor_search_rules.hpp index 9b7ce53661..02577a4049 100644 --- a/src/mlpack/methods/neighbor_search/neighbor_search_rules.hpp +++ b/src/mlpack/methods/neighbor_search/neighbor_search_rules.hpp @@ -34,6 +34,9 @@ template class NeighborSearchRules { public: + //! The type of element held in MatType. + typedef typename TreeType::Mat::elem_type ElemType; + /** * Construct the NeighborSearchRules object. This is usually done from within * the NeighborSearch class at search time. @@ -60,7 +63,7 @@ class NeighborSearchRules * @param distances Matrix storing distances of neighbors for each query * point. */ - void GetResults(arma::Mat& neighbors, arma::mat& distances); + void GetResults(arma::Mat& neighbors, arma::Mat& distances); /** * Get the distance from the query point to the reference point. diff --git a/src/mlpack/methods/neighbor_search/neighbor_search_rules_impl.hpp b/src/mlpack/methods/neighbor_search/neighbor_search_rules_impl.hpp index 0c1f50d58e..f754427dc2 100644 --- a/src/mlpack/methods/neighbor_search/neighbor_search_rules_impl.hpp +++ b/src/mlpack/methods/neighbor_search/neighbor_search_rules_impl.hpp @@ -61,7 +61,7 @@ NeighborSearchRules::NeighborSearchRules( template void NeighborSearchRules::GetResults( arma::Mat& neighbors, - arma::mat& distances) + arma::Mat& distances) { neighbors.set_size(k, querySet.n_cols); distances.set_size(k, querySet.n_cols); diff --git a/src/mlpack/tests/knn_test.cpp b/src/mlpack/tests/knn_test.cpp index dd302a85df..797c717e0f 100644 --- a/src/mlpack/tests/knn_test.cpp +++ b/src/mlpack/tests/knn_test.cpp @@ -746,6 +746,44 @@ TEST_CASE("KNNSingleTreeVsNaive", "[KNNTest]") } } +/** + * Test the single-tree nearest-neighbors method with the naive method. + * + * The main difference with the above test is that this one loads the reference + * dataset as a float32, and the distances as a float32 as well. + * + * Errors are produced if the results are not identical. + */ +TEST_CASE("KNNSingleTreeVsNaiveF32", "[KNNTest]") +{ + arma::fmat dataset; + + // Hard-coded filename: bad? + // Code duplication: also bad! + if (!data::Load("test_data_3_1000.csv", dataset)) + FAIL("Cannot load test dataset test_data_3_1000.csv!"); + + NeighborSearch + knn(dataset, SINGLE_TREE_MODE); + + // Set up computation for naive mode. + NeighborSearch + naive(dataset, NAIVE_MODE); + + arma::Mat neighborsTree; + arma::fmat distancesTree; + knn.Search(15, neighborsTree, distancesTree); + + arma::Mat neighborsNaive; + arma::fmat distancesNaive; + naive.Search(15, neighborsNaive, distancesNaive); + + for (size_t i = 0; i < neighborsTree.n_elem; ++i) + { + REQUIRE(neighborsTree[i] ==neighborsNaive[i]); + REQUIRE(distancesTree[i] == Approx(distancesNaive[i]).epsilon(1e-7)); + } +} /** * Test the cover tree single-tree nearest-neighbors method against the naive * method. This uses only a random reference dataset. From 63e512ef2e3e61f686a67b325e3d3dde62969cf0 Mon Sep 17 00:00:00 2001 From: Omar Shrit Date: Mon, 11 Dec 2023 22:36:26 +0100 Subject: [PATCH 4/5] Fix the declarations for the knn float Signed-off-by: Omar Shrit --- src/mlpack/tests/knn_test.cpp | 24 ++++++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/src/mlpack/tests/knn_test.cpp b/src/mlpack/tests/knn_test.cpp index 797c717e0f..3f77aba9ca 100644 --- a/src/mlpack/tests/knn_test.cpp +++ b/src/mlpack/tests/knn_test.cpp @@ -16,6 +16,18 @@ using namespace mlpack; +/** + * A couple of handful declarations for float32 testing. + * These will be removed when we refactor the Bounds to accept MatType. + * For now, we will keep the following declarations. + */ +template +using FloatHRectBound = HRectBound; + +template +using FloatKDTree = BinarySpaceTree; + /** * Test that Unmap() works in the dual-tree case (see unmap.hpp). */ @@ -763,12 +775,16 @@ TEST_CASE("KNNSingleTreeVsNaiveF32", "[KNNTest]") if (!data::Load("test_data_3_1000.csv", dataset)) FAIL("Cannot load test dataset test_data_3_1000.csv!"); - NeighborSearch - knn(dataset, SINGLE_TREE_MODE); + NeighborSearch knn(dataset, SINGLE_TREE_MODE); // Set up computation for naive mode. - NeighborSearch - naive(dataset, NAIVE_MODE); + NeighborSearch naive(dataset, NAIVE_MODE); arma::Mat neighborsTree; arma::fmat distancesTree; From 884657910760421d574f692c7aa41417643153ac Mon Sep 17 00:00:00 2001 From: Omar Shrit Date: Mon, 11 Dec 2023 23:24:29 +0100 Subject: [PATCH 5/5] Fix arma::vec to arma::Col Signed-off-by: Omar Shrit --- src/mlpack/core/tree/binary_space_tree/binary_space_tree.hpp | 2 +- .../core/tree/binary_space_tree/binary_space_tree_impl.hpp | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/mlpack/core/tree/binary_space_tree/binary_space_tree.hpp b/src/mlpack/core/tree/binary_space_tree/binary_space_tree.hpp index a042ba849d..8289e0179f 100644 --- a/src/mlpack/core/tree/binary_space_tree/binary_space_tree.hpp +++ b/src/mlpack/core/tree/binary_space_tree/binary_space_tree.hpp @@ -504,7 +504,7 @@ class BinarySpaceTree size_t& Count() { return count; } //! Store the center of the bounding region in the given vector. - void Center(arma::vec& center) const { bound.Center(center); } + void Center(arma::Col& center) const { bound.Center(center); } private: /** diff --git a/src/mlpack/core/tree/binary_space_tree/binary_space_tree_impl.hpp b/src/mlpack/core/tree/binary_space_tree/binary_space_tree_impl.hpp index 0a1e1d31db..da5bd3d3ba 100644 --- a/src/mlpack/core/tree/binary_space_tree/binary_space_tree_impl.hpp +++ b/src/mlpack/core/tree/binary_space_tree/binary_space_tree_impl.hpp @@ -909,7 +909,7 @@ void BinarySpaceTree:: splitter, maxLeafSize); // Calculate parent distances for those two nodes. - arma::vec center, leftCenter, rightCenter; + arma::Col center, leftCenter, rightCenter; Center(center); left->Center(leftCenter); right->Center(rightCenter); @@ -977,7 +977,7 @@ SplitNode(std::vector& oldFromNew, oldFromNew, splitter, maxLeafSize); // Calculate parent distances for those two nodes. - arma::vec center, leftCenter, rightCenter; + arma::Col center, leftCenter, rightCenter; Center(center); left->Center(leftCenter); right->Center(rightCenter);