SparseArrays.jl
SparseArrays.jl copied to clipboard
Add a sparse `ev_view` for `SymTridiagonal` backed by sparse arrays
This is a fix for https://github.com/JuliaLang/julia/issues/46355, where SymTridiagonal matrices backed by sparse arrays couldn't be summed/subtracted/etc. The root cause is that adding @views of SparseArrays produces a dense array.
https://github.com/JuliaLang/julia/blob/ec98087cbf19bac26f6be05df9282746b0cffe78/stdlib/LinearAlgebra/src/tridiag.jl#L207
https://github.com/JuliaLang/julia/blob/ec98087cbf19bac26f6be05df9282746b0cffe78/stdlib/LinearAlgebra/src/LinearAlgebra.jl#L449-L453
julia> z = @view sparse([1, 2, 3])[1:3]
3-element view(::SparseVector{Int64, Int64}, 1:3) with eltype Int64:
1
2
3
julia> z + z
3-element Vector{Int64}:
2
4
6
This view usage was added in https://github.com/JuliaLang/julia/pull/42472 to deal with an implementation detail of SymTridiagonals where the off-diagonal has an additional element that should usually be ignored. A view was used to access only the actual elements of the off-diagonal without copying. This PR makes it so that sparse arrays just use a regular (copying) slice, until a better fix is found (I am not sure if there has been any discussion about the sparse @view sum thing).
wouldn't it be just better to fix SparseVectorView + SparseVectorView addition? @ViralBShah yet another argument for AbstractCompressedSparseVector?
Yes, it would be better, if that is something that should actually be done. I don't know if there are any weird considerations that need to be addressed when summing two views though, so I am fixing it this way for now.
Yeah we should just fix the views.
@ViralBShah so do i extract the AbstractCompressedVector from #201 and merge it? We can make the default for all operations on AbstractCompressedVectors return SparseVectors
@Wimmerer Any thoughts on your part?
@ViralBShah so do i extract the AbstractCompressedVector from #201 and merge it? We can make the default for all operations on
AbstractCompressedVectors returnSparseVectors
@SobhanMP Yes.
Codecov Report
Merging #225 (984209f) into main (f51b64c) will increase coverage by
0.01%. The diff coverage is100.00%.
@@ Coverage Diff @@
## main #225 +/- ##
==========================================
+ Coverage 92.11% 92.12% +0.01%
==========================================
Files 11 11
Lines 7203 7204 +1
==========================================
+ Hits 6635 6637 +2
+ Misses 568 567 -1
| Impacted Files | Coverage Δ | |
|---|---|---|
| src/SparseArrays.jl | 100.00% <100.00%> (ø) |
|
| src/linalg.jl | 84.58% <0.00%> (+0.08%) |
:arrow_up: |
:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more