add fkeep! and children for sparse vectors #15585
Conversation
| x | ||
| end | ||
|
|
||
| immutable DroptolFuncVec <: Base.Func{3} end |
There was a problem hiding this comment.
AFAIK passing anonymous functions is fast now, so you don't need to use functors, right?
There was a problem hiding this comment.
Please see PR description.
KristofferC
force-pushed
the
kc/fkeep_sparse_vector
branch
2 times, most recently
from
81b55e9 to
0ffe3cc
Compare
|
Updated to address comments. I looked and anonymous functions seems to work as well as functors but I rather keep the PRs small. |
| @@ -1680,7 +1680,7 @@ function sort{Tv,Ti}(x::SparseVector{Tv,Ti}; kws...) | |||
| SparseVector(n,newnzind,newnzvals) | |||
| end | |||
|
|
|||
| function fkeep!{Tv,Ti}(x::SparseVector{Tv,Ti}, f, other, trim::Bool = true) | |||
| function fkeep!(x::SparseVector{Tv,Ti}, f, other, trim::Bool = true) | |||
There was a problem hiding this comment.
I think the extraneous {Tv,Ti} is the cause of the build failure.
There was a problem hiding this comment.
Yeah, will fix. Thanks
KristofferC
force-pushed
the
kc/fkeep_sparse_vector
branch
2 times, most recently
from
cfcd3b6 to
83326ca
Compare
| is `true`, this method trims `A.rowval` and `A.nzval` to length `nnz(A)` after dropping | ||
| elements. | ||
| through `A`, requiring `O(A.n, nnz(A))`-time for matrices and `O(nnz(A))`-time for vectors | ||
| and no space beyond that passed in. If `trim` is `true`, this method trims `A.rowval/A.nzind` and |
There was a problem hiding this comment.
trims
A.rowvalorA.nzind
like that?
KristofferC
force-pushed
the
kc/fkeep_sparse_vector
branch
from
83326ca to
3a476f7
Compare
|
Updated to address @tkelman documentation comment. |
|
Test error seems irrelevant (SubArrays) on one worker. |
|
This is good to go from my P.O.V |
| xdrop = Base.droptol!(copy(x), 1.5) | ||
| @test exact_equal(xdrop, SparseVector(7, [1, 2, 5, 6, 7], [3., 2., -2., -3., 3.])) | ||
| Base.droptol!(xdrop, 2.5) | ||
| @test exact_equal(xdrop, SparseVector(7, [1, 6, 7], [3., -3., 3.])) |
There was a problem hiding this comment.
could you add a test of droptol! where the tolerance is exactly equal to one of the nonzero values? then I think this is done
KristofferC
force-pushed
the
kc/fkeep_sparse_vector
branch
from
3a476f7 to
5a30b99
Compare
KristofferC
force-pushed
the
kc/fkeep_sparse_vector
branch
from
5a30b99 to
2ad39b4
Compare
|
This will slightly clash with #15804 since it adds a few functor methods. Maybe they can be fixed in the same way in that PR. |
|
hopefully @martinholters won't mind getting rid of a couple more there? |
|
I won't. |
Since these exist for sparse matrices it makes sense that they do for sparse vectors.
I used functors to keep the same style of code for matrices and vectors. If functors gives no performance benefit they can be swapped out for both matrices and vectors in a separate PR.