[mlpack-git] [mlpack/mlpack] Density Estimation Tree made sparse-enabled (#802)

[Ryan Curtin]

Don't worry about the Travis failure, that seems to be some other test where the tolerance needs adjusting.

The impression I am getting---and correct me if I am wrong---is that you agree it would be nice to have only one implementation of `ExtractSplits()` that had, e.g., `MatType` as a template parameter, but this is not possible due to the current drawbacks of the Armadillo sparse matrix implementation.  I agree that there are definitely problems with the Armadillo CSC implementation so for now I think it's reasonable to have two versions of `ExtractSplits()`, but I think in the long run we should aim in the following directions:

 * reimplement `SpMat` as coordinate list, because really I have not found any cases where CSC gives significant speedup.  Coordinate lists also mean that a lot of non-linear-algebraic operations like sorting, iterating, and so forth become a million times easier.  This task is something I will do, I am just not sure when I'll have the time.

 * Get better sparse matrix file reading in place (and submitted upstream to Armadillo when possible).  I know that Armadillo has coordinate list file loading support, but there isn't a way to use that currently with any of the mlpack command-line programs.  Maybe that is worth looking into too.  What formats are you thinking of?  The more I think of it the following *should* work:

```
arma::sp_mat matrix;
data::Load("coordinate_list.csv", matrix);
```

 * Take your `sort()` implementation and submit it upstream to Armadillo.  Would you like to do this?  I can help.  Basically we'll just need to write tests for it then send it to Conrad and he's likely to take a look over and then include it.

Are you finished with this PR otherwise?  If so I'll go ahead and merge it.  Thanks again for your time on this, it is a definite improvement.

-- 
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/802#issuecomment-257598603
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mailman.cc.gatech.edu/pipermail/mlpack-git/attachments/20161101/4d44533d/attachment-0001.html>