Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix sparse constructor when Tridiagonal/SymTridiagonal are empty #42574

Merged
merged 7 commits into from
Oct 15, 2021

Conversation

mcognetta
Copy link
Contributor

Constructing a sparse matrix from empty structured matrices works for some, but not all, structured matrix types. This PR fixes the cases of Tridiagonal and SymTridiagonal. Diagonal and Bidiagonal already work as expected.


Currently:

julia> sparse(Diagonal(zeros(0, 0)))
0×0 SparseMatrixCSC{Float64, Int64} with 0 stored entries

julia> sparse(Bidiagonal(zeros(0, 0), :U))
0×0 SparseMatrixCSC{Float64, Int64} with 0 stored entries

julia> sparse(Tridiagonal(zeros(0, 0)))
ERROR: ArgumentError: invalid Array dimensions
Stacktrace:
 [1] Array
   @ ./boot.jl:452 [inlined]
 [2] SparseMatrixCSC{Float64, Int64}(T::Tridiagonal{Float64, Vector{Float64}})
   @ SparseArrays ~/github/julia-dev/stdlib/SparseArrays/src/sparsematrix.jl:575
 [3] SparseMatrixCSC
   @ ~/github/julia-dev/usr/share/julia/stdlib/v1.8/SparseArrays/src/sparsematrix.jl:563 [inlined]
 [4] sparse(T::Tridiagonal{Float64, Vector{Float64}})
   @ SparseArrays ~/github/julia-dev/stdlib/SparseArrays/src/sparsematrix.jl:760
 [5] top-level scope
   @ REPL[10]:1

julia> sparse(SymTridiagonal(zeros(0, 0)))
ERROR: ArgumentError: invalid Array dimensions
Stacktrace:
 [1] Array
   @ ./boot.jl:452 [inlined]
 [2] SparseMatrixCSC{Float64, Int64}(T::SymTridiagonal{Float64, Vector{Float64}})
   @ SparseArrays ~/github/julia-dev/stdlib/SparseArrays/src/sparsematrix.jl:606
 [3] SparseMatrixCSC
   @ ~/github/julia-dev/usr/share/julia/stdlib/v1.8/SparseArrays/src/sparsematrix.jl:594 [inlined]
 [4] sparse(T::SymTridiagonal{Float64, Vector{Float64}})
   @ SparseArrays ~/github/julia-dev/stdlib/SparseArrays/src/sparsematrix.jl:758
 [5] top-level scope
   @ REPL[11]:1

This PR:

julia> sparse(Diagonal(zeros(0, 0)))
0×0 SparseMatrixCSC{Float64, Int64} with 0 stored entries

julia> sparse(Bidiagonal(zeros(0, 0), :U))
0×0 SparseMatrixCSC{Float64, Int64} with 0 stored entries

julia> sparse(Tridiagonal(zeros(0, 0)))
0×0 SparseMatrixCSC{Float64, Int64} with 0 stored entries

julia> sparse(SymTridiagonal(zeros(0, 0)))
0×0 SparseMatrixCSC{Float64, Int64} with 0 stored entries

@dkarrasch dkarrasch added the sparse Sparse arrays label Oct 10, 2021
stdlib/SparseArrays/src/sparsematrix.jl Outdated Show resolved Hide resolved
stdlib/SparseArrays/src/sparsematrix.jl Outdated Show resolved Hide resolved
@mcognetta
Copy link
Contributor Author

Added a fix and tests for the 1x1 case.

Copy link
Member

@dkarrasch dkarrasch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's construct the SparseMatrixCSC directly also in the m=1 case. While you're at it, could you please fix the new line 659 by

SparseMatrixCSC(m, m, Vector(Ti(1):Ti(m+1)), Vector(Ti(1):Ti(m)), Vector{Tv}(D.diag))

otherwise it doesn't respect the index type Ti.

stdlib/SparseArrays/src/sparsematrix.jl Outdated Show resolved Hide resolved
stdlib/SparseArrays/src/sparsematrix.jl Outdated Show resolved Hide resolved
@mcognetta
Copy link
Contributor Author

mcognetta commented Oct 14, 2021

Let's construct the SparseMatrixCSC directly also in the m=1 case. While you're at it, could you please fix the new line 659 by

SparseMatrixCSC(m, m, Vector(Ti(1):Ti(m+1)), Vector(Ti(1):Ti(m)), Vector{Tv}(D.diag))

otherwise it doesn't respect the index type Ti.

Fixed. Added a test for the SparseMatrixCSC{Tv, Ti}(::Diagonal) case as well to ensure Ti is respected.

@dkarrasch dkarrasch merged commit b3c268c into JuliaLang:master Oct 15, 2021
@mcognetta mcognetta deleted the sparse_matrix_empty_structured branch October 15, 2021 20:36
LilithHafner pushed a commit to LilithHafner/julia that referenced this pull request Feb 22, 2022
LilithHafner pushed a commit to LilithHafner/julia that referenced this pull request Mar 8, 2022
@dkarrasch dkarrasch added the backport 1.6 Change should be backported to release-1.6 label Oct 13, 2022
@dkarrasch
Copy link
Member

This should be backported to v1.6.

KristofferC pushed a commit that referenced this pull request Dec 21, 2022
…42574)

Co-authored-by: Daniel Karrasch <[email protected]>
(cherry picked from commit b3c268c)
KristofferC pushed a commit that referenced this pull request Dec 21, 2022
…42574)

Co-authored-by: Daniel Karrasch <[email protected]>
(cherry picked from commit b3c268c)
KristofferC pushed a commit that referenced this pull request Dec 21, 2022
…42574)

Co-authored-by: Daniel Karrasch <[email protected]>
(cherry picked from commit b3c268c)
staticfloat pushed a commit that referenced this pull request Dec 23, 2022
KristofferC pushed a commit that referenced this pull request Oct 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 1.6 Change should be backported to release-1.6 sparse Sparse arrays
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants