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

Cannot add SymTridiagonal matrices based on sparse vectors #942

Closed
eschnett opened this issue Aug 15, 2022 · 9 comments · Fixed by JuliaSparse/SparseArrays.jl#416
Closed
Labels
regression Regression in behavior compared to a previous version

Comments

@eschnett
Copy link
Contributor

This fails:

julia> using LinearAlgebra
julia> using SparseArrays
julia> A = SymTridiagonal(sparsevec(Int[1]), sparsevec(Int[]))
julia> A + A
ERROR: MethodError: no method matching SymTridiagonal(::SparseVector{Int64, Int64}, ::Vector{Int64})

I assume the problem is that SymTridiagonal matrices accidentally convert the internal representation of the off-diagonal elements to a dense vector.

I am using

julia> versioninfo()
Julia Version 1.9.0-DEV.1130
Commit c80316e125* (2022-08-15 13:05 UTC)
Platform Info:
  OS: macOS (x86_64-apple-darwin21.6.0)
  CPU: 16 × Intel(R) Core(TM) i9-9980HK CPU @ 2.40GHz
  WORD_SIZE: 64
  LIBM: libopenlibm
  LLVM: libLLVM-14.0.5 (ORCJIT, skylake)
  Threads: 8 on 16 virtual cores
@jishnub
Copy link
Collaborator

jishnub commented Aug 16, 2022

This works on v1.7.3, but not on higher versions

julia> A + A
1×1 SymTridiagonal{Int64, SparseVector{Int64, Int64}}:
 2

julia> versioninfo()
Julia Version 1.7.3
Commit 742b9abb4d (2022-05-06 12:58 UTC)
Platform Info:
  OS: Linux (x86_64-pc-linux-gnu)
  CPU: 11th Gen Intel(R) Core(TM) i5-1135G7 @ 2.40GHz
  WORD_SIZE: 64
  LIBM: libopenlibm
  LLVM: libLLVM-12.0.1 (ORCJIT, tigerlake)
Environment:
  JULIA_EDITOR = subl

@inkydragon
Copy link
Member

@inkydragon inkydragon added the regression Regression in behavior compared to a previous version label Aug 16, 2022
@mcognetta
Copy link
Contributor

The issue is that adding @views of SparseArrays makes it dense.

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

I will see what can be done quickly.

@mcognetta
Copy link
Contributor

Ok I have a fix but I am not quite sure how to commit to SparseArrays now that it is its own package.

@jishnub
Copy link
Collaborator

jishnub commented Aug 16, 2022

Just open a PR on the SparseArrays repo?

@mcognetta
Copy link
Contributor

I meant that a copy of SparseArrays still shows up in the Julia stdlib directory when opening it in VSCode, etc. but changes to it didn't get tracked by git.

Anyway: JuliaSparse/SparseArrays.jl#225

@inkydragon
Copy link
Member

@mcognetta After the pr is merged, just ask someone to BumpStdlibs. like JuliaLang/julia#46266

@SobhanMP
Copy link
Contributor

Is there any reason the diagonals of SymTridiagonal cannot have different types?

@mcognetta
Copy link
Contributor

I think that it is possible, but that is a much bigger change than necessary to fix this (all structured matrix types would have to be updated, and Tridiagonal could have 3 different types. I haven't heard of much need for this, but that's not necessarily an end-all reason for not doing it).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
regression Regression in behavior compared to a previous version
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants