Fix apply_diff_group for some actions (#673)

* Inverse of differential, fix for apply_diff_group issue * news entry * `inverse_translate_diff` fix * Fixes * Another fix * test for issue #669 * improve coverage * more tests * some tests and figuring out * fix inv_diff * fix translate_diff on general unitary * remove some tests that don't apply to all actions * why fail there? * tests * add some tests * this was right * Giles is helpful * fix circle group inv_diff * add some more tests * add pullbacks for group inverse * test for translations * update news * fix tests * fix the other test * coverage false positives * update version
JuliaManifolds · Nov 6, 2023 · 0a71858 · 0a71858
1 parent f9a6b19
commit 0a71858
Show file tree

Hide file tree

Showing 25 changed files with 589 additions and 86 deletions.
diff --git a/NEWS.md b/NEWS.md
@@ -5,12 +5,17 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
-## [0.9.4] - 2023-11-04
+## [0.9.4] - 2023-11-06
 
 ### Added
 
+- Functions `inv_diff`, `inv_diff!`, `adjoint_inv_diff` and `adjoint_inv_diff!` that correspond to differentials and pullbacks of group inversion.
 - Julia 1.10-rc CI workflow.
 
+### Fixed
+
+- Fixed issue with incorrect implementation of `apply_diff_group` in `GroupOperationAction` with left backward and right forward action [#669](https://github.com/JuliaManifolds/Manifolds.jl/issues/669).
+
 ## [0.9.3] - 2023-10-28
 
 ### Added

diff --git a/Project.toml b/Project.toml
@@ -1,7 +1,7 @@
 name = "Manifolds"
 uuid = "1cead3c2-87b3-11e9-0ccd-23c62b72b94e"
 authors = ["Seth Axen <[email protected]>", "Mateusz Baran <[email protected]>", "Ronny Bergmann <[email protected]>", "Antoine Levitt <[email protected]>"]
-version = "0.9.3"
+version = "0.9.4"
 
 [deps]
 Distributions = "31c24e10-a181-5473-b8eb-7969acd0382f"

diff --git a/docs/src/references.bib b/docs/src/references.bib
@@ -338,6 +338,19 @@ @article{GaoSonAbsilStykel:2021
     TITLE   = {Riemannian Optimization on the Symplectic Stiefel Manifold},
     JOURNAL = {SIAM Journal on Optimization}
 }
+@inproceedings{Giles:2008,
+	address = {Berlin, Heidelberg},
+	series = {Lecture {Notes} in {Computational} {Science} and {Engineering}},
+	title = {Collected {Matrix} {Derivative} {Results} for {Forward} and {Reverse} {Mode} {Algorithmic} {Differentiation}},
+	isbn = {978-3-540-68942-3},
+	doi = {10.1007/978-3-540-68942-3_4},
+	booktitle = {Advances in {Automatic} {Differentiation}},
+	publisher = {Springer},
+	author = {Giles, Mike B.},
+	editor = {Bischof, Christian H. and Bücker, H. Martin and Hovland, Paul and Naumann, Uwe and Utke, Jean},
+	year = {2008},
+	pages = {35--44},
+}
 @incollection{GuiguiMaignantTroouvePennec:2021,
     DOI       = {10.1007/978-3-030-80209-7_12},
     YEAR      = {2021},

diff --git a/ext/ManifoldsTestExt/tests_general.jl b/ext/ManifoldsTestExt/tests_general.jl
@@ -153,7 +153,7 @@ function test_manifold(
             ) for i in 1:n
         ]
     end
-    Test.@testset "dimension" begin
+    Test.@testset "dimension" begin # COV_EXCL_LINE
         Test.@test isa(manifold_dimension(M), expected_dimension_type)
         Test.@test manifold_dimension(M) ≥ 0
     end
@@ -180,7 +180,7 @@ function test_manifold(
         end
     end
 
-    Test.@testset "is_point" begin
+    Test.@testset "is_point" begin # COV_EXCL_LINE
         for pt in pts
             atol = is_point_atol_multiplier * find_eps(pt)
             Test.@test is_point(M, pt; atol=atol)
@@ -303,7 +303,7 @@ function test_manifold(
         mutating=is_mutating,
     )
 
-    Test.@testset "(inverse &) retraction tests" begin
+    Test.@testset "(inverse &) retraction tests" begin # COV_EXCL_LINE
         for (p, X) in zip(pts, tv)
             epsx = find_eps(p)
             point_atol = is_point_atol_multiplier * find_eps(p)
@@ -325,7 +325,7 @@ function test_manifold(
                 end
                 Test.@test is_point(M, new_pt; atol=point_atol)
                 (test_inplace && is_mutating) &&
-                    Test.@testset "inplace test for retract!" begin
+                    Test.@testset "inplace test for retract!" begin # COV_EXCL_LINE
                         p2 = copy(M, p)
                         X2 = copy(M, p, X)
                         q = retract(M, p2, X2, retr_method)
@@ -367,7 +367,7 @@ function test_manifold(
         end
     end
 
-    Test.@testset "atlases" begin
+    Test.@testset "atlases" begin # COV_EXCL_LINE
         if !isempty(test_atlases)
             Test.@test get_default_atlas(M) isa AbstractAtlas{ℝ}
         end
@@ -411,7 +411,7 @@ function test_manifold(
         end
     end
 
-    Test.@testset "basic linear algebra in tangent space" begin
+    Test.@testset "basic linear algebra in tangent space" begin # COV_EXCL_LINE
         for (p, X) in zip(pts, tv)
             Test.@test isapprox(M, p, 0 * X, zero_vector(M, p); atol=find_eps(pts[1]))
             Test.@test isapprox(M, p, 2 * X, X + X)
@@ -421,7 +421,7 @@ function test_manifold(
     end
 
     test_tangent_vector_broadcasting &&
-        Test.@testset "broadcasted linear algebra in tangent space" begin
+        Test.@testset "broadcasted linear algebra in tangent space" begin # COV_EXCL_LINE
             for (p, X) in zip(pts, tv)
                 Test.@test isapprox(M, p, 3 * X, 2 .* X .+ X)
                 Test.@test isapprox(M, p, -X, X .- 2 .* X)
@@ -469,7 +469,7 @@ function test_manifold(
     !(
         default_retraction_method === nothing ||
         default_inverse_retraction_method === nothing
-    ) && Test.@testset "vector transport" begin
+    ) && Test.@testset "vector transport" begin # COV_EXCL_LINE
         tvatol = is_tangent_atol_multiplier * find_eps(pts[1])
         X1 = inverse_retract(M, pts[1], pts[2], default_inverse_retraction_method)
         X2 = inverse_retract(M, pts[1], pts[3], default_inverse_retraction_method)
@@ -499,7 +499,7 @@ function test_manifold(
             vector_transport_retractions,
             vector_transport_inverse_retractions,
         )
-            Test.@testset "vector transport method $(vtm)" begin
+            Test.@testset "vector transport method $(vtm)" begin # COV_EXCL_LINE
                 tvatol = is_tangent_atol_multiplier * find_eps(pts[1])
                 X1 = inverse_retract(M, pts[1], pts[2], irtr_m)
                 X2 = inverse_retract(M, pts[1], pts[3], irtr_m)
@@ -531,7 +531,7 @@ function test_manifold(
                         vector_transport_to!(M, v1t1_m, pts[1], X1, pts32, vtm)
                         Test.@test isapprox(M, pts32, v1t1, v1t1_m; atol=tvatol)
                         test_inplace &&
-                            Test.@testset "inplace test for vector_transport_to!" begin
+                            Test.@testset "inplace test for vector_transport_to!" begin # COV_EXCL_LINE
                                 X1a = copy(M, pts[1], X1)
                                 Xt = vector_transport_to(M, pts[1], X1, pts32, vtm)
                                 vector_transport_to!(M, X1a, pts[1], X1a, pts32, vtm)
@@ -559,7 +559,7 @@ function test_manifold(
     end
 
     for btype in basis_types_vecs
-        Test.@testset "Basis support for $(btype)" begin
+        Test.@testset "Basis support for $(btype)" begin # COV_EXCL_LINE
             p = pts[1]
             b = get_basis(M, p, btype)
             Test.@test isa(b, CachedBasis)
@@ -713,7 +713,7 @@ function test_manifold(
         end
     end
 
-    Test.@testset "number_eltype" begin
+    Test.@testset "number_eltype" begin # COV_EXCL_LINE
         for (p, X) in zip(pts, tv)
             Test.@test number_eltype(X) == number_eltype(p)
             p = retract(M, p, X, default_retraction_method)
@@ -822,7 +822,7 @@ function test_manifold(
         end
     end
 
-    Test.@testset "tangent vector distributions" begin
+    Test.@testset "tangent vector distributions" begin # COV_EXCL_LINE
         for tvd in tvector_distributions
             supp = Manifolds.support(tvd)
             Test.@test supp isa
@@ -864,9 +864,10 @@ function test_parallel_transport(
 )
     length(P) < 2 &&
         error("The Parallel Transport test set requires at least 2 points in P")
-    Test.@testset "Test Parallel Transport" begin
+    Test.@testset "Test Parallel Transport" begin # COV_EXCL_LINE
         along && @warn "parallel transport along test not yet implemented"
-        Test.@testset "To (a point)" begin # even with to =false this displays no tests
+        Test.@testset "To (a point)" begin # COV_EXCL_LINE
+            # even with to =false this displays no tests
             if to
                 for i in 1:(length(P) - 1)
                     p = P[i]
@@ -890,7 +891,7 @@ function test_parallel_transport(
                 end
             end
         end
-        Test.@testset "(Tangent Vector) Direction" begin
+        Test.@testset "(Tangent Vector) Direction" begin # COV_EXCL_LINE
             if direction
                 for i in 1:(length(P) - 1)
                     p = P[i]