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

[Ryan Curtin]

I agree that the second approach is better here.

Unless I have misunderstood, `normalModeMaxNumChildren` is something specific to the X tree, so I think we should try to remove the member from `RectangleTree` and put it in `XTreeSplit` if possible.  I think that this is reasonable, but will require some refactoring (so we can save it for later if you want):

We'd need to make `RectangleTree` use instantiated split objects and (optionally) descent type objects.  So instead of `SplitType::SplitNode()`, `RectangleTree` should hold a `SplitType split` object (or something like this), and then call `split.SplitNode()` instead.  That could allow the SplitType to hold state (like for instance the value of `normalModeMaxNumChildren`), and then the splitting function could make use of that state, and the other R-tree variants wouldn't need to hold something like `normalModeMaxNumChildren`.

Let me know what you think; hopefully I have understood everything correctly.  Like I said I'm happy to leave `normalModeMaxNumChildren` in the code for now and then open a ticket noting that it should be refactored at some point later in the future.

---
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/pull/556#issuecomment-208621517
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mailman.cc.gatech.edu/pipermail/mlpack-git/attachments/20160411/8dc8b6a8/attachment.html>