Skip to content

Commit

Permalink
Revert *support Inv(Tri) (#338)
Browse files Browse the repository at this point in the history
  • Loading branch information
@DanielVandH
DanielVandH authored Jul 11, 2024
1 parent 0bfe6e5 commit 66ae7a2
Show file tree
Hide file tree
Showing 3 changed files with 2 additions and 31 deletions.
2 changes: 1 addition & 1 deletion Project.toml
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
name = "LazyArrays"
uuid = "5078a376-72f3-5289-bfd5-ec5146d43c02"
version = "2.1.7"
version = "2.1.8"

[deps]
ArrayLayouts = "4c555306-a7a7-4459-81d9-ec55ddd5c99a"
Expand Down
15 changes: 1 addition & 14 deletions src/linalg/inv.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -184,17 +184,4 @@ getindex(L::ApplyMatrix{<:Any,typeof(/)}, k::Integer, ::Colon) = permutedims(L.a
getindex(L::ApplyMatrix{<:Any,typeof(/)}, k::Integer, j::Integer) = L[k,:][j]


inv_layout(::LazyLayouts, _, A) = ApplyArray(inv, A)

###
# row/colsupport triangular
###
function colsupport(lay::AbstractInvLayout{<:TriangularLayout}, A, j)
B, = arguments(lay, A)
return colsupport(B, j)
end

function rowsupport(lay::AbstractInvLayout{<:TriangularLayout}, A, k)
B, = arguments(lay, A)
return rowsupport(B, k)
end
inv_layout(::LazyLayouts, _, A) = ApplyArray(inv, A)
16 changes: 0 additions & 16 deletions test/ldivtests.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -181,20 +181,4 @@ end
@test Z \ Y ≈ [-2.0 1.0; 1.5 -0.5]
end

@testset "Issue #329" begin
for op in (UpperTriangular, UnitUpperTriangular)
A = UpperTriangular(ApplyArray(inv, rand(5, 5)))
B = inv(A)
@test colsupport.(Ref(B), 1:5) == Base.OneTo.(1:5)
@test rowsupport.(Ref(B), 1:5) == range.(1:5, 5)
end

for op in (LowerTriangular, UnitLowerTriangular)
A = LowerTriangular(ApplyArray(inv, rand(15, 15)))
B = inv(A)
@test colsupport.(Ref(B), 1:15) == range.(1:15, 15)
@test rowsupport.(Ref(B), 1:15) == Base.OneTo.(1:15)
end
end

end # module

8 comments on commit 66ae7a2

@dlfivefifty
Copy link
Member

Choose a reason for hiding this comment

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

@JuliaRegistrator register

@JuliaRegistrator
Copy link

Choose a reason for hiding this comment

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

Registration pull request created: JuliaRegistries/General/110872

Tip: Release Notes

Did you know you can add release notes too? Just add markdown formatted text underneath the comment after the text
"Release notes:" and it will be added to the registry PR, and if TagBot is installed it will also be added to the
release that TagBot creates. i.e.

@JuliaRegistrator register

Release notes:

## Breaking changes

- blah

To add them here just re-invoke and the PR will be updated.

Tagging

After the above pull request is merged, it is recommended that a tag is created on this repository for the registered package version.

This will be done automatically if the Julia TagBot GitHub Action is installed, or can be done manually through the github interface, or via:

git tag -a v2.1.8 -m "<description of version>" 66ae7a21981d83b30e5686fc0bfa3d8b2ca6cb28
git push origin v2.1.8

@dlfivefifty
Copy link
Member

Choose a reason for hiding this comment

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

I think something like first(rowsupport(A,k)):size(A,2) works ...

@DanielVandH
Copy link
Contributor Author

Choose a reason for hiding this comment

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

function colsupport(lay::AbstractInvLayout{TriLay}, A, j) where {S,TriLay<:TriangularLayout{S}}
    B, = arguments(lay, A)
    if S == 'U'
        return firstindex(B, 2):(j - firstindex(B, 2) + 1)
    else # S == 'L' 
        return j:size(B, 2)
    end 
end 
function rowsupport(lay::AbstractInvLayout{TriLay}, A, k) where {S,TriLay<:TriangularLayout{S}}
    B, = arguments(lay, A) 
    if S == 'U' 
        return k:size(B, 1) 
    else # S == 'L'
        return firstindex(B, 1):(k - firstindex(B, 1) + 1)
    end 
end

would probably do it

@dlfivefifty
Copy link
Member

Choose a reason for hiding this comment

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

Actually I think we should just do

colsupport(::AbstractInvLayout{<:TriangularLayout}, A, j) = colsupport(TriangularLayout{UnknownLayout}(), A, j)

@DanielVandH
Copy link
Contributor Author

Choose a reason for hiding this comment

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

TriangularLayout{UnknownLayout}() isn't a method so I assume you mean

function colsupport(::AbstractInvLayout{TriLay}, A, j) where {UPLO, UNIT, TriLay <: TriangularLayout{UPLO, UNIT}}
    return colsupport(TriangularLayout{UPLO, UNIT, UnknownLayout}(), A, j)
end
function rowsupport(::AbstractInvLayout{TriLay}, A, j) where {UPLO, UNIT, TriLay <: TriangularLayout{UPLO, UNIT}}
    return rowsupport(TriangularLayout{UPLO, UNIT, UnknownLayout}(), A, j)
end

This doesn't work for e.g.

julia>     U = UpperTriangular(ApplyArray(inv, rand(5, 5)))
5×5 UpperTriangular{Float64, ApplyArray{Float64, 2, typeof(inv), Tuple{Matrix{Float64}}}}:
 -0.997733   3.43753  -1.91513    0.568638  -0.222995
   ⋅        -5.70382   2.23719    2.09242   -0.393939
   ⋅          ⋅       -0.185788   3.06888   -0.183617
   ⋅          ⋅         ⋅        -0.814276   1.61954
   ⋅          ⋅         ⋅          ⋅         0.098716

julia>     invU = inv(U)
inv(5×5 UpperTriangular{Float64, ApplyArray{Float64, 2, typeof(inv), Tuple{Matrix{Float64}}}}):
 Error showing value of type ApplyArray{Float64, 2, typeof(inv), Tuple{UpperTriangular{Float64, ApplyArray{Float64, 2, typeof(inv), Tuple{Matrix{Float64}}}}}}:
ERROR: MethodError: no method matching triangulardata(::ApplyArray{Float64, 2, typeof(inv), Tuple{UpperTriangular{Float64, ApplyArray{Float64, 2, typeof(inv), Tuple{Matrix{Float64}}}}}})

Closest candidates are:
  triangulardata(::SubArray{<:Any, 2, <:Any, <:Tuple{var"#s2", var"#s1"} where {var"#s2"<:Union{Base.OneTo, Base.Slice}, var"#s1"<:Union{Base.OneTo, Base.Slice}}})
   @ ArrayLayouts C:\Users\danjv\.julia\packages\ArrayLayouts\31idh\src\memorylayout.jl:522
  triangulardata(::Transpose)
   @ ArrayLayouts C:\Users\danjv\.julia\packages\ArrayLayouts\31idh\src\memorylayout.jl:521
  triangulardata(::Adjoint)
   @ ArrayLayouts C:\Users\danjv\.julia\packages\ArrayLayouts\31idh\src\memorylayout.jl:520
  ...

Stacktrace:
  [1] colsupport(::TriangularLayout{'U', 'N', UnknownLayout}, A::ApplyArray{Float64, 2, typeof(inv), Tuple{UpperTriangular{Float64, ApplyArray{Float64, 2, typeof(inv), Tuple{…}}}}}, j::Int64)
    @ ArrayLayouts C:\Users\danjv\.julia\packages\ArrayLayouts\31idh\src\triangular.jl:2
  [2] colsupport(::LazyArrays.InvLayout{TriangularLayout{'U', 'N', LazyLayout}}, A::ApplyArray{Float64, 2, typeof(inv), Tuple{UpperTriangular{Float64, ApplyArray{…}}}}, j::Int64)
    @ LazyArrays c:\Users\danjv\.julia\dev\LazyArrays.jl\src\linalg\inv.jl:207
  [3] colsupport(A::ApplyArray{Float64, 2, typeof(inv), Tuple{UpperTriangular{Float64, ApplyArray{Float64, 2, typeof(inv), Tuple{Matrix{Float64}}}}}}, j::Int64)
    @ ArrayLayouts C:\Users\danjv\.julia\packages\ArrayLayouts\31idh\src\memorylayout.jl:662
  [4] layout_replace_in_print_matrix(::LazyArrays.InvLayout{TriangularLayout{…}}, A::ApplyArray{Float64, 2, typeof(inv), Tuple{…}}, i::Int64, j::Int64, s::String)
    @ ArrayLayouts C:\Users\danjv\.julia\packages\ArrayLayouts\31idh\src\ArrayLayouts.jl:377
  [5] replace_in_print_matrix(A::ApplyArray{Float64, 2, typeof(inv), Tuple{UpperTriangular{Float64, ApplyArray{Float64, 2, typeof(inv), Tuple{Matrix{…}}}}}}, i::Int64, j::Int64, s::String)
    @ ArrayLayouts C:\Users\danjv\.julia\packages\ArrayLayouts\31idh\src\ArrayLayouts.jl:380
  [6] print_matrix_row(io::IOContext{Base.TTY}, X::AbstractVecOrMat, A::Vector{Tuple{Int64, Int64}}, i::Int64, cols::Vector{Int64}, sep::String, idxlast::Int64)
    @ Base .\arrayshow.jl:120
  [7] axes_print_matrix_row (repeats 2 times)
    @ C:\Users\danjv\.julia\packages\FillArrays\eOEVm\src\FillArrays.jl:725 [inlined]
  [8] print_matrix_row(io::IOContext{Base.TTY}, X::ApplyArray{Float64, 2, typeof(inv), Tuple{…}}, A::Vector{Tuple{…}}, i::Int64, cols::Vector{Int64}, sep::String, idxlast::Int64)
    @ ArrayLayouts C:\Users\danjv\.julia\packages\ArrayLayouts\31idh\src\ArrayLayouts.jl:395
  [9] _print_matrix(io::IOContext{…}, X::AbstractVecOrMat, pre::String, sep::String, post::String, hdots::String, vdots::String, ddots::String, hmod::Int64, vmod::Int64, rowsA::UnitRange{…}, colsA::UnitRange{…})
    @ Base .\arrayshow.jl:213
 [10] print_matrix(io::IOContext{…}, X::ApplyArray{…}, pre::String, sep::String, post::String, hdots::String, vdots::String, ddots::String, hmod::Int64, vmod::Int64)
    @ Base .\arrayshow.jl:171
 [11] print_matrix
    @ .\arrayshow.jl:171 [inlined]
 [12] print_array
    @ .\arrayshow.jl:358 [inlined]
 [13] show(io::IOContext{Base.TTY}, ::MIME{Symbol("text/plain")}, X::ApplyArray{Float64, 2, typeof(inv), Tuple{UpperTriangular{Float64, ApplyArray{Float64, 2, typeof(inv), Tuple{Matrix{…}}}}}})
    @ Base .\arrayshow.jl:399
 [14] (::REPL.var"#55#56"{REPL.REPLDisplay{REPL.LineEditREPL}, MIME{Symbol("text/plain")}, Base.RefValue{Any}})(io::Any)
    @ REPL C:\Users\danjv\.julia\juliaup\julia-1.10.4+0.x64.w64.mingw32\share\julia\stdlib\v1.10\REPL\src\REPL.jl:273
 [15] with_repl_linfo(f::Any, repl::REPL.LineEditREPL)
    @ REPL C:\Users\danjv\.julia\juliaup\julia-1.10.4+0.x64.w64.mingw32\share\julia\stdlib\v1.10\REPL\src\REPL.jl:569
 [16] display(d::REPL.REPLDisplay, mime::MIME{Symbol("text/plain")}, x::Any)
    @ REPL C:\Users\danjv\.julia\juliaup\julia-1.10.4+0.x64.w64.mingw32\share\julia\stdlib\v1.10\REPL\src\REPL.jl:259
 [17] display(d::REPL.REPLDisplay, x::Any)
    @ REPL C:\Users\danjv\.julia\juliaup\julia-1.10.4+0.x64.w64.mingw32\share\julia\stdlib\v1.10\REPL\src\REPL.jl:278
 [18] display(x::Any)
    @ Base.Multimedia .\multimedia.jl:340
 [19] #invokelatest#2
    @ .\essentials.jl:892 [inlined]
 [20] invokelatest
    @ .\essentials.jl:889 [inlined]
 [21] print_response(errio::IO, response::Any, show_value::Bool, have_color::Bool, specialdisplay::Union{Nothing, AbstractDisplay})
    @ REPL C:\Users\danjv\.julia\juliaup\julia-1.10.4+0.x64.w64.mingw32\share\julia\stdlib\v1.10\REPL\src\REPL.jl:315
 [22] (::REPL.var"#57#58"{REPL.LineEditREPL, Pair{Any, Bool}, Bool, Bool})(io::Any)
    @ REPL C:\Users\danjv\.julia\juliaup\julia-1.10.4+0.x64.w64.mingw32\share\julia\stdlib\v1.10\REPL\src\REPL.jl:284
 [23] with_repl_linfo(f::Any, repl::REPL.LineEditREPL)
    @ REPL C:\Users\danjv\.julia\juliaup\julia-1.10.4+0.x64.w64.mingw32\share\julia\stdlib\v1.10\REPL\src\REPL.jl:569
 [24] print_response(repl::REPL.AbstractREPL, response::Any, show_value::Bool, have_color::Bool)
    @ REPL C:\Users\danjv\.julia\juliaup\julia-1.10.4+0.x64.w64.mingw32\share\julia\stdlib\v1.10\REPL\src\REPL.jl:282
 [25] (::REPL.var"#do_respond#80"{Bool, Bool, REPL.var"#93#103"{…}, REPL.LineEditREPL, REPL.LineEdit.Prompt})(s::REPL.LineEdit.MIState, buf::Any, ok::Bool)
    @ REPL C:\Users\danjv\.julia\juliaup\julia-1.10.4+0.x64.w64.mingw32\share\julia\stdlib\v1.10\REPL\src\REPL.jl:911
 [26] (::VSCodeServer.var"#103#106"{REPL.var"#do_respond#80"{Bool, Bool, REPL.var"#93#103"{…}, REPL.LineEditREPL, REPL.LineEdit.Prompt}})(mi::REPL.LineEdit.MIState, buf::IOBuffer, ok::Bool)
    @ VSCodeServer c:\Users\danjv\.vscode\extensions\julialang.language-julia-1.83.2\scripts\packages\VSCodeServer\src\repl.jl:122
 [27] #invokelatest#2
    @ .\essentials.jl:892 [inlined]
 [28] invokelatest
    @ .\essentials.jl:889 [inlined]
 [29] run_interface(terminal::REPL.Terminals.TextTerminal, m::REPL.LineEdit.ModalInterface, s::REPL.LineEdit.MIState)
    @ REPL.LineEdit C:\Users\danjv\.julia\juliaup\julia-1.10.4+0.x64.w64.mingw32\share\julia\stdlib\v1.10\REPL\src\LineEdit.jl:2656
 [30] run_frontend(repl::REPL.LineEditREPL, backend::REPL.REPLBackendRef)
    @ REPL C:\Users\danjv\.julia\juliaup\julia-1.10.4+0.x64.w64.mingw32\share\julia\stdlib\v1.10\REPL\src\REPL.jl:1312
 [31] (::REPL.var"#62#68"{REPL.LineEditREPL, REPL.REPLBackendRef})()
    @ REPL C:\Users\danjv\.julia\juliaup\julia-1.10.4+0.x64.w64.mingw32\share\julia\stdlib\v1.10\REPL\src\REPL.jl:386
Some type information was truncated. Use `show(err)` to see complete types.

@DanielVandH
Copy link
Contributor Author

@DanielVandH DanielVandH commented on 66ae7a2 Jul 11, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Mine also doesn't work since sometimes j or k are UnitRanges. Should be

function colsupport(lay::AbstractInvLayout{TriLay}, A, j) where {S,TriLay<:TriangularLayout{S}}
    isempty(j) && return 1:0
    B, = arguments(lay, A)
    if S == 'U'
        return firstindex(B, 2):(maximum(j) - firstindex(B, 2) + 1)
    else # S == 'L' 
        return minimum(j):size(B, 2)
    end 
end 
function rowsupport(lay::AbstractInvLayout{TriLay}, A, k) where {S,TriLay<:TriangularLayout{S}}
    isempty(k) && return 1:0
    B, = arguments(lay, A) 
    if S == 'U' 
        return minimum(k):size(B, 1) 
    else # S == 'L'
        return firstindex(B, 1):(maximum(k) - firstindex(B, 1) + 1)
    end 
end

Will make a PR once I get it fully tested in my b=-1 pr

@dlfivefifty
Copy link
Member

Choose a reason for hiding this comment

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

Oh yeah triangularlayout assumes it’s wrapping data

Please sign in to comment.