Use least_squares solver from ndarray-linalg #125
Conversation
Codecov Report
@@ Coverage Diff @@
## master #125 +/- ##
==========================================
+ Coverage 59.84% 59.86% +0.01%
==========================================
Files 74 74
Lines 7006 7007 +1
==========================================
+ Hits 4193 4195 +2
+ Misses 2813 2812 -1
Continue to review full report at Codecov.
|
algorithms/linfa-linear/src/ols.rs
Outdated
| .solve_into(rhs) | ||
| // It would be cleaner to use ndarray_linalg::LeastSquaresSvd::least_squares, | ||
| // but that is currently not possible since B = C is required | ||
| let X = X.to_owned(); |
There was a problem hiding this comment.
can you accept a view representation as parameter and avoid the copy here?
There was a problem hiding this comment.
I don't see how, since the types of the data containers for X and y are different (B and C respectively). In an upcoming version of ndarray-linalg this restriction has been lifted so we don't have to make an explicit copy here (rust-ndarray/ndarray-linalg#272).
There was a problem hiding this comment.
the data is copied for the intercept branch anyways. I pushed a PR https://github.com/janmarthedal/linfa/pull/1 which adds the copy only for the branch without intercept computation
There was a problem hiding this comment.
Ah, I see, that makes sense. Thank you for helping out!
Only copy dataset for branch without intercept computation
Use the least squares solver from ndarray-linalg instead of solving the normal equations explicitly.
Resolves #25