<p>In <a href="https://github.com/mlpack/mlpack/pull/747#discussion_r74694565">src/mlpack/core/tree/space_split/midpoint_space_split.hpp</a>:</p>
<pre style='color:#555'>> + /**
> + * Create a splitting hyperplane considering the midpoint of the values in a
> + * certain projection.
> + *
> + * @param bound The bound used for this node.
> + * @param data The dataset used by the tree.
> + * @param points Vector of indexes of points to be considered.
> + * @param hyp Resulting splitting hyperplane.
> + * @return Flag to determine if split is possible.
> + */
> + template<typename HyperplaneType>
> + static bool SplitSpace(
> + const typename HyperplaneType::BoundType& bound,
> + const MatType& data,
> + const arma::Col<size_t>& points,
> + HyperplaneType& hyp);
</pre>
<p>I think you mentioned in <a href="https://github.com/mlpack/mlpack/pull/726" class="issue-link js-issue-link" data-url="https://github.com/mlpack/mlpack/issues/726" data-id="166095364" data-error-text="Failed to load issue title" data-permission-text="Issue title is private">#726</a> that <a href="https://github.com/lozhnikov" class="user-mention">@lozhnikov</a> could use this class for splitting the random projection tree. I agree, that could be done, but maybe it is worth refactoring this as well as the <code>MeanSplit</code> and <code>MedianSplit</code> classes in order to have the same API. The key here would be to make <code>MatType</code> a template parameter to the <code>SplitSpace()</code> method and then you could pass either an Armadillo contiguous subview (i.e. <code>mat.cols(begin, begin + count - 1)</code>) or a non-contiguous subview (i.e. <code>mat.cols(points)</code> where <code>points</code> is an <code>arma::uvec</code> with indices of points).</p>
<p>As you wrote in that ticket, it seems reasonable to do some refactoring and have three template parameters: the type of splitting hyperplane, how to choose the hyperplane, and which splitting value we will consider. What do you think is the best way to do this refactoring? Should we make a class that is used by <code>BinarySpaceTree</code> that has these three parameters, or should we make <code>BinarySpaceTree</code> have those three parameters directly and then make sure we can still make the vantage point tree using those parameters?</p>
<p>Whatever we do will be reverse-incompatible for the "internal API", so it would mean the next version is at least a minor version bump (I'm just fine with that).</p>
<p style="font-size:small;-webkit-text-size-adjust:none;color:#666;">—<br />You are receiving this because you are subscribed to this thread.<br />Reply to this email directly, <a href="https://github.com/mlpack/mlpack/pull/747/files/fe090ee13c7cad79e2b7eb8b6690628ba3ead1ed#r74694565">view it on GitHub</a>, or <a href="https://github.com/notifications/unsubscribe-auth/AJ4bFND8OMdBj-6H9IuYWHg08MZhQt9mks5qfnCYgaJpZM4JZzLU">mute the thread</a>.<img alt="" height="1" src="https://github.com/notifications/beacon/AJ4bFC1avjcJjpchdD9ohTetyHWMvdW1ks5qfnCYgaJpZM4JZzLU.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/747/files/fe090ee13c7cad79e2b7eb8b6690628ba3ead1ed#r74694565"></link>
<meta itemprop="name" content="View Pull Request"></meta>
</div>
<meta itemprop="description" content="View this Pull Request on GitHub"></meta>
</div>
<script type="application/json" data-scope="inboxmarkup">{"api_version":"1.0","publisher":{"api_key":"05dde50f1d1a384dd78767c55493e4bb","name":"GitHub"},"entity":{"external_key":"github/mlpack/mlpack","title":"mlpack/mlpack","subtitle":"GitHub repository","main_image_url":"https://cloud.githubusercontent.com/assets/143418/17495839/a5054eac-5d88-11e6-95fc-7290892c7bb5.png","avatar_image_url":"https://cloud.githubusercontent.com/assets/143418/15842166/7c72db34-2c0b-11e6-9aed-b52498112777.png","action":{"name":"Open in GitHub","url":"https://github.com/mlpack/mlpack"}},"updates":{"snippets":[{"icon":"PERSON","message":"@rcurtin in #747: I think you mentioned in #726 that @lozhnikov could use this class for splitting the random projection tree. I agree, that could be done, but maybe it is worth refactoring this as well as the `MeanSplit` and `MedianSplit` classes in order to have the same API. The key here would be to make `MatType` a template parameter to the `SplitSpace()` method and then you could pass either an Armadillo contiguous subview (i.e. `mat.cols(begin, begin + count - 1)`) or a non-contiguous subview (i.e. `mat.cols(points)` where `points` is an `arma::uvec` with indices of points).\r\n\r\nAs you wrote in that ticket, it seems reasonable to do some refactoring and have three template parameters: the type of splitting hyperplane, how to choose the hyperplane, and which splitting value we will consider. What do you think is the best way to do this refactoring? Should we make a class that is used by `BinarySpaceTree` that has these three parameters, or should we make `BinarySpaceTree` have those three parameters directly and then make sure we can still make the vantage point tree using those parameters?\r\n\r\nWhatever we do will be reverse-incompatible for the \"internal API\", so it would mean the next version is at least a minor version bump (I'm just fine with that)."}],"action":{"name":"View Pull Request","url":"https://github.com/mlpack/mlpack/pull/747/files/fe090ee13c7cad79e2b7eb8b6690628ba3ead1ed#r74694565"}}}</script>