JuliaLang / julia

The Julia Programming Language
https://julialang.org/
MIT License
45.13k stars 5.43k forks source link

Symmetric(::Symmetric) and Hermitian(::Hermitian) with bug #30813

Open KlausC opened 5 years ago

KlausC commented 5 years ago

I think it is common understanding, that for X::AbstractMatrix.

  1. Symmetric(X, uplo) == Symmetric(Matrix(X), uplo)
  2. Symmetric(X) == Symmetric(X, :U)

Actually we observe


julia> VERSION
v"1.2.0-DEV.212"

julia> A = [1 2; 3 4]
2×2 Array{Int64,2}:
 1  2
 3  4
julia> X = Symmetric(A, :L)
2×2 Symmetric{Int64,Array{Int64,2}}:
 1  3
 3  4
julia> Symmetric(X) == Symmetric(X, :U) # result contradicts 2.
ERROR: ArgumentError: Cannot construct Symmetric; uplo doesn't match
Stacktrace:
 [1] Symmetric(::Symmetric{Int64,Array{Int64,2}}, ::Symbol) at
      /home/julia/julia/usr/share/julia/stdlib/v1.2/LinearAlgebra/src/symmetric.jl:162
 [2] top-level scope at REPL[87]:1

julia> Symmetric(X) == Symmetric(Matrix(X))
true
julia> Symmetric(Matrix(X), :U) # result contradicts 1.
2×2 Symmetric{Int64,Array{Int64,2}}:
 1  3
 3  4
julia> Symmetric(Matrix(X), :L)
2×2 Symmetric{Int64,Array{Int64,2}}:
 1  3
 3  4

I think, we should have Symmetric(X::Symmetric, uplo::Symbol=:U) = X independent of uplo and X.uplo.

The same is true for Hermitian

andreasnoack commented 5 years ago

Would you just ignore the uplo argument?

fredrikekre commented 5 years ago

Ref https://github.com/JuliaLang/julia/issues/21616#issuecomment-298059168, #21622 and #22264

KlausC commented 5 years ago

Yes, I would ignore the uplo argument.

The argument of "correct aliasing" given in #21616 (comment) is just another word for sticking to the uplo value of the outer constructor. IMHO that is a much less valuable property than the given rules 1. and 2.

Rule 1 is well-definedness of the functions Symmetrix(., uplo) (if x == y => f(x) == f(y)) Rule 2 is the meaning of the default value

KlausC commented 5 years ago

@fredrikekre I read Ref #21616 (comment) carefully, but I still don't understand, why Symmetric(A, :U).uplo == 'U' is more important than Symmetric(A) == Symmetric(Matrix(A)) for all abstract matrices A. The second one is a general principle, while the first one looks more like a random technical restriction.

Look also at this text, which tries to present a fresh view to the meaning of wrappers: universal matrix wrappers.