<p>Yes, I'm done with the PR - feel free to merge it, whenever is ok for you.</p>

<p>I wish I could agree on the <code>ExtractSort</code> issue, but there are sparse-specific problems, in short - <code>row_col_iterator</code> will not report the zero values, but the split decision algorithm does need them - to make a split attempt between a zero and the adjacent non-zero element. The all-universal implementation with sparse enabled <code>arma::sort</code> and then index-based iteration will be correct, but terribly slow because of all the zeroes, that will be reported.</p>

<p>My implementation on <code>arma::sort</code> is as part of MLPack's arma extensions. It is a less-than-a-page source code, which I can even paste into a comment, I guess :-) However, if it's really better to go the PR to Armadillo way - I can do it, I just don't know exactly when.</p>

<p>Regarding the coordinate list sparse matrix implementation - isn't the CSC storage faster when it comes to column-based iteration? Anything that is fast on both rows and columns directed iteration will be fine.</p>

<p>Regarding the format loading - I'd say that the format to be supported is Matrix Market, which is a coordinate list with the header. Such format will also save one swipe through the input file, which is happening now - for obtaining the matrix size, and which technically is not even quite correct, because if you have zeroes at the last columns/rows you will not have entries for these in the coordinate list and will deduce a false matrix size.</p>

<p style="font-size:small;-webkit-text-size-adjust:none;color:#666;">&mdash;<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/802#issuecomment-257605495">view it on GitHub</a>, or <a href="https://github.com/notifications/unsubscribe-auth/AJ4bFKzfAqSYO05byhF1Zxa8C1_mkdO-ks5q52E6gaJpZM4KZnsm">mute the thread</a>.<img alt="" height="1" src="https://github.com/notifications/beacon/AJ4bFMyTCyB5L5QjVHzk6XU7Or9p_Modks5q52E6gaJpZM4KZnsm.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/802#issuecomment-257605495"></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":"@thejonan in #802: Yes, I'm done with the PR - feel free to merge it, whenever is ok for you.\r\n\r\nI wish I could agree on the `ExtractSort` issue, but there are sparse-specific problems, in short - `row_col_iterator` will not report the zero values, but the split decision algorithm does need them - to make a split attempt between a zero and the adjacent non-zero element. The all-universal implementation with sparse enabled `arma::sort` and then index-based iteration will be correct, but terribly slow because of all the zeroes, that will be reported.\r\n\r\nMy implementation on `arma::sort` is as part of MLPack's arma extensions. It is a less-than-a-page source code, which I can even paste into a comment, I guess :-) However, if it's really better to go the PR to Armadillo way - I can do it, I just don't know exactly when.\r\n\r\nRegarding the coordinate list sparse matrix implementation - isn't the CSC storage faster when it comes to column-based iteration? Anything that is fast on both rows and columns directed iteration will be fine.\r\n\r\nRegarding the format loading - I'd say that the format to be supported is Matrix Market, which is a coordinate list with the header. Such format will also save one swipe through the input file, which is happening now - for obtaining the matrix size, and which technically is not even quite correct, because if you have zeroes at the last columns/rows you will not have entries for these in the coordinate list and will deduce a false matrix size.\r\n"}],"action":{"name":"View Pull Request","url":"https://github.com/mlpack/mlpack/pull/802#issuecomment-257605495"}}}</script>