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

special matrix types should define constructors for AbstractMatrix not Matrix #165

Closed
jakebolewski opened this issue Feb 1, 2015 · 5 comments
Assignees

Comments

@jakebolewski
Copy link
Member

Right now this is inconsistently applied:

julia> t = SymTridiagonal(r)
5x5 Base.LinAlg.SymTridiagonal{Float64}:
 2.78888    0.578169   0.0        0.0       0.0
 0.578169   1.51863   -1.14248    0.0       0.0
 0.0       -1.14248    2.33564   -0.598465  0.0
 0.0        0.0       -0.598465   5.91828   0.540706
 0.0        0.0        0.0        0.540706  2.06008

julia> Bidiagonal(t)
ERROR: ArgumentError: Matrix cannot be represented as Bidiagonal
 in call at base.jl:35

but the following works:

julia> LowerTriangular(t)
5x5 Base.LinAlg.LowerTriangular{Float64,Base.LinAlg.SymTridiagonal{Float64}}:
 2.78888    0.0       0.0       0.0       0.0
 0.578169   1.51863   0.0       0.0       0.0
 0.0       -1.14248   2.33564   0.0       0.0
 0.0        0.0      -0.598465  5.91828   0.0
 0.0        0.0       0.0       0.540706  2.06008
@jakebolewski jakebolewski self-assigned this Feb 1, 2015
@jakebolewski
Copy link
Member Author

Along these lines, not every special matrix type defines a Matrix constructor.

julia> Bidiagonal(rand(5,5))
ERROR: MethodError: `convert` has no method matching convert(::Type{Base.LinAlg.Bidiagonal{T}}, ::Array{Float64,2})

Edit: I was missing the upper / lower boolean flag, so this in fact defined.

@andreasnoack
Copy link
Member

Bidiagonal and (Sym)Tridiagonal are different from Symmetric, Hermitian and Triangular. The latter are views so I don't think it is expected that the former should behave the same. We could define constructors for Bidiagonal and (Sym)Tridagonal that copied the diagonals of the AbstractMatrix into new buffers. Would that be better?

@tkelman
Copy link

tkelman commented Nov 19, 2015

That would make sense to me as the convert method that the constructor would also fall back to. Should probably error if any elements not representable in the structured type are nonzero? Though that could be very expensive to check. Should these be analogous to checked integer conversions ("input does not fit in destination type") or masking operations that ignore some data so can change the meaning? Or maybe the constructor should do one and convert should do the other?

@oxinabox
Copy link
Contributor

oxinabox commented Jan 8, 2020

Should probably error if any elements not representable in the structured type are nonzero?

That is what we were doing as of v0.4 release
https://github.com/JuliaLang/julia/blob/v0.4.0/base/linalg/tridiag.jl#L28
Which is really what this issue is about.
Though its not titled very clearly.
It is not a MethodError the method is there, it just applies an extra check that isn't there in the other casess.

And its still with us today.
Breaking the new tests in JuliaLang/julia#34308

Though now days SymTridiaginal errors for all nonsymmetric dense or otherwise.
And I think the Bidiagonal example was odd when this was openned.
Since (then and now) if you pass in the 2nd argument of which bidiagonal it works fine
https://github.com/JuliaLang/julia/blob/v0.4.0/base/linalg/bidiag.jl#L34
But if you don't then it errors for structured (and method errors for unstructured)

So new examples may be need:

Diagonal still has odd behavour:

  • Diagonal(rand(4,4)) works fine and just cuts out the diagonal, ignoring nondiagonal elements.
  • Diagonal(LowerTriangular(rand(4,4))) errors because the matrix has zeros on the non-diagonal

etc

@brenhinkeller
Copy link
Contributor

brenhinkeller commented Nov 16, 2022

Things are looking pretty consistent here now in the far future, so I'm going to go ahead and close this, but feel free to reopen if that's a mistake!

julia> Diagonal(rand(4,4))
4×4 Diagonal{Float64, Vector{Float64}}:
 0.873568   ⋅         ⋅          ⋅
  ⋅        0.497213   ⋅          ⋅
  ⋅         ⋅        0.0724948   ⋅
  ⋅         ⋅         ⋅         0.0924769

julia> Diagonal(rand(4,4))^C

julia> Diagonal(rand(4,4))
4×4 Diagonal{Float64, Vector{Float64}}:
 0.31052   ⋅         ⋅         ⋅
  ⋅       0.030493   ⋅         ⋅
  ⋅        ⋅        0.290663   ⋅
  ⋅        ⋅         ⋅        0.983586

julia> Diagonal(LowerTriangular(rand(4,4)))
4×4 Diagonal{Float64, Vector{Float64}}:
 0.833831   ⋅         ⋅         ⋅
  ⋅        0.844355   ⋅         ⋅
  ⋅         ⋅        0.881856   ⋅
  ⋅         ⋅         ⋅        0.377466

julia> Tridiagonal(rand(4,4))
4×4 Tridiagonal{Float64, Vector{Float64}}:
 0.705242   0.598533   ⋅         ⋅
 0.0438911  0.918191  0.219985   ⋅
  ⋅         0.84228   0.969106  0.14297
  ⋅          ⋅        0.751401  0.38486

julia> Bidiagonal(rand(4,4),:U)
4×4 Bidiagonal{Float64, Vector{Float64}}:
 0.588485  0.863636    ⋅         ⋅
  ⋅        0.0831775  0.896926   ⋅
  ⋅         ⋅         0.907708  0.341017
  ⋅         ⋅          ⋅        0.0906147

julia> versioninfo()
Julia Version 1.8.2
Commit 36034abf260 (2022-09-29 15:21 UTC)
Platform Info:
  OS: macOS (arm64-apple-darwin21.3.0)
  CPU: 10 × Apple M1 Max
  WORD_SIZE: 64
  LIBM: libopenlibm
  LLVM: libLLVM-13.0.1 (ORCJIT, apple-m1)
  Threads: 1 on 8 virtual cores

@KristofferC KristofferC transferred this issue from JuliaLang/julia Nov 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants