From 3ffe741fdc9c435e4b67837dbbb67919752b4d43 Mon Sep 17 00:00:00 2001
From: Mateusz Baran <mateuszbaran89@gmail.com>
Date: Sat, 14 Nov 2020 15:38:28 +0100
Subject: [PATCH] Some work on making groups embedded (#251)

* some work on making groups embedded

* additionally to just TransparentIsometric the AbstractGroupManifold should also be transparent wrt vector/coordinates.

* runs formatter.

* fixed special orthogonal test

* Update src/manifolds/Rotations.jl

Co-authored-by: Seth Axen <seth.axen@gmail.com>

* version bumped

Co-authored-by: Ronny Bergmann <git@ronnybergmann.net>
Co-authored-by: Seth Axen <seth.axen@gmail.com>
---
 Project.toml                      |  2 +-
 src/groups/group.jl               | 38 ++++++++++++++++++++++---------
 src/manifolds/Rotations.jl        |  5 +++-
 test/groups/special_orthogonal.jl |  2 +-
 4 files changed, 33 insertions(+), 14 deletions(-)

diff --git a/Project.toml b/Project.toml
index ad15395453..a224b5425c 100644
--- a/Project.toml
+++ b/Project.toml
@@ -1,7 +1,7 @@
 name = "Manifolds"
 uuid = "1cead3c2-87b3-11e9-0ccd-23c62b72b94e"
 authors = ["Seth Axen <seth.axen@gmail.com>", "Mateusz Baran <mateuszbaran89@gmail.com>", "Ronny Bergmann <manopt@ronnybergmann.net>", "Antoine Levitt <antoine.levitt@gmail.com>"]
-version = "0.4.5"
+version = "0.4.6"
 
 [deps]
 Distributions = "31c24e10-a181-5473-b8eb-7969acd0382f"
diff --git a/src/groups/group.jl b/src/groups/group.jl
index f3f2dd8ce4..82149e3c22 100644
--- a/src/groups/group.jl
+++ b/src/groups/group.jl
@@ -29,8 +29,8 @@ Abstract type for a Lie group, a group that is also a smooth manifold with an
 implement at least [`inv`](@ref), [`identity`](@ref), [`compose`](@ref), and
 [`translate_diff`](@ref).
 """
-abstract type AbstractGroupManifold{𝔽,O<:AbstractGroupOperation} <:
-              AbstractDecoratorManifold{𝔽} end
+abstract type AbstractGroupManifold{𝔽,O<:AbstractGroupOperation,T<:AbstractEmbeddingType} <:
+              AbstractEmbeddedManifold{𝔽,T} end
 
 """
     GroupManifold{𝔽,M<:Manifold{𝔽},O<:AbstractGroupOperation} <: AbstractGroupManifold{𝔽,O}
@@ -45,7 +45,7 @@ Group manifolds by default forward metric-related operations to the wrapped mani
     GroupManifold(manifold, op)
 """
 struct GroupManifold{𝔽,M<:Manifold{𝔽},O<:AbstractGroupOperation} <:
-       AbstractGroupManifold{𝔽,O}
+       AbstractGroupManifold{𝔽,O,TransparentIsometricEmbedding}
     manifold::M
     op::O
 end
@@ -67,11 +67,13 @@ function base_group(M::Manifold)
 end
 base_group(G::AbstractGroupManifold) = G
 
-decorator_group_dispatch(M::Manifold) = Val(false)
+base_manifold(G::GroupManifold) = G.manifold
+
+decorator_group_dispatch(::Manifold) = Val(false)
 function decorator_group_dispatch(M::AbstractDecoratorManifold)
     return decorator_group_dispatch(decorated_manifold(M))
 end
-decorator_group_dispatch(M::AbstractGroupManifold) = Val(true)
+decorator_group_dispatch(::AbstractGroupManifold) = Val(true)
 
 function is_group_decorator(M::Manifold)
     return _extract_val(decorator_group_dispatch(M))
@@ -85,6 +87,22 @@ if VERSION ≥ v"1.3"
     (::Type{T})(M::Manifold) where {T<:AbstractGroupOperation} = GroupManifold(M, T())
 end
 
+function decorator_transparent_dispatch(
+    ::typeof(get_coordinates!),
+    ::AbstractGroupManifold,
+    args...,
+)
+    return Val(:transparent)
+end
+function decorator_transparent_dispatch(
+    ::typeof(get_vector!),
+    ::AbstractGroupManifold,
+    args...,
+)
+    return Val(:transparent)
+end
+
+
 ###################
 # Action directions
 ###################
@@ -322,14 +340,12 @@ for MT in GROUP_MANIFOLD_BASIS_DISAMBIGUATION
     )
 end
 
-@decorator_transparent_fallback :transparent function check_manifold_point(
-    G::AbstractGroupManifold,
-    e::Identity;
-    kwargs...,
-)
+manifold_dimension(G::GroupManifold) = manifold_dimension(G.manifold)
+
+function check_manifold_point(G::AbstractGroupManifold, e::Identity; kwargs...)
     return DomainError(e, "The identity element $(e) does not belong to $(G).")
 end
-@decorator_transparent_fallback :transparent function check_manifold_point(
+function check_manifold_point(
     G::GT,
     e::Identity{GT};
     kwargs...,
diff --git a/src/manifolds/Rotations.jl b/src/manifolds/Rotations.jl
index 1bede6186a..aa26b6be0e 100644
--- a/src/manifolds/Rotations.jl
+++ b/src/manifolds/Rotations.jl
@@ -79,7 +79,10 @@ function check_manifold_point(M::Rotations{N}, p; kwargs...) where {N}
         return DomainError(det(p), "The determinant of $p has to be +1 but it is $(det(p))")
     end
     if !isapprox(transpose(p) * p, one(p); kwargs...)
-        return DomainError(norm(p), "$p has to be orthogonal but it's not")
+        return DomainError(
+            norm(transpose(p) * p - one(p)),
+            "$p must be orthogonal but it's not at kwargs $kwargs",
+        )
     end
     return nothing
 end
diff --git a/test/groups/special_orthogonal.jl b/test/groups/special_orthogonal.jl
index 8f8f98d82c..8e3f843d33 100644
--- a/test/groups/special_orthogonal.jl
+++ b/test/groups/special_orthogonal.jl
@@ -90,7 +90,7 @@ include("group_utils.jl")
             inverse_retraction_methods = inverse_retraction_methods,
             exp_log_atol_multiplier = 20,
             retraction_atol_multiplier = 12,
-            is_tangent_atol_multiplier = 1,
+            is_tangent_atol_multiplier = 1.2,
         )
 
         @test injectivity_radius(G) == injectivity_radius(M)