diff --git a/src/mlpack/methods/range_search/range_search.hpp b/src/mlpack/methods/range_search/range_search.hpp index d35d09b9c2..4b0c08f0b4 100644 --- a/src/mlpack/methods/range_search/range_search.hpp +++ b/src/mlpack/methods/range_search/range_search.hpp @@ -51,9 +51,9 @@ class RangeSearch * naive mode or single-tree mode. Additionally, an instantiated metric can be * given, for cases where the distance metric holds data. * - * This method will copy the matrices to internal copies, which are rearranged - * during tree-building. You can avoid this extra copy by pre-constructing - * the trees and passing them using a different constructor. + * This method will move the matrices to internal copies, which are + * rearranged during tree-building. You can avoid creating an extra copy by + * pre-constructing the trees and passing them in using std::move. * * @param referenceSet Reference dataset. * @param naive Whether the computation should be done in O(n^2) naive mode. @@ -61,31 +61,7 @@ class RangeSearch * opposed to dual-tree computation). * @param metric Instantiated distance metric. */ - RangeSearch(const MatType& referenceSet, - const bool naive = false, - const bool singleMode = false, - const MetricType metric = MetricType()); - - /** - * Initialize the RangeSearch object with the given reference dataset (this is - * the dataset which is searched), taking ownership of the matrix. - * Optionally, perform the computation in naive mode or single-tree mode. - * Additionally, an instantiated metric can be given, for cases where the - * distance metric holds data. - * - * This method will not copy the data matrix, but will take ownership of it, - * and depending on the type of tree used, may rearrange the points. If you - * would rather a copy be made, consider using the constructor that takes a - * const reference to the data instead. - * - * @param referenceSet Set of reference points. - * @param naive If true, brute force naive search will be used (as opposed to - * dual-tree search). This overrides singleMode (if it is set to true). - * @param singleMode If true, single-tree search will be used (as opposed to - * dual-tree search). - * @param metric An optional instance of the MetricType class. - */ - RangeSearch(MatType&& referenceSet, + RangeSearch(MatType referenceSet, const bool naive = false, const bool singleMode = false, const MetricType metric = MetricType()); @@ -149,17 +125,11 @@ class RangeSearch /** * Copy the given RangeSearch model. + * Use std::move to pass in the model if the old copy is no longer needed. * * @param other RangeSearch model to copy. */ - RangeSearch& operator=(const RangeSearch& other); - - /** - * Take ownership of the given RangeSearch model. - * - * @param other RangeSearch model to take ownership of. - */ - RangeSearch& operator=(RangeSearch&& other); + RangeSearch& operator=(RangeSearch other); /** * Destroy the RangeSearch object. If trees were created, they will be @@ -170,22 +140,15 @@ class RangeSearch /** * Set the reference set to a new reference set, and build a tree if * necessary. This method is called 'Train()' in order to match the rest of - * the mlpack abstractions, even though calling this "training" is maybe a bit - * of a stretch. + * the mlpack abstractions, even though calling this "training" is maybe a + * bit of a stretch. + * + * Use std::move to pass in the reference set if the old copy is no longer + * needed. * * @param referenceSet New set of reference data. */ - void Train(const MatType& referenceSet); - - /** - * Set the reference set to a new reference set, taking ownership of the set. - * A tree is built if necessary. This method is called 'Train()' in order to - * match the rest of the mlpack abstractions, even though calling this - * "training" is maybe a bit of a stretch. - * - * @param referenceSet New set of reference data. - */ - void Train(MatType&& referenceSet); + void Train(MatType referenceSet); /** * Set the reference tree to a new reference tree. @@ -335,8 +298,6 @@ class RangeSearch //! If true, this object is responsible for deleting the trees. bool treeOwner; - //! If true, we own the reference set. - bool setOwner; //! If true, O(n^2) naive computation is used. bool naive; diff --git a/src/mlpack/methods/range_search/range_search_impl.hpp b/src/mlpack/methods/range_search/range_search_impl.hpp index 141ca217b6..d4cbcc5f21 100644 --- a/src/mlpack/methods/range_search/range_search_impl.hpp +++ b/src/mlpack/methods/range_search/range_search_impl.hpp @@ -48,32 +48,7 @@ template class TreeType> RangeSearch::RangeSearch( - const MatType& referenceSetIn, - const bool naive, - const bool singleMode, - const MetricType metric) : - referenceTree(naive ? NULL : BuildTree(referenceSetIn, - oldFromNewReferences)), - referenceSet(naive ? &referenceSetIn : &referenceTree->Dataset()), - treeOwner(!naive), // If in naive mode, we are not building any trees. - setOwner(false), - naive(naive), - singleMode(!naive && singleMode), // Naive overrides single mode. - metric(metric), - baseCases(0), - scores(0) -{ - // Nothing to do. -} - -// Move constructor. -template class TreeType> -RangeSearch::RangeSearch( - MatType&& referenceSet, + MatType referenceSet, const bool naive, const bool singleMode, const MetricType metric) : @@ -82,7 +57,6 @@ RangeSearch::RangeSearch( referenceSet(naive ? new MatType(std::move(referenceSet)) : &referenceTree->Dataset()), treeOwner(!naive), - setOwner(naive), naive(naive), singleMode(!naive && singleMode), metric(metric), @@ -104,7 +78,6 @@ RangeSearch::RangeSearch( referenceTree(referenceTree), referenceSet(&referenceTree->Dataset()), treeOwner(false), - setOwner(false), naive(false), singleMode(singleMode), metric(metric), @@ -126,7 +99,6 @@ RangeSearch::RangeSearch( referenceTree(NULL), referenceSet(new MatType()), // Empty matrix. treeOwner(false), - setOwner(true), naive(naive), singleMode(singleMode), metric(metric), @@ -154,7 +126,6 @@ RangeSearch::RangeSearch( referenceSet(other.referenceTree ? &referenceTree->Dataset() : new MatType(*other.referenceSet)), treeOwner(other.referenceTree), - setOwner(!other.referenceTree), naive(other.naive), singleMode(other.singleMode), metric(other.metric), @@ -174,7 +145,6 @@ RangeSearch::RangeSearch(RangeSearch&& other) : referenceTree(other.referenceTree), referenceSet(other.referenceSet), treeOwner(other.treeOwner), - setOwner(other.setOwner), naive(other.naive), singleMode(other.singleMode), metric(std::move(other.metric)), @@ -187,7 +157,6 @@ RangeSearch::RangeSearch(RangeSearch&& other) : BuildTree(const_cast(*other.referenceSet), other.oldFromNewReferences); other.treeOwner = true; - other.setOwner = true; other.naive = false; other.singleMode = false; other.baseCases = 0; @@ -200,42 +169,12 @@ template class TreeType> RangeSearch& -RangeSearch::operator=(const RangeSearch& other) +RangeSearch::operator=(RangeSearch other) { // Clean memory first. if (treeOwner) delete referenceTree; - if (setOwner) - delete referenceSet; - - // Copy the other model. - oldFromNewReferences = other.oldFromNewReferences; - referenceTree = other.referenceTree ? new Tree(*other.referenceTree) : NULL; - referenceSet = other.referenceTree ? &referenceTree->Dataset() : - new MatType(*other.referenceSet); - treeOwner = other.referenceTree; - setOwner = !other.referenceTree; - naive = other.naive; - singleMode = other.singleMode; - metric = other.metric; - baseCases = other.baseCases; - scores = other.scores; - - return *this; -} - -template class TreeType> -RangeSearch& -RangeSearch::operator=(RangeSearch&& other) -{ - // Clean memory first. - if (treeOwner) - delete referenceTree; - if (setOwner) + if (naive) delete referenceSet; // Move the other model. @@ -243,25 +182,12 @@ RangeSearch::operator=(RangeSearch&& other) referenceTree = other.referenceTree; referenceSet = other.referenceSet; treeOwner = other.treeOwner; - setOwner = other.setOwner; naive = other.naive; singleMode = other.singleMode; metric = std::move(other.metric); baseCases = other.baseCases; scores = other.scores; - // Clean other model. - other.referenceSet = new MatType(); - other.referenceTree = - BuildTree(const_cast(*other.referenceSet), - other.oldFromNewReferences); - other.treeOwner = true; - other.setOwner = true; - other.naive = false; - other.singleMode = false; - other.baseCases = 0; - other.scores = 0; - return *this; } @@ -274,7 +200,7 @@ RangeSearch::~RangeSearch() { if (treeOwner && referenceTree) delete referenceTree; - if (setOwner && referenceSet) + if (naive && referenceSet) delete referenceSet; } @@ -284,42 +210,7 @@ template class TreeType> void RangeSearch::Train( - const MatType& referenceSet) -{ - // Clean up the old tree, if we built one. - if (treeOwner && referenceTree) - delete referenceTree; - - // Rebuild the tree, if necessary. - if (!naive) - { - referenceTree = BuildTree(const_cast(referenceSet), - oldFromNewReferences); - treeOwner = true; - } - else - { - treeOwner = false; - } - - // Delete the old reference set, if we owned it. - if (setOwner && this->referenceSet) - delete this->referenceSet; - - if (!naive) - this->referenceSet = &referenceTree->Dataset(); - else - this->referenceSet = &referenceSet; - setOwner = false; -} - -template class TreeType> -void RangeSearch::Train( - MatType&& referenceSet) + MatType referenceSet) { // Clean up the old tree, if we built one. if (treeOwner && referenceTree) @@ -338,18 +229,16 @@ void RangeSearch::Train( } // Delete the old reference set, if we owned it. - if (setOwner && this->referenceSet) + if (naive && this->referenceSet) delete this->referenceSet; if (!naive) { this->referenceSet = &referenceTree->Dataset(); - setOwner = false; } else { this->referenceSet = new MatType(std::move(referenceSet)); - setOwner = true; } } @@ -367,13 +256,10 @@ void RangeSearch::Train( if (treeOwner && referenceTree) delete this->referenceTree; - if (setOwner && referenceSet) - delete this->referenceSet; this->referenceTree = referenceTree; this->referenceSet = &referenceTree->Dataset(); treeOwner = false; - setOwner = false; } template::serialize( { if (Archive::is_loading::value) { - if (setOwner && referenceSet) + if (referenceSet) delete referenceSet; - - setOwner = true; } ar & BOOST_SERIALIZATION_NVP(referenceSet); @@ -798,12 +682,8 @@ void RangeSearch::serialize( // necessary. if (Archive::is_loading::value) { - if (setOwner && referenceSet) - delete referenceSet; - referenceSet = &referenceTree->Dataset(); metric = referenceTree->Metric(); // Get the metric from the tree. - setOwner = false; } } } diff --git a/src/mlpack/methods/range_search/rs_model.hpp b/src/mlpack/methods/range_search/rs_model.hpp index 6abd6cd07b..9e429a14be 100644 --- a/src/mlpack/methods/range_search/rs_model.hpp +++ b/src/mlpack/methods/range_search/rs_model.hpp @@ -287,16 +287,11 @@ class RSModel /** * Copy the given RSModel. * + * Use std::move to pass in the model if the old copy is no longer needed. + * * @param other RSModel to copy. */ - RSModel& operator=(const RSModel& other); - - /** - * Take ownership of the given RSModel. - * - * @param other RSModel to take ownership of. - */ - RSModel& operator=(RSModel&& other); + RSModel& operator=(RSModel other); /** * Clean memory, if necessary. diff --git a/src/mlpack/methods/range_search/rs_model_impl.hpp b/src/mlpack/methods/range_search/rs_model_impl.hpp index 8045aa168a..23879f278d 100644 --- a/src/mlpack/methods/range_search/rs_model_impl.hpp +++ b/src/mlpack/methods/range_search/rs_model_impl.hpp @@ -59,22 +59,7 @@ inline RSModel::RSModel(RSModel&& other) : other.rSearch = decltype(other.rSearch)(); } -// Copy operator. -inline RSModel& RSModel::operator=(const RSModel& other) -{ - boost::apply_visitor(DeleteVisitor(), rSearch); - - treeType = other.treeType; - leafSize = other.leafSize; - randomBasis = other.randomBasis; - q = other.q; - rSearch = other.rSearch; - - return *this; -} - -// Move operator. -inline RSModel& RSModel::operator=(RSModel&& other) +inline RSModel& RSModel::operator=(RSModel other) { boost::apply_visitor(DeleteVisitor(), rSearch); @@ -84,12 +69,6 @@ inline RSModel& RSModel::operator=(RSModel&& other) q = std::move(other.q); rSearch = std::move(other.rSearch); - // Reset other model. - other.treeType = TreeTypes::KD_TREE; - other.leafSize = 0; - other.randomBasis = false; - other.rSearch = decltype(other.rSearch)(); - return *this; }