<p>I've found another bug in r/r*/x trees. But I am not sure in the best solution.</p>
<p>Suppose you use the RectangleTree(const RectangleTree& other,const bool deepCopy) constructor.<br>
(it is used in all Split methods)<br>
It results in a new dataset. <br>
<code>dataset(new MatType(*other.dataset)),<br>
ownsDataset(true),<br>
</code><br>
Therefore we should not delete this node until we delete all its children which are created using the RectangleTree(Rectangle tree *parentNode) constructor.<br>
But it may happen (and it happens) in split methods and in RectangleTree::CondenseTree.</p>
<p>Possible solutions:<br>
1. Fix constructor<br>
<code>dataset(deepCopy ? new MatType(*other.dataset) : &other->Dataset()),<br>
ownsDataset(deepCopy),<br>
</code><br>
But I am not sure of deepCopy=true (Is it used anywhere? In this case a deep copy should be a root)<br>
2. Why do you use<br>
<code>const MatType* dataset;</code> ?<br>
Maybe it is better to use something like this<br>
<code>MatType *dataset;<br>
MatType* DatasetPtr() const { return dataset; }<br>
MatType*& DatasetPtr() { return dataset; }<br>
</code><br>
But it results in a recursive change of all pointers in all descendant nodes.</p>
<p style="font-size:small;-webkit-text-size-adjust:none;color:#666;">—<br>Reply to this email directly or <a href="https://github.com/mlpack/mlpack/pull/556#issuecomment-195739959">view it on GitHub</a>.<img alt="" height="1" src="https://github.com/notifications/beacon/AJ4bFJXe_DgciB10Q2tTLT_rY3-VAygfks5psrzVgaJpZM4HsSai.gif" width="1" /></p>
<div itemscope itemtype="http://schema.org/EmailMessage">
<div itemprop="action" itemscope itemtype="http://schema.org/ViewAction">
<link itemprop="url" href="https://github.com/mlpack/mlpack/pull/556#issuecomment-195739959"></link>
<meta itemprop="name" content="View Pull Request"></meta>
</div>
<meta itemprop="description" content="View this Pull Request on GitHub"></meta>
</div>