From ed1d1fc0f50b4fc2bc782a2b12bf3175726bfa0e Mon Sep 17 00:00:00 2001 From: Tor Erlend Fjelde Date: Wed, 6 Sep 2023 13:29:46 +0100 Subject: [PATCH 01/16] added impl of setproperties for Cholesky --- src/nonstandard.jl | 19 ++++++++++++++++ test/runtests.jl | 54 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 73 insertions(+) diff --git a/src/nonstandard.jl b/src/nonstandard.jl index 2ba642d..8089334 100644 --- a/src/nonstandard.jl +++ b/src/nonstandard.jl @@ -56,3 +56,22 @@ constructorof(::Type{<:LinRange}) = linrange_constructor ### Expr: args get splatted # ::Expr annotation is to make it type-stable on Julia 1.3- constructorof(::Type{<:Expr}) = (head, args) -> Expr(head, args...)::Expr + +### Cholesky +@generated function ConstructionBase.setproperties( + C::LinearAlgebra.Cholesky, patch::NamedTuple{names} +) where {names} + # At most one of :L, :U, :UL can be patched. + ((:L in names) + (:U in names) + (:UL in names) <= 1) || return :(error("Can only patch one of :L, :U, :UL at the time")) + UL_expr = if :L in names + :(C.uplo === 'U' ? permutedims(patch.L) : patch.L) + elseif :U in names + :(C.uplo === 'L' ? permutedims(patch.U) : patch.U) + elseif :UL in names + :(patch.UL) + else + :(C.UL) + end + + return :(LinearAlgebra.Cholesky($UL_expr, C.uplo, C.info)) +end diff --git a/test/runtests.jl b/test/runtests.jl index c31ae24..19e39e5 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -194,6 +194,60 @@ end @inferred constructorof(typeof(lr1))(getfields(lr2)...) end + @testset "Cholesky" begin + X = Matrix(Diagonal(ones(3))) + @testset "uplo=$uplo" for uplo in ['L', 'U'] + C = Cholesky(X, uplo, 0) + + @testset "only setting value" begin + # Empty patch. + C_new = ConstructionBase.setproperties(C, NamedTuple()) + for f in propertynames(C) + @test getproperty(C_new, f) == getproperty(C, f) + end + + # Update `L`. + C_new = ConstructionBase.setproperties(C, (L=parent(C.L),)) + for f in propertynames(C) + @test getproperty(C_new, f) == getproperty(C, f) + end + + # Update `U`. + C_new = ConstructionBase.setproperties(C, (U=parent(C.U),)) + for f in propertynames(C) + @test getproperty(C_new, f) == getproperty(C, f) + end + + # Update `UL` + C_new = ConstructionBase.setproperties(C, (UL=parent(C.UL),)) + for f in propertynames(C) + @test getproperty(C_new, f) == getproperty(C, f) + end # Update `L`. + C_new = ConstructionBase.setproperties(C, (L=parent(C.L),)) + for f in propertynames(C) + @test getproperty(C_new, f) == getproperty(C, f) + end + + # Update `U`. + C_new = ConstructionBase.setproperties(C, (U=parent(C.U),)) + for f in propertynames(C) + @test getproperty(C_new, f) == getproperty(C, f) + end + + # Update `UL` + C_new = ConstructionBase.setproperties(C, (UL=parent(C.UL),)) + for f in propertynames(C) + @test getproperty(C_new, f) == getproperty(C, f) + end + + # Invalid patches. + @test_throws ErrorException ConstructionBase.setproperties(C, (L=parent(C.L), U=parent(C.U),)) + @test_throws ErrorException ConstructionBase.setproperties(C, (UL=parent(C.UL), U=parent(C.U),)) + @test_throws ErrorException ConstructionBase.setproperties(C, (UL=parent(C.UL), L=parent(C.L),)) + end + end + end + @testset "Expr" begin e = :(a + b) @test e == @inferred constructorof(typeof(e))(getfields(e)...) From bffad90b4e284b88d977d9099df146257d530f58 Mon Sep 17 00:00:00 2001 From: Tor Erlend Fjelde Date: Wed, 6 Sep 2023 13:35:13 +0100 Subject: [PATCH 02/16] remove @generated function in favour of 4 seperate implementations --- src/nonstandard.jl | 28 ++++++++++++---------------- 1 file changed, 12 insertions(+), 16 deletions(-) diff --git a/src/nonstandard.jl b/src/nonstandard.jl index 8089334..a58cb4f 100644 --- a/src/nonstandard.jl +++ b/src/nonstandard.jl @@ -58,20 +58,16 @@ constructorof(::Type{<:LinRange}) = linrange_constructor constructorof(::Type{<:Expr}) = (head, args) -> Expr(head, args...)::Expr ### Cholesky -@generated function ConstructionBase.setproperties( - C::LinearAlgebra.Cholesky, patch::NamedTuple{names} -) where {names} - # At most one of :L, :U, :UL can be patched. - ((:L in names) + (:U in names) + (:UL in names) <= 1) || return :(error("Can only patch one of :L, :U, :UL at the time")) - UL_expr = if :L in names - :(C.uplo === 'U' ? permutedims(patch.L) : patch.L) - elseif :U in names - :(C.uplo === 'L' ? permutedims(patch.U) : patch.U) - elseif :UL in names - :(patch.UL) - else - :(C.UL) - end - - return :(LinearAlgebra.Cholesky($UL_expr, C.uplo, C.info)) +setproperties(C::LinearAlgebra.Cholesky, patch::NamedTuple{()}) = C +function setproperties(C::LinearAlgebra.Cholesky, patch::NamedTuple{(:L,)}) + return LinearAlgebra.Cholesky(C.uplo === 'U' ? permutedims(patch.L) : patch.L, C.uplo, C.info) +end +function setproperties(C::LinearAlgebra.Cholesky, patch::NamedTuple{(:U,)}) + return LinearAlgebra.Cholesky(C.uplo === 'L' ? permutedims(patch.U) : patch.U, C.uplo, C.info) +end +function setproperties(C::LinearAlgebra.Cholesky, patch::NamedTuple{(:UL,)}) + return LinearAlgebra.Cholesky(patch.UL, C.uplo, C.info) +end +@nospecialize function setproperties(C::LinearAlgebra.Cholesky, patch::NamedTuple) + error("Can only patch one of :L, :U, :UL at the time") end From 2b945ca54829e8a8f22944cda8f21b0ff388bc95 Mon Sep 17 00:00:00 2001 From: Tor Erlend Fjelde Date: Wed, 6 Sep 2023 13:41:03 +0100 Subject: [PATCH 03/16] made tests for Cholesky not pass if setproperties is no-op --- test/runtests.jl | 32 +++++++++----------------------- 1 file changed, 9 insertions(+), 23 deletions(-) diff --git a/test/runtests.jl b/test/runtests.jl index 19e39e5..08582c6 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -195,7 +195,9 @@ end end @testset "Cholesky" begin - X = Matrix(Diagonal(ones(3))) + # X = Matrix(Diagonal(ones(3))) + x = randn(3, 3) + X = x * x' @testset "uplo=$uplo" for uplo in ['L', 'U'] C = Cholesky(X, uplo, 0) @@ -207,37 +209,21 @@ end end # Update `L`. - C_new = ConstructionBase.setproperties(C, (L=parent(C.L),)) + C_new = ConstructionBase.setproperties(C, (L=2 .* parent(C.L),)) for f in propertynames(C) - @test getproperty(C_new, f) == getproperty(C, f) + @test getproperty(C_new, f) == 2 .* getproperty(C, f) end # Update `U`. - C_new = ConstructionBase.setproperties(C, (U=parent(C.U),)) + C_new = ConstructionBase.setproperties(C, (U=2 .* parent(C.U),)) for f in propertynames(C) - @test getproperty(C_new, f) == getproperty(C, f) + @test getproperty(C_new, f) == 2 .* getproperty(C, f) end # Update `UL` - C_new = ConstructionBase.setproperties(C, (UL=parent(C.UL),)) - for f in propertynames(C) - @test getproperty(C_new, f) == getproperty(C, f) - end # Update `L`. - C_new = ConstructionBase.setproperties(C, (L=parent(C.L),)) + C_new = ConstructionBase.setproperties(C, (UL=2 .* parent(C.UL),)) for f in propertynames(C) - @test getproperty(C_new, f) == getproperty(C, f) - end - - # Update `U`. - C_new = ConstructionBase.setproperties(C, (U=parent(C.U),)) - for f in propertynames(C) - @test getproperty(C_new, f) == getproperty(C, f) - end - - # Update `UL` - C_new = ConstructionBase.setproperties(C, (UL=parent(C.UL),)) - for f in propertynames(C) - @test getproperty(C_new, f) == getproperty(C, f) + @test getproperty(C_new, f) == 2 .* getproperty(C, f) end # Invalid patches. From a452270f90a7cc3fde31fcbf721cd02da6ad9674 Mon Sep 17 00:00:00 2001 From: Tor Erlend Fjelde Date: Wed, 6 Sep 2023 14:05:39 +0100 Subject: [PATCH 04/16] improved error for setproperties of Cholesky as per suggestion of @devmotion --- src/nonstandard.jl | 8 ++++++-- test/runtests.jl | 7 ++++--- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/src/nonstandard.jl b/src/nonstandard.jl index a58cb4f..26f1c18 100644 --- a/src/nonstandard.jl +++ b/src/nonstandard.jl @@ -68,6 +68,10 @@ end function setproperties(C::LinearAlgebra.Cholesky, patch::NamedTuple{(:UL,)}) return LinearAlgebra.Cholesky(patch.UL, C.uplo, C.info) end -@nospecialize function setproperties(C::LinearAlgebra.Cholesky, patch::NamedTuple) - error("Can only patch one of :L, :U, :UL at the time") +function setproperties(C::LinearAlgebra.Cholesky, patch::NamedTuple) + if haskey(patch, :L) || haskey(patch, :U) || haskey(patch, :UL) + throw(ArgumentError("Can only patch one of :L, :U, :UL at the time")) + end + + throw(ArgumentError("Invalid patch for `Cholesky`: $(patch)")) end diff --git a/test/runtests.jl b/test/runtests.jl index 08582c6..50fbd8e 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -227,9 +227,10 @@ end end # Invalid patches. - @test_throws ErrorException ConstructionBase.setproperties(C, (L=parent(C.L), U=parent(C.U),)) - @test_throws ErrorException ConstructionBase.setproperties(C, (UL=parent(C.UL), U=parent(C.U),)) - @test_throws ErrorException ConstructionBase.setproperties(C, (UL=parent(C.UL), L=parent(C.L),)) + @test_throws "Can only patch one of" ConstructionBase.setproperties(C, (L=parent(C.L), U=parent(C.U),)) + @test_throws "Can only patch one of" ConstructionBase.setproperties(C, (UL=parent(C.UL), U=parent(C.U),)) + @test_throws "Can only patch one of" ConstructionBase.setproperties(C, (UL=parent(C.UL), L=parent(C.L),)) + @test_throws "Invalid patch" ConstructionBase.setproperties(C, (asdf=parent(C.UL),)) end end end From 1084be1eabe07943e050e8937d1d4d01b5bd033a Mon Sep 17 00:00:00 2001 From: Tor Erlend Fjelde Date: Wed, 6 Sep 2023 14:12:36 +0100 Subject: [PATCH 05/16] reverted to more primitive test_throws as Julia 1.0 does not support matching of strings --- test/runtests.jl | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/runtests.jl b/test/runtests.jl index 50fbd8e..e919df3 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -227,10 +227,10 @@ end end # Invalid patches. - @test_throws "Can only patch one of" ConstructionBase.setproperties(C, (L=parent(C.L), U=parent(C.U),)) - @test_throws "Can only patch one of" ConstructionBase.setproperties(C, (UL=parent(C.UL), U=parent(C.U),)) - @test_throws "Can only patch one of" ConstructionBase.setproperties(C, (UL=parent(C.UL), L=parent(C.L),)) - @test_throws "Invalid patch" ConstructionBase.setproperties(C, (asdf=parent(C.UL),)) + @test_throws ArgumentError ConstructionBase.setproperties(C, (L=parent(C.L), U=parent(C.U),)) + @test_throws ArgumentError ConstructionBase.setproperties(C, (UL=parent(C.UL), U=parent(C.U),)) + @test_throws ArgumentError ConstructionBase.setproperties(C, (UL=parent(C.UL), L=parent(C.L),)) + @test_throws ArgumentError ConstructionBase.setproperties(C, (asdf=parent(C.UL),)) end end end From f0d98ff3b5ad1ae20d244f917626705297eb07af Mon Sep 17 00:00:00 2001 From: Tor Erlend Fjelde Date: Wed, 6 Sep 2023 14:16:38 +0100 Subject: [PATCH 06/16] Update src/nonstandard.jl Co-authored-by: David Widmann --- src/nonstandard.jl | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/nonstandard.jl b/src/nonstandard.jl index 26f1c18..ef04dd0 100644 --- a/src/nonstandard.jl +++ b/src/nonstandard.jl @@ -69,9 +69,5 @@ function setproperties(C::LinearAlgebra.Cholesky, patch::NamedTuple{(:UL,)}) return LinearAlgebra.Cholesky(patch.UL, C.uplo, C.info) end function setproperties(C::LinearAlgebra.Cholesky, patch::NamedTuple) - if haskey(patch, :L) || haskey(patch, :U) || haskey(patch, :UL) - throw(ArgumentError("Can only patch one of :L, :U, :UL at the time")) - end - throw(ArgumentError("Invalid patch for `Cholesky`: $(patch)")) end From bdd89ae99625ff06a55b148424d538d180a7aaa2 Mon Sep 17 00:00:00 2001 From: Tor Erlend Fjelde Date: Thu, 7 Sep 2023 09:20:41 +0100 Subject: [PATCH 07/16] Update test/runtests.jl --- test/runtests.jl | 1 - 1 file changed, 1 deletion(-) diff --git a/test/runtests.jl b/test/runtests.jl index e919df3..5c96cd2 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -195,7 +195,6 @@ end end @testset "Cholesky" begin - # X = Matrix(Diagonal(ones(3))) x = randn(3, 3) X = x * x' @testset "uplo=$uplo" for uplo in ['L', 'U'] From 4b8db5639e901a637ba3c1438a47e850103e45a2 Mon Sep 17 00:00:00 2001 From: Tor Erlend Fjelde Date: Thu, 7 Sep 2023 15:37:22 +0100 Subject: [PATCH 08/16] unwrap patch in setproperties for `Cholesky` using `parent` --- src/nonstandard.jl | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/nonstandard.jl b/src/nonstandard.jl index ef04dd0..8322023 100644 --- a/src/nonstandard.jl +++ b/src/nonstandard.jl @@ -60,13 +60,13 @@ constructorof(::Type{<:Expr}) = (head, args) -> Expr(head, args...)::Expr ### Cholesky setproperties(C::LinearAlgebra.Cholesky, patch::NamedTuple{()}) = C function setproperties(C::LinearAlgebra.Cholesky, patch::NamedTuple{(:L,)}) - return LinearAlgebra.Cholesky(C.uplo === 'U' ? permutedims(patch.L) : patch.L, C.uplo, C.info) + return LinearAlgebra.Cholesky(parent(C.uplo === 'U' ? permutedims(patch.L) : patch.L), C.uplo, C.info) end function setproperties(C::LinearAlgebra.Cholesky, patch::NamedTuple{(:U,)}) - return LinearAlgebra.Cholesky(C.uplo === 'L' ? permutedims(patch.U) : patch.U, C.uplo, C.info) + return LinearAlgebra.Cholesky(parent(C.uplo === 'L' ? permutedims(patch.U) : patch.U), C.uplo, C.info) end function setproperties(C::LinearAlgebra.Cholesky, patch::NamedTuple{(:UL,)}) - return LinearAlgebra.Cholesky(patch.UL, C.uplo, C.info) + return LinearAlgebra.Cholesky(parent(patch.UL), C.uplo, C.info) end function setproperties(C::LinearAlgebra.Cholesky, patch::NamedTuple) throw(ArgumentError("Invalid patch for `Cholesky`: $(patch)")) From 4f69cc971738fb6ebd125543f362e71f5939137f Mon Sep 17 00:00:00 2001 From: Tor Erlend Fjelde Date: Thu, 7 Sep 2023 15:40:16 +0100 Subject: [PATCH 09/16] added tests for the type-result of Cholesky setproperty to --- test/runtests.jl | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/test/runtests.jl b/test/runtests.jl index 5c96cd2..0ea96ea 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -203,24 +203,28 @@ end @testset "only setting value" begin # Empty patch. C_new = ConstructionBase.setproperties(C, NamedTuple()) + @test typeof(C_new) === typeof(C) for f in propertynames(C) @test getproperty(C_new, f) == getproperty(C, f) end # Update `L`. C_new = ConstructionBase.setproperties(C, (L=2 .* parent(C.L),)) + @test typeof(C_new) === typeof(C) for f in propertynames(C) @test getproperty(C_new, f) == 2 .* getproperty(C, f) end # Update `U`. C_new = ConstructionBase.setproperties(C, (U=2 .* parent(C.U),)) + @test typeof(C_new) === typeof(C) for f in propertynames(C) @test getproperty(C_new, f) == 2 .* getproperty(C, f) end # Update `UL` C_new = ConstructionBase.setproperties(C, (UL=2 .* parent(C.UL),)) + @test typeof(C_new) === typeof(C) for f in propertynames(C) @test getproperty(C_new, f) == 2 .* getproperty(C, f) end From 7575d09a3d58854f63c1276605fbeedb4d958c2c Mon Sep 17 00:00:00 2001 From: Tor Erlend Fjelde Date: Fri, 8 Sep 2023 09:19:39 +0100 Subject: [PATCH 10/16] restrict setproperties for Cholesky to only work with LowerTriangular and UpperTriangular --- src/nonstandard.jl | 9 ++++-- test/runtests.jl | 70 ++++++++++++++++++++++++---------------------- 2 files changed, 42 insertions(+), 37 deletions(-) diff --git a/src/nonstandard.jl b/src/nonstandard.jl index 8322023..1f44a67 100644 --- a/src/nonstandard.jl +++ b/src/nonstandard.jl @@ -59,13 +59,16 @@ constructorof(::Type{<:Expr}) = (head, args) -> Expr(head, args...)::Expr ### Cholesky setproperties(C::LinearAlgebra.Cholesky, patch::NamedTuple{()}) = C -function setproperties(C::LinearAlgebra.Cholesky, patch::NamedTuple{(:L,)}) +function setproperties(C::LinearAlgebra.Cholesky, patch::NamedTuple{(:L,),Tuple{L}}) where {L<:LinearAlgebra.LowerTriangular} return LinearAlgebra.Cholesky(parent(C.uplo === 'U' ? permutedims(patch.L) : patch.L), C.uplo, C.info) end -function setproperties(C::LinearAlgebra.Cholesky, patch::NamedTuple{(:U,)}) +function setproperties(C::LinearAlgebra.Cholesky, patch::NamedTuple{(:U,),Tuple{U}}) where {U<:LinearAlgebra.UpperTriangular} return LinearAlgebra.Cholesky(parent(C.uplo === 'L' ? permutedims(patch.U) : patch.U), C.uplo, C.info) end -function setproperties(C::LinearAlgebra.Cholesky, patch::NamedTuple{(:UL,)}) +function setproperties( + C::LinearAlgebra.Cholesky, + patch::NamedTuple{(:UL,),Tuple{UL}} +) where {UL<:Union{LinearAlgebra.LowerTriangular,LinearAlgebra.UpperTriangular}} return LinearAlgebra.Cholesky(parent(patch.UL), C.uplo, C.info) end function setproperties(C::LinearAlgebra.Cholesky, patch::NamedTuple) diff --git a/test/runtests.jl b/test/runtests.jl index 0ea96ea..9b93eeb 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -200,41 +200,43 @@ end @testset "uplo=$uplo" for uplo in ['L', 'U'] C = Cholesky(X, uplo, 0) - @testset "only setting value" begin - # Empty patch. - C_new = ConstructionBase.setproperties(C, NamedTuple()) - @test typeof(C_new) === typeof(C) - for f in propertynames(C) - @test getproperty(C_new, f) == getproperty(C, f) - end - - # Update `L`. - C_new = ConstructionBase.setproperties(C, (L=2 .* parent(C.L),)) - @test typeof(C_new) === typeof(C) - for f in propertynames(C) - @test getproperty(C_new, f) == 2 .* getproperty(C, f) - end - - # Update `U`. - C_new = ConstructionBase.setproperties(C, (U=2 .* parent(C.U),)) - @test typeof(C_new) === typeof(C) - for f in propertynames(C) - @test getproperty(C_new, f) == 2 .* getproperty(C, f) - end - - # Update `UL` - C_new = ConstructionBase.setproperties(C, (UL=2 .* parent(C.UL),)) - @test typeof(C_new) === typeof(C) - for f in propertynames(C) - @test getproperty(C_new, f) == 2 .* getproperty(C, f) - end - - # Invalid patches. - @test_throws ArgumentError ConstructionBase.setproperties(C, (L=parent(C.L), U=parent(C.U),)) - @test_throws ArgumentError ConstructionBase.setproperties(C, (UL=parent(C.UL), U=parent(C.U),)) - @test_throws ArgumentError ConstructionBase.setproperties(C, (UL=parent(C.UL), L=parent(C.L),)) - @test_throws ArgumentError ConstructionBase.setproperties(C, (asdf=parent(C.UL),)) + # Empty patch. + C_new = ConstructionBase.setproperties(C, NamedTuple()) + @test typeof(C_new) === typeof(C) + for f in propertynames(C) + @test getproperty(C_new, f) == getproperty(C, f) end + + # Update `L`. + C_new = ConstructionBase.setproperties(C, (L=2 .* C.L,)) + @test typeof(C_new) === typeof(C) + for f in propertynames(C) + @test getproperty(C_new, f) == 2 .* getproperty(C, f) + end + + # Update `U`. + C_new = ConstructionBase.setproperties(C, (U=2 .* C.U,)) + @test typeof(C_new) === typeof(C) + for f in propertynames(C) + @test getproperty(C_new, f) == 2 .* getproperty(C, f) + end + + # Update `UL` + C_new = ConstructionBase.setproperties(C, (UL=2 .* C.UL,)) + @test typeof(C_new) === typeof(C) + for f in propertynames(C) + @test getproperty(C_new, f) == 2 .* getproperty(C, f) + end + + # We can only set the properties with `LowerTriangular` or `UpperTriangular` matrices. + @test_throws ArgumentError ConstructionBase.setproperties(C, (L=parent(C.L),)) + @test_throws ArgumentError ConstructionBase.setproperties(C, (U=parent(C.U),)) + # Can only set one at the time. + @test_throws ArgumentError ConstructionBase.setproperties(C, (L=C.L, U=C.U,)) + @test_throws ArgumentError ConstructionBase.setproperties(C, (UL=C.UL, U=C.U,)) + @test_throws ArgumentError ConstructionBase.setproperties(C, (UL=C.UL, L=C.L,)) + # And make sure any other patch will fail. + @test_throws ArgumentError ConstructionBase.setproperties(C, (asdf=C.UL,)) end end From eeb51978b44f2469f7debb1a825eb7d74ef23b71 Mon Sep 17 00:00:00 2001 From: Tor Erlend Fjelde Date: Fri, 8 Sep 2023 09:21:57 +0100 Subject: [PATCH 11/16] bumped patch version --- Project.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Project.toml b/Project.toml index ed3f011..2986670 100644 --- a/Project.toml +++ b/Project.toml @@ -1,7 +1,7 @@ name = "ConstructionBase" uuid = "187b0558-2788-49d3-abe0-74a17ed4e7c9" authors = ["Takafumi Arakaki", "Rafael Schouten", "Jan Weidner"] -version = "1.5.3" +version = "1.5.4" [deps] LinearAlgebra = "37e2e46d-f89d-539d-b4ee-838fcccc9c8e" From 8caceec210ae5a50f9a5e025ed948e93072640ad Mon Sep 17 00:00:00 2001 From: Tor Erlend Fjelde Date: Fri, 8 Sep 2023 09:40:32 +0100 Subject: [PATCH 12/16] removed permutedims and parent in favour of copy and transpose --- src/nonstandard.jl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/nonstandard.jl b/src/nonstandard.jl index 1f44a67..f197a03 100644 --- a/src/nonstandard.jl +++ b/src/nonstandard.jl @@ -60,10 +60,10 @@ constructorof(::Type{<:Expr}) = (head, args) -> Expr(head, args...)::Expr ### Cholesky setproperties(C::LinearAlgebra.Cholesky, patch::NamedTuple{()}) = C function setproperties(C::LinearAlgebra.Cholesky, patch::NamedTuple{(:L,),Tuple{L}}) where {L<:LinearAlgebra.LowerTriangular} - return LinearAlgebra.Cholesky(parent(C.uplo === 'U' ? permutedims(patch.L) : patch.L), C.uplo, C.info) + return LinearAlgebra.Cholesky(C.uplo === 'U' ? copy(transpose(patch.L.data)) : patch.L.data, C.uplo, C.info) end function setproperties(C::LinearAlgebra.Cholesky, patch::NamedTuple{(:U,),Tuple{U}}) where {U<:LinearAlgebra.UpperTriangular} - return LinearAlgebra.Cholesky(parent(C.uplo === 'L' ? permutedims(patch.U) : patch.U), C.uplo, C.info) + return LinearAlgebra.Cholesky(C.uplo === 'L' ? copy(transpose(patch.U.data)) : patch.U.data, C.uplo, C.info) end function setproperties( C::LinearAlgebra.Cholesky, From 01a64479f8d31024e6cde35761e71c4f57459d86 Mon Sep 17 00:00:00 2001 From: Tor Erlend Fjelde Date: Fri, 8 Sep 2023 09:46:11 +0100 Subject: [PATCH 13/16] use data field instead of parent for `UL` update to `Cholesky` --- src/nonstandard.jl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/nonstandard.jl b/src/nonstandard.jl index f197a03..e6f205f 100644 --- a/src/nonstandard.jl +++ b/src/nonstandard.jl @@ -69,7 +69,7 @@ function setproperties( C::LinearAlgebra.Cholesky, patch::NamedTuple{(:UL,),Tuple{UL}} ) where {UL<:Union{LinearAlgebra.LowerTriangular,LinearAlgebra.UpperTriangular}} - return LinearAlgebra.Cholesky(parent(patch.UL), C.uplo, C.info) + return LinearAlgebra.Cholesky(patch.UL.data, C.uplo, C.info) end function setproperties(C::LinearAlgebra.Cholesky, patch::NamedTuple) throw(ArgumentError("Invalid patch for `Cholesky`: $(patch)")) From 0a27f7a629842e816368433b6962743f036dedf5 Mon Sep 17 00:00:00 2001 From: Tor Erlend Fjelde Date: Fri, 8 Sep 2023 10:25:21 +0100 Subject: [PATCH 14/16] Apply suggestions from code review Co-authored-by: David Widmann --- src/nonstandard.jl | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/nonstandard.jl b/src/nonstandard.jl index e6f205f..2027160 100644 --- a/src/nonstandard.jl +++ b/src/nonstandard.jl @@ -59,16 +59,16 @@ constructorof(::Type{<:Expr}) = (head, args) -> Expr(head, args...)::Expr ### Cholesky setproperties(C::LinearAlgebra.Cholesky, patch::NamedTuple{()}) = C -function setproperties(C::LinearAlgebra.Cholesky, patch::NamedTuple{(:L,),Tuple{L}}) where {L<:LinearAlgebra.LowerTriangular} - return LinearAlgebra.Cholesky(C.uplo === 'U' ? copy(transpose(patch.L.data)) : patch.L.data, C.uplo, C.info) +function setproperties(C::LinearAlgebra.Cholesky, patch::NamedTuple{(:L,),Tuple{<:LinearAlgebra.LowerTriangular}}) + return LinearAlgebra.Cholesky(C.uplo === 'U' ? copy(patch.L.data') : patch.L.data, C.uplo, C.info) end -function setproperties(C::LinearAlgebra.Cholesky, patch::NamedTuple{(:U,),Tuple{U}}) where {U<:LinearAlgebra.UpperTriangular} - return LinearAlgebra.Cholesky(C.uplo === 'L' ? copy(transpose(patch.U.data)) : patch.U.data, C.uplo, C.info) +function setproperties(C::LinearAlgebra.Cholesky, patch::NamedTuple{(:U,),Tuple{<:LinearAlgebra.UpperTriangular}}) + return LinearAlgebra.Cholesky(C.uplo === 'L' ? copy(patch.U.data') : patch.U.data, C.uplo, C.info) end function setproperties( C::LinearAlgebra.Cholesky, - patch::NamedTuple{(:UL,),Tuple{UL}} -) where {UL<:Union{LinearAlgebra.LowerTriangular,LinearAlgebra.UpperTriangular}} + patch::NamedTuple{(:UL,),Tuple{<:Union{LinearAlgebra.LowerTriangular,LinearAlgebra.UpperTriangular}}} +) return LinearAlgebra.Cholesky(patch.UL.data, C.uplo, C.info) end function setproperties(C::LinearAlgebra.Cholesky, patch::NamedTuple) From 2b2e395c49903ce24a08ead01653d8695ad376ae Mon Sep 17 00:00:00 2001 From: Tor Erlend Fjelde Date: Fri, 8 Sep 2023 13:39:44 +0100 Subject: [PATCH 15/16] remove broadcasted mul in Choeslky tests --- test/runtests.jl | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/test/runtests.jl b/test/runtests.jl index 9b93eeb..7cee50d 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -208,24 +208,24 @@ end end # Update `L`. - C_new = ConstructionBase.setproperties(C, (L=2 .* C.L,)) + C_new = ConstructionBase.setproperties(C, (L=2 * C.L,)) @test typeof(C_new) === typeof(C) for f in propertynames(C) - @test getproperty(C_new, f) == 2 .* getproperty(C, f) + @test getproperty(C_new, f) == 2 * getproperty(C, f) end # Update `U`. - C_new = ConstructionBase.setproperties(C, (U=2 .* C.U,)) + C_new = ConstructionBase.setproperties(C, (U=2 * C.U,)) @test typeof(C_new) === typeof(C) for f in propertynames(C) - @test getproperty(C_new, f) == 2 .* getproperty(C, f) + @test getproperty(C_new, f) == 2 * getproperty(C, f) end # Update `UL` - C_new = ConstructionBase.setproperties(C, (UL=2 .* C.UL,)) + C_new = ConstructionBase.setproperties(C, (UL=2 * C.UL,)) @test typeof(C_new) === typeof(C) for f in propertynames(C) - @test getproperty(C_new, f) == 2 .* getproperty(C, f) + @test getproperty(C_new, f) == 2 * getproperty(C, f) end # We can only set the properties with `LowerTriangular` or `UpperTriangular` matrices. From 6988051c466de3b9de26e7256cca5cf5ceffda3d Mon Sep 17 00:00:00 2001 From: Tor Erlend Fjelde Date: Fri, 8 Sep 2023 15:21:11 +0100 Subject: [PATCH 16/16] fixed issues with dispatch --- src/nonstandard.jl | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/nonstandard.jl b/src/nonstandard.jl index 2027160..4b114a7 100644 --- a/src/nonstandard.jl +++ b/src/nonstandard.jl @@ -59,15 +59,15 @@ constructorof(::Type{<:Expr}) = (head, args) -> Expr(head, args...)::Expr ### Cholesky setproperties(C::LinearAlgebra.Cholesky, patch::NamedTuple{()}) = C -function setproperties(C::LinearAlgebra.Cholesky, patch::NamedTuple{(:L,),Tuple{<:LinearAlgebra.LowerTriangular}}) +function setproperties(C::LinearAlgebra.Cholesky, patch::NamedTuple{(:L,),<:Tuple{<:LinearAlgebra.LowerTriangular}}) return LinearAlgebra.Cholesky(C.uplo === 'U' ? copy(patch.L.data') : patch.L.data, C.uplo, C.info) end -function setproperties(C::LinearAlgebra.Cholesky, patch::NamedTuple{(:U,),Tuple{<:LinearAlgebra.UpperTriangular}}) +function setproperties(C::LinearAlgebra.Cholesky, patch::NamedTuple{(:U,),<:Tuple{<:LinearAlgebra.UpperTriangular}}) return LinearAlgebra.Cholesky(C.uplo === 'L' ? copy(patch.U.data') : patch.U.data, C.uplo, C.info) end function setproperties( C::LinearAlgebra.Cholesky, - patch::NamedTuple{(:UL,),Tuple{<:Union{LinearAlgebra.LowerTriangular,LinearAlgebra.UpperTriangular}}} + patch::NamedTuple{(:UL,),<:Tuple{<:Union{LinearAlgebra.LowerTriangular,LinearAlgebra.UpperTriangular}}} ) return LinearAlgebra.Cholesky(patch.UL.data, C.uplo, C.info) end