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

[lozhnikov]

I've found another bug in r/r*/x trees. But I am not sure in the best solution.

Suppose you use the RectangleTree(const RectangleTree& other,const bool deepCopy) constructor.
(it is used in all Split methods)
It results in a new dataset. 
`    dataset(new MatType(*other.dataset)),
    ownsDataset(true),
`
Therefore we should not delete this node until we delete all its children which are created using the RectangleTree(Rectangle tree *parentNode) constructor.
But it may happen (and it happens) in split methods and in RectangleTree::CondenseTree.

Possible solutions:
1. Fix constructor
`    dataset(deepCopy ? new MatType(*other.dataset) : &other->Dataset()),
    ownsDataset(deepCopy),
`
But I am not sure of deepCopy=true (Is it used anywhere? In this case a deep copy should be a root)
2. Why do you use
`const MatType* dataset;` ?
Maybe it is better to use something like this
`MatType *dataset;
 MatType* DatasetPtr() const { return dataset; }
 MatType*& DatasetPtr() { return dataset; }
`
But it results in a recursive change of all pointers in all descendant nodes.

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