JuliaLang / julia

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

QR Factorization breaks for complex matrix when adding a .H property in juliapro1.0.1.1 #29707

Closed mattcbro closed 6 years ago

mattcbro commented 6 years ago

Adding transpose operations to the base property breaks QR.

using LinearAlgebra
function Base.getproperty(x::AbstractMatrix, name::Symbol)
    if name === :T
        return transpose(x)
    elseif name === :H # can also do this, though not sure why we'd want to overload with `'`
        return adjoint(x)
    else
        return getfield(x, name)
    end
end
A = randn(5,3) + 1im * randn(5,3)

This nicely defines A.H as the conjugate transpose (adjoint) of A and A.T as it's transpose. However it seems to interfere with the matrix factorizations, particularly qr. If I do,

F = qr(A)

I get the error shown below. I do know that .H is used in the Hessenburg decomposition, so perhaps that's where the interference lies. I suspect this is not a bug per se but some sort of name clash.

julia> F = qr(A) LinearAlgebra.QRCompactWY{Complex{Float64},Array{Complex{Float64},2}} Q factor: 5×5 LinearAlgebra.QRCompactWYQ{Complex{Float64},Array{Complex{Float64},2}}: Error showing value of type LinearAlgebra.QRCompactWY{Complex{Float64},Array{Complex{Float64},2}}: ERROR: MethodError: no method matching strides(::Transpose{Complex{Float64},LinearAlgebra.QRCompactWYQ{Complex{Float64},Array{Complex{Float64},2}}}) Closest candidates are: strides(::SubArray) at subarray.jl:264 strides(::Base.CodeUnits) at strings/basic.jl:696 strides(::PermutedDimsArray{T,N,perm,iperm,AA} where AA<:AbstractArray where iperm) where {T, N, perm} at permuteddimsarray.jl:62 ... Stacktrace: [1] stride(::Transpose{Complex{Float64},LinearAlgebra.QRCompactWYQ{Complex{Float64},Array{Complex{Float64},2}}}, ::Int64) at ./abstractarray.jl:342 [2] stride1(::Transpose{Complex{Float64},LinearAlgebra.QRCompactWYQ{Complex{Float64},Array{Complex{Float64},2}}}) at /home/julia/worker/juliapro-release-centos7-0_6/build/tmp_julia/share/julia/stdlib/v1.0/LinearAlgebra/src/LinearAlgebra.jl:188 [3] _chkstride1 at /home/julia/worker/juliapro-release-centos7-0_6/build/tmp_julia/share/julia/stdlib/v1.0/LinearAlgebra/src/LinearAlgebra.jl:194 [inlined] (repeats 2 times) [4] chkstride1 at /home/julia/worker/juliapro-release-centos7-0_6/build/tmp_julia/share/julia/stdlib/v1.0/LinearAlgebra/src/LinearAlgebra.jl:192 [inlined] [5] gemqrt!(::Char, ::Char, ::Array{Complex{Float64},2}, ::Transpose{Complex{Float64},LinearAlgebra.QRCompactWYQ{Complex{Float64},Array{Complex{Float64},2}}}, ::Array{Complex{Float64},1}) at /home/julia/worker/juliapro-release-centos7-0_6/build/tmp_julia/share/julia/stdlib/v1.0/LinearAlgebra/src/lapack.jl:2831 [6] lmul!(::LinearAlgebra.QRCompactWYQ{Complex{Float64},Array{Complex{Float64},2}}, ::Array{Complex{Float64},1}) at /home/julia/worker/juliapro-release-centos7-0_6/build/tmp_julia/share/julia/stdlib/v1.0/LinearAlgebra/src/qr.jl:522 [7] getindex(::LinearAlgebra.QRCompactWYQ{Complex{Float64},Array{Complex{Float64},2}}, ::Int64, ::Int64) at /home/julia/worker/juliapro-release-centos7-0_6/build/tmp_julia/share/julia/stdlib/v1.0/LinearAlgebra/src/qr.jl:517 [8] isassigned(::LinearAlgebra.QRCompactWYQ{Complex{Float64},Array{Complex{Float64},2}}, ::Int64, ::Int64) at ./abstractarray.jl:351 [9] alignment(::IOContext{REPL.Terminals.TTYTerminal}, ::LinearAlgebra.QRCompactWYQ{Complex{Float64},Array{Complex{Float64},2}}, ::UnitRange{Int64}, ::UnitRange{Int64}, ::Int64, ::Int64, ::Int64) at ./arrayshow.jl:67 [10] print_matrix(::IOContext{REPL.Terminals.TTYTerminal}, ::LinearAlgebra.QRCompactWYQ{Complex{Float64},Array{Complex{Float64},2}}, ::String, ::String, ::String, ::String, ::String, ::String, ::Int64, ::Int64) at ./arrayshow.jl:186 [11] print_matrix at ./arrayshow.jl:159 [inlined] [12] print_array at ./arrayshow.jl:308 [inlined] [13] show(::IOContext{REPL.Terminals.TTYTerminal}, ::MIME{Symbol("text/plain")}, ::LinearAlgebra.QRCompactWYQ{Complex{Float64},Array{Complex{Float64},2}}) at ./arrayshow.jl:345 [14] show(::IOContext{REPL.Terminals.TTYTerminal}, ::MIME{Symbol("text/plain")}, ::LinearAlgebra.QRCompactWY{Complex{Float64},Array{Complex{Float64},2}}) at /home/julia/worker/juliapro-release-centos7-0_6/build/tmp_julia/share/julia/stdlib/v1.0/LinearAlgebra/src/qr.jl:400 [15] display(::REPL.REPLDisplay, ::MIME{Symbol("text/plain")}, ::Any) at /home/julia/worker/juliapro-release-centos7-0_6/build/tmp_julia/share/julia/stdlib/v1.0/REPL/src/REPL.jl:131 [16] display(::REPL.REPLDisplay, ::Any) at /home/julia/worker/juliapro-release-centos7-0_6/build/tmp_julia/share/julia/stdlib/v1.0/REPL/src/REPL.jl:135 [17] display(::Any) at ./multimedia.jl:287 [18] #7 at /home/matt/programs/JuliaPro-1.0.1.1/JuliaPro/pkgs-1.0.1.1/packages/Media/Lrdeg/src/compat.jl:28 [inlined] [19] hookless(::getfield(Media, Symbol("##7#8")){LinearAlgebra.QRCompactWY{Complex{Float64},Array{Complex{Float64},2}}}) at /home/matt/programs/JuliaPro-1.0.1.1/JuliaPro/pkgs-1.0.1.1/packages/Media/Lrdeg/src/compat.jl:14 [20] render(::Media.NoDisplay, ::LinearAlgebra.QRCompactWY{Complex{Float64},Array{Complex{Float64},2}}) at /home/matt/programs/JuliaPro-1.0.1.1/JuliaPro/pkgs-1.0.1.1/packages/Media/Lrdeg/src/compat.jl:27 [21] render(::LinearAlgebra.QRCompactWY{Complex{Float64},Array{Complex{Float64},2}}) at /home/matt/programs/JuliaPro-1.0.1.1/JuliaPro/pkgs-1.0.1.1/packages/Media/Lrdeg/src/system.jl:160 [22] display(::Media.DisplayHook, ::LinearAlgebra.QRCompactWY{Complex{Float64},Array{Complex{Float64},2}}) at /home/matt/programs/JuliaPro-1.0.1.1/JuliaPro/pkgs-1.0.1.1/packages/Media/Lrdeg/src/compat.jl:9 [23] display(::Any) at ./multimedia.jl:287 [24] #invokelatest#1 at ./essentials.jl:697 [inlined] [25] invokelatest at ./essentials.jl:696 [inlined] [26] print_response(::IO, ::Any, ::Any, ::Bool, ::Bool, ::Any) at /home/julia/worker/juliapro-release-centos7-0_6/build/tmp_julia/share/julia/stdlib/v1.0/REPL/src/REPL.jl:154 [27] print_response(::REPL.AbstractREPL, ::Any, ::Any, ::Bool, ::Bool) at /home/julia/worker/juliapro-release-centos7-0_6/build/tmp_julia/share/julia/stdlib/v1.0/REPL/src/REPL.jl:139 [28] (::getfield(REPL, Symbol("#do_respond#40")){Bool,getfield(Atom, Symbol("##164#165")),REPL.LineEditREPL,REPL.LineEdit.Prompt})(::Any, ::Any, ::Any) at /home/julia/worker/juliapro-release-centos7-0_6/build/tmp_julia/share/julia/stdlib/v1.0/REPL/src/REPL.jl:708 [29] #invokelatest#1 at ./essentials.jl:697 [inlined] [30] invokelatest at ./essentials.jl:696 [inlined] [31] run_interface(::REPL.Terminals.TextTerminal, ::REPL.LineEdit.ModalInterface, ::REPL.LineEdit.MIState) at /home/julia/worker/juliapro-release-centos7-0_6/build/tmp_julia/share/julia/stdlib/v1.0/REPL/src/LineEdit.jl:2261 [32] run_frontend(::REPL.LineEditREPL, ::REPL.REPLBackendRef) at /home/julia/worker/juliapro-release-centos7-0_6/build/tmp_julia/share/julia/stdlib/v1.0/REPL/src/REPL.jl:1029 [33] run_repl(::REPL.AbstractREPL, ::Any) at /home/julia/worker/juliapro-release-centos7-0_6/build/tmp_julia/share/julia/stdlib/v1.0/REPL/src/REPL.jl:191 [34] (::getfield(Base, Symbol("##719#721")){Bool,Bool,Bool,Bool})(::Module) at ./logging.jl:311 [35] #invokelatest#1 at ./essentials.jl:697 [inlined] [36] invokelatest at ./essentials.jl:696 [inlined] [37] macro expansion at ./logging.jl:308 [inlined] [38] run_main_repl(::Bool, ::Bool, ::Bool, ::Bool, ::Bool) at ./client.jl:330 [39] exec_options(::Base.JLOptions) at ./client.jl:242 [40] _start() at ./client.jl:421

StefanKarpinski commented 6 years ago

This seems like rather serious type piracy, no?

KristofferC commented 6 years ago

Will break any AbstractMatrix that tries to access a field with one of those names. This is not expected to just work.

mattcbro commented 6 years ago

Will break any AbstractMatrix that tries to access a field with one of those names. This is not expected to just work.

Is there a list of such field names? Moreover is the type returned by qr actually an AbstractMatrix, and why would it use a Hessenberg matrix field .H? Maybe I could pick some field names for transposition that are more unique.

KristofferC commented 6 years ago

The list is unbounded since this is done to an abstract type. Type piracy on an abstract type like this will always potentially break code.

mattcbro commented 6 years ago

The list is unbounded since this is done to an abstract type. Type piracy on an abstract type like this will always potentially break code.

Is the code breaking due to name conflicts with existing field names? Furthermore, if so, I presume that if I get it to work for one field name, it potentially could conflict with later updates?

Hmm just found out what the term "type piracy" means too.