[mlpack-git] [mlpack/mlpack] Remove code redundancy in ns_model_impl.hpp (#674)

[MarcosPividori]

Hi!
I have been considering different approaches to this problem.
We want a data type to represent a generic neighbor search class, no matter which tree type is considered. However, we want to distinguish between KDTree/BallTree and the rest of tree types, because we want to set the leaf size in the former and the NeighborSearch Class doesn't provide a general option for that. This is the reason of the difference in the code, between KDTrees/BallTrees and the rest of the tree types.
So, as you suggested, I considered three possible approaches:

- Add TreeType as a template parameter and use SFINAE to decide if the tree should be built with specific leafSize or not. It involved a lot of lines of code...

- Create a generic NeighborSearchGen class, and make NeighborSearch class inherit from it. Then, I implemented a NeighborSearchLeaf class that encapsulates an instance of NeighborSearch class, and adds the functionality to deal with different leaf sizes. It is implemented in: https://github.com/MarcosPividori/mlpack/tree/improve-nsmodel

- Finally I considered boost variant:
  ```
    boost::variant<NSType<SortPolicy, tree::KDTree>*,
                   NSType<SortPolicy, tree::StandardCoverTree>*,
                   NSType<SortPolicy, tree::RTree>*,
                   NSType<SortPolicy, tree::RStarTree>*,
                   NSType<SortPolicy, tree::BallTree>*,
                   NSType<SortPolicy, tree::XTree>*> nSearch;
  ```
  I use template specialization in the visitors to decide if the leafSize should be considered or not.
 This is by far the simplest option, which include the least changes in actual code. Also, it only uses inheritance when defining the visitors, so I think it won't be confusing. It is implemented in: https://github.com/MarcosPividori/mlpack/tree/variants-for-ns-model

What do you think? I made a pull request in : https://github.com/mlpack/mlpack/pull/693 

---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/mlpack/mlpack/issues/674#issuecomment-225604519
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mailman.cc.gatech.edu/pipermail/mlpack-git/attachments/20160613/0289f43d/attachment.html>