[mlpack-git] [mlpack/mlpack] Density Estimation Tree made sparse-enabled (#802)
notifications at github.com
Tue Nov 1 11:33:24 EDT 2016
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:
* 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:
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the mlpack-git