From 5848fb4f465ee0ea2f92cba454672ee6b5b7eabf Mon Sep 17 00:00:00 2001 From: Ronny Bergmann Date: Fri, 25 Jun 2021 12:07:42 +0200 Subject: [PATCH 1/5] Fixes a bug, that occured when calling `exp!(Rotations(2), p, p, X)` since `q=p` was modified. --- Project.toml | 2 +- src/manifolds/Rotations.jl | 8 +------- test/rotations.jl | 6 ++++++ 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/Project.toml b/Project.toml index bdb69d1a8a..b7385e1354 100644 --- a/Project.toml +++ b/Project.toml @@ -1,7 +1,7 @@ name = "Manifolds" uuid = "1cead3c2-87b3-11e9-0ccd-23c62b72b94e" authors = ["Seth Axen ", "Mateusz Baran ", "Ronny Bergmann ", "Antoine Levitt "] -version = "0.5.5" +version = "0.5.6" [deps] Colors = "5ae59095-9a9b-59fe-a467-6f913c188581" diff --git a/src/manifolds/Rotations.jl b/src/manifolds/Rotations.jl index e1a96cf0b7..573c82bd83 100644 --- a/src/manifolds/Rotations.jl +++ b/src/manifolds/Rotations.jl @@ -165,13 +165,7 @@ function exp!(M::Rotations{2}, q, p, X) @assert size(q) == (2, 2) θ = get_coordinates(M, p, X, DefaultOrthogonalBasis())[1] sinθ, cosθ = sincos(θ) - @inbounds begin - q[1] = cosθ - q[2] = sinθ - q[3] = -sinθ - q[4] = cosθ - end - return copyto!(q, p * q) + return copyto!(q, p * [cosθ sinθ; -sinθ cosθ]) end function exp!(M::Rotations{3}, q, p, X) θ = norm(M, p, X) / sqrt(2) diff --git a/test/rotations.jl b/test/rotations.jl index 6d24d0997a..59b0ab114e 100644 --- a/test/rotations.jl +++ b/test/rotations.jl @@ -62,6 +62,12 @@ include("utils.jl") v = log(M, pts[1], pts[2]) @test norm(M, pts[1], v) ≈ (angles[2] - angles[1]) * sqrt(2) + # check that exp! does not have a side effect + q = deeocopy(pts[1]) + q2 = exp(M, pts[1], v) + exp!(M, q, q, v) + @test norm(q - q2) ≈ 0 + v14_polar = inverse_retract(M, pts[1], pts[4], Manifolds.PolarInverseRetraction()) p4_polar = retract(M, pts[1], v14_polar, Manifolds.PolarRetraction()) @test isapprox(M, pts[4], p4_polar) From 920d2304ce7b17dee21c1614fd4149cb5a5936cb Mon Sep 17 00:00:00 2001 From: Ronny Bergmann Date: Fri, 25 Jun 2021 13:01:01 +0200 Subject: [PATCH 2/5] Update src/manifolds/Rotations.jl Co-authored-by: Mateusz Baran --- src/manifolds/Rotations.jl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/manifolds/Rotations.jl b/src/manifolds/Rotations.jl index 573c82bd83..5642da28b7 100644 --- a/src/manifolds/Rotations.jl +++ b/src/manifolds/Rotations.jl @@ -165,7 +165,7 @@ function exp!(M::Rotations{2}, q, p, X) @assert size(q) == (2, 2) θ = get_coordinates(M, p, X, DefaultOrthogonalBasis())[1] sinθ, cosθ = sincos(θ) - return copyto!(q, p * [cosθ sinθ; -sinθ cosθ]) + return copyto!(q, p * SA[cosθ sinθ; -sinθ cosθ]) end function exp!(M::Rotations{3}, q, p, X) θ = norm(M, p, X) / sqrt(2) From b0e228cc07692ecf616be6bd1c6001e54aa67f68 Mon Sep 17 00:00:00 2001 From: Ronny Bergmann Date: Fri, 25 Jun 2021 13:12:44 +0200 Subject: [PATCH 3/5] Update test/rotations.jl Co-authored-by: Mateusz Baran --- test/rotations.jl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/rotations.jl b/test/rotations.jl index 59b0ab114e..3c26b5133c 100644 --- a/test/rotations.jl +++ b/test/rotations.jl @@ -63,7 +63,7 @@ include("utils.jl") @test norm(M, pts[1], v) ≈ (angles[2] - angles[1]) * sqrt(2) # check that exp! does not have a side effect - q = deeocopy(pts[1]) + q = copy(M, pts[1]) q2 = exp(M, pts[1], v) exp!(M, q, q, v) @test norm(q - q2) ≈ 0 From 58455469fec8bf6b4d506bd7084ef9073ca2f865 Mon Sep 17 00:00:00 2001 From: Ronny Bergmann Date: Fri, 25 Jun 2021 14:15:26 +0200 Subject: [PATCH 4/5] Update Rotations.jl I might have mixed up row- and column major indexing and hence have introduced the rotation the wrong direction around. --- src/manifolds/Rotations.jl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/manifolds/Rotations.jl b/src/manifolds/Rotations.jl index 5642da28b7..8649cce657 100644 --- a/src/manifolds/Rotations.jl +++ b/src/manifolds/Rotations.jl @@ -165,7 +165,7 @@ function exp!(M::Rotations{2}, q, p, X) @assert size(q) == (2, 2) θ = get_coordinates(M, p, X, DefaultOrthogonalBasis())[1] sinθ, cosθ = sincos(θ) - return copyto!(q, p * SA[cosθ sinθ; -sinθ cosθ]) + return copyto!(q, p * SA[cosθ -sinθ; sinθ cosθ]) end function exp!(M::Rotations{3}, q, p, X) θ = norm(M, p, X) / sqrt(2) From 2d5ce6e873420a3066d566d3c263aec5f554b754 Mon Sep 17 00:00:00 2001 From: Ronny Bergmann Date: Fri, 25 Jun 2021 15:48:34 +0200 Subject: [PATCH 5/5] Update rotations.jl fix allocation of `q`. --- test/rotations.jl | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/rotations.jl b/test/rotations.jl index 3c26b5133c..a4dfed2b28 100644 --- a/test/rotations.jl +++ b/test/rotations.jl @@ -63,7 +63,8 @@ include("utils.jl") @test norm(M, pts[1], v) ≈ (angles[2] - angles[1]) * sqrt(2) # check that exp! does not have a side effect - q = copy(M, pts[1]) + q = allocate(pts[1]) + copyto!(M, q, pts[1]) q2 = exp(M, pts[1], v) exp!(M, q, q, v) @test norm(q - q2) ≈ 0