[mlpack-git] [mlpack] Bugfix #350 and some fixes in RStarTreeSplit and XTreeSplit (#556)

Ryan Curtin notifications at github.com
Thu Mar 10 18:07:22 EST 2016


Thanks for the `lowest()` fix, I'm not sure how I misunderstood the documentation for that.

I tried running `mlpack_allknn` with your branch to test the speed but it segfaulted.  So I recompiled with debugging symbols, and ran it through valgrind, and saw this:

```
==24731== Invalid read of size 8
==24731==    at 0x989CF8: bool mlpack::tree::XTreeSplit::SplitNonLeafNode<mlpack::tree::RectangleTree<mlpack::metric::LMetric<2, true>, mlpack::neighbor::NeighborSearchStat<mlpack::neighbor::NearestNeighborSort>, arma::Mat<double>, mlpack::tree::XTreeSplit, mlpack::tree::RStarTreeDescentHeuristic> >(mlpack::tree::RectangleTree<mlpack::metric::LMetric<2, true>, mlpack::neighbor::NeighborSearchStat<mlpack::neighbor::NearestNeighborSort>, arma::Mat<double>, mlpack::tree::XTreeSplit, mlpack::tree::RStarTreeDescentHeuristic>*, std::vector<bool, std::allocator<bool> >&) (x_tree_split_impl.hpp:811)
(plus lots more output)
```

I haven't had a chance to look into it yet, and probably won't for a little while, unfortunately.

A last thought---adding the support for X trees to NSModel and RSModel and RAModel is great (thanks!) but we should add the X tree after the ball tree in order to preserve reverse compatibility.  Otherwise old models that were built with a ball tree will be loaded as an X tree (and the load will fail) because X_TREE will be equal to what BALL_TREE was in older versions.

I'm excited about merging this functionality in, so thanks for taking the time to look into this!  If you don't want to add your name that's fine, but personally I think your contribution is pretty substantial. Usually the level for inclusion in the credits is just "more than one line of code". :)

---
Reply to this email directly or view it on GitHub:
https://github.com/mlpack/mlpack/pull/556#issuecomment-195091510
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mailman.cc.gatech.edu/pipermail/mlpack-git/attachments/20160310/8bef6538/attachment.html>


More information about the mlpack-git mailing list