Repository: JuliaManifolds/ManifoldsBase.jl
Pull request #48 (2020-09-11 18:26:26)

Fixing embedded manifolds
This is a bit more involved than I originally thought it would be. It should fix the issue noticed here: https://github.com/JuliaManifolds/Manifolds.jl/pull/216#discussion_r487152910 . There are also a few related fixes. Let me know if this makes sense.

codecov[bot]: # [Codecov](https://codecov.io/gh/JuliaManifolds/ManifoldsBase.jl/pull/48?src=pr&el=h1) Report
> Merging [#48](https://codecov.io/gh/JuliaManifolds/ManifoldsBase.jl/pull/48?src=pr&el=desc) into [master](https://codecov.io/gh/JuliaManifolds/ManifoldsBase.jl/commit/7471d948b4e5a0485cff823313233fb2397c485b?el=desc) will **decrease** coverage by `0.52%`.
> The diff coverage is `100.00%`.

[![Impacted file tree graph](https://codecov.io/gh/JuliaManifolds/ManifoldsBase.jl/pull/48/graphs/tree.svg?width=650&height=150&src=pr&token=bQsBUU9knE)](https://codecov.io/gh/JuliaManifolds/ManifoldsBase.jl/pull/48?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master      #48      +/-   ##
==========================================
- Coverage   99.61%   99.08%   -0.53%     
==========================================
  Files           8        8              
  Lines         770      768       -2     
==========================================
- Hits          767      761       -6     
- Misses          3        7       +4     
```


| [Impacted Files](https://codecov.io/gh/JuliaManifolds/ManifoldsBase.jl/pull/48?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [src/EmbeddedManifold.jl](https://codecov.io/gh/JuliaManifolds/ManifoldsBase.jl/pull/48/diff?src=pr&el=tree#diff-c3JjL0VtYmVkZGVkTWFuaWZvbGQuamw=) | `98.70% <100.00%> (+0.07%)` | :arrow_up: |
| [src/ManifoldsBase.jl](https://codecov.io/gh/JuliaManifolds/ManifoldsBase.jl/pull/48/diff?src=pr&el=tree#diff-c3JjL01hbmlmb2xkc0Jhc2Uuamw=) | `97.29% <0.00%> (-1.81%)` | :arrow_down: |
| [src/bases.jl](https://codecov.io/gh/JuliaManifolds/ManifoldsBase.jl/pull/48/diff?src=pr&el=tree#diff-c3JjL2Jhc2VzLmps) | `98.18% <0.00%> (-1.22%)` | :arrow_down: |
| [src/DecoratorManifold.jl](https://codecov.io/gh/JuliaManifolds/ManifoldsBase.jl/pull/48/diff?src=pr&el=tree#diff-c3JjL0RlY29yYXRvck1hbmlmb2xkLmps) | `100.00% <0.00%> (ø)` | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/JuliaManifolds/ManifoldsBase.jl/pull/48?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/JuliaManifolds/ManifoldsBase.jl/pull/48?src=pr&el=footer). Last update [7471d94...3ebe905](https://codecov.io/gh/JuliaManifolds/ManifoldsBase.jl/pull/48?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

mateuszbaran: I think the PR should be OK now :slightly_smiling_face: .

Patch:
diff --git a/Project.toml b/Project.toml
index 5bd8875d..6fc691cc 100644
--- a/Project.toml
+++ b/Project.toml
@@ -1,7 +1,7 @@
 name = "ManifoldsBase"
 uuid = "3362f125-f0bb-47a3-aa74-596ffd7ef2fb"
 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.9.1"
+version = "0.9.2"
 
 [deps]
 LinearAlgebra = "37e2e46d-f89d-539d-b4ee-838fcccc9c8e"
diff --git a/src/DefaultManifold.jl b/src/DefaultManifold.jl
index 69a13aab..dfa73ee8 100644
--- a/src/DefaultManifold.jl
+++ b/src/DefaultManifold.jl
@@ -88,7 +88,7 @@ norm(::DefaultManifold, p, X) = norm(X)
 project!(::DefaultManifold, q, p) = copyto!(q, p)
 project!(::DefaultManifold, Y, p, X) = copyto!(Y, X)
 
-@generated representation_size(::DefaultManifold{T}) where {T} = Tuple(T.parameters...)
+@generated representation_size(::DefaultManifold{T}) where {T} = tuple(T.parameters...)
 
 function vector_transport_along!(
     ::DefaultManifold,
diff --git a/src/EmbeddedManifold.jl b/src/EmbeddedManifold.jl
index 63ff0f1d..dfccdf14 100644
--- a/src/EmbeddedManifold.jl
+++ b/src/EmbeddedManifold.jl
@@ -15,6 +15,12 @@ The embedding is further specified by an [`AbstractEmbeddingType`](@ref).
 This means, that technically an embedded manifold is a decorator for the embedding, i.e.
 functions of this type get, in the semi-transparent way of the
 [`AbstractDecoratorManifold`](@ref), passed on to the embedding.
+
+!!! note
+    
+    Points on an `AbstractEmbeddedManifold` are still represented using representation
+    of the embedded manifold. Use [`embed`](@ref) to go to the representation of the embedding
+    and [`project`](@ref) to go the other way.
 """
 abstract type AbstractEmbeddedManifold{𝔽,T<:AbstractEmbeddingType} <:
               AbstractDecoratorManifold{𝔽} end
@@ -89,6 +95,16 @@ function EmbeddedManifold(
     return EmbeddedManifold{𝔽,MT,NT,ET}(M, N)
 end
 
+function allocate_result(M::AbstractEmbeddedManifold, f::typeof(embed), x...)
+    T = allocate_result_type(M, f, x)
+    return allocate(x[end], T, representation_size(decorated_manifold(M)))
+end
+
+function allocate_result(M::AbstractEmbeddedManifold, f::typeof(project), x...)
+    T = allocate_result_type(M, f, x)
+    return allocate(x[end], T, representation_size(base_manifold(M)))
+end
+
 """
     base_manifold(M::AbstractEmbeddedManifold, d::Val{N} = Val(-1))
 
@@ -128,7 +144,7 @@ end
 """
     check_tangent_vector(M::AbstractEmbeddedManifold, p, X; check_base_point = true, kwargs...)
 
-check that `embed(M,p,X)` is a valid tangent to `embed(p,X)`, where `check_base_point`
+check that `embed(M, p, X)` is a valid tangent to `embed(M, p)`, where `check_base_point`
 determines whether the validity of `p` is checked, too.
 """
 function check_tangent_vector(


Test patch:
diff --git a/test/bases.jl b/test/bases.jl
index bfe36fa4..ce18276c 100644
--- a/test/bases.jl
+++ b/test/bases.jl
@@ -339,139 +339,71 @@ end
     @test sprint(show, ProjectedOrthonormalBasis(:gram_schmidt, ℂ)) ==
           "ProjectedOrthonormalBasis(:gram_schmidt, ℂ)"
 
-    if VERSION >= v"1.6.0-DEV.430"
-        @test sprint(show, "text/plain", DiagonalizingOrthonormalBasis(Float64[1, 2, 3])) ==
-              """
-DiagonalizingOrthonormalBasis(ℝ) with eigenvalue 0 in direction:
-3-element Vector{Float64}:
-  1.0
-  2.0
-  3.0"""
-    else
-        @test sprint(show, "text/plain", DiagonalizingOrthonormalBasis(Float64[1, 2, 3])) ==
-              """
-DiagonalizingOrthonormalBasis(ℝ) with eigenvalue 0 in direction:
-3-element Array{Float64,1}:
-  1.0
-  2.0
-  3.0"""
-    end
+    @test sprint(show, "text/plain", DiagonalizingOrthonormalBasis(Float64[1, 2, 3])) == """
+                                                                               DiagonalizingOrthonormalBasis(ℝ) with eigenvalue 0 in direction:
+                                                                               3-element $(sprint(show, Vector{Float64})):
+                                                                                 1.0
+                                                                                 2.0
+                                                                                 3.0"""
 
     M = DefaultManifold(2, 3)
     x = collect(reshape(1.0:6.0, (2, 3)))
     pb = get_basis(M, x, DefaultOrthonormalBasis())
-    if VERSION >= v"1.6.0-DEV.430"
-        @test sprint(show, "text/plain", pb) == """
-        DefaultOrthonormalBasis(ℝ) with 6 basis vectors:
-         E1 =
-          2×3 Matrix{Float64}:
-           1.0  0.0  0.0
-           0.0  0.0  0.0
-         E2 =
-          2×3 Matrix{Float64}:
-           0.0  0.0  0.0
-           1.0  0.0  0.0
-         ⋮
-         E5 =
-          2×3 Matrix{Float64}:
-           0.0  0.0  1.0
-           0.0  0.0  0.0
-         E6 =
-          2×3 Matrix{Float64}:
-           0.0  0.0  0.0
-           0.0  0.0  1.0"""
-    else
-        @test sprint(show, "text/plain", pb) == """
-        DefaultOrthonormalBasis(ℝ) with 6 basis vectors:
-         E1 =
-          2×3 Array{Float64,2}:
-           1.0  0.0  0.0
-           0.0  0.0  0.0
-         E2 =
-          2×3 Array{Float64,2}:
-           0.0  0.0  0.0
-           1.0  0.0  0.0
-         ⋮
-         E5 =
-          2×3 Array{Float64,2}:
-           0.0  0.0  1.0
-           0.0  0.0  0.0
-         E6 =
-          2×3 Array{Float64,2}:
-           0.0  0.0  0.0
-           0.0  0.0  1.0"""
-    end
+
+    @test sprint(show, "text/plain", pb) == """
+    DefaultOrthonormalBasis(ℝ) with 6 basis vectors:
+     E1 =
+      2×3 $(sprint(show, Matrix{Float64})):
+       1.0  0.0  0.0
+       0.0  0.0  0.0
+     E2 =
+      2×3 $(sprint(show, Matrix{Float64})):
+       0.0  0.0  0.0
+       1.0  0.0  0.0
+     ⋮
+     E5 =
+      2×3 $(sprint(show, Matrix{Float64})):
+       0.0  0.0  1.0
+       0.0  0.0  0.0
+     E6 =
+      2×3 $(sprint(show, Matrix{Float64})):
+       0.0  0.0  0.0
+       0.0  0.0  1.0"""
 
     b = DiagonalizingOrthonormalBasis(get_vectors(M, x, pb)[1])
     dpb = CachedBasis(b, Float64[1, 2, 3, 4, 5, 6], get_vectors(M, x, pb))
-    if VERSION >= v"1.6.0-DEV.430"
-        @test sprint(show, "text/plain", dpb) == """
-        DiagonalizingOrthonormalBasis(ℝ) with eigenvalue 0 in direction:
-         2×3 Matrix{Float64}:
-           1.0  0.0  0.0
-           0.0  0.0  0.0
-        and 6 basis vectors.
-        Basis vectors:
-         E1 =
-          2×3 Matrix{Float64}:
-           1.0  0.0  0.0
-           0.0  0.0  0.0
-         E2 =
-          2×3 Matrix{Float64}:
-           0.0  0.0  0.0
-           1.0  0.0  0.0
-         ⋮
-         E5 =
-          2×3 Matrix{Float64}:
-           0.0  0.0  1.0
-           0.0  0.0  0.0
-         E6 =
-          2×3 Matrix{Float64}:
-           0.0  0.0  0.0
-           0.0  0.0  1.0
-        Eigenvalues:
-         6-element Vector{Float64}:
-          1.0
-          2.0
-          3.0
-          4.0
-          5.0
-          6.0"""
-    else
-        @test sprint(show, "text/plain", dpb) == """
-        DiagonalizingOrthonormalBasis(ℝ) with eigenvalue 0 in direction:
-         2×3 Array{Float64,2}:
-           1.0  0.0  0.0
-           0.0  0.0  0.0
-        and 6 basis vectors.
-        Basis vectors:
-         E1 =
-          2×3 Array{Float64,2}:
-           1.0  0.0  0.0
-           0.0  0.0  0.0
-         E2 =
-          2×3 Array{Float64,2}:
-           0.0  0.0  0.0
-           1.0  0.0  0.0
-         ⋮
-         E5 =
-          2×3 Array{Float64,2}:
-           0.0  0.0  1.0
-           0.0  0.0  0.0
-         E6 =
-          2×3 Array{Float64,2}:
-           0.0  0.0  0.0
-           0.0  0.0  1.0
-        Eigenvalues:
-         6-element Array{Float64,1}:
-          1.0
-          2.0
-          3.0
-          4.0
-          5.0
-          6.0"""
-    end
-
+    @test sprint(show, "text/plain", dpb) == """
+    DiagonalizingOrthonormalBasis(ℝ) with eigenvalue 0 in direction:
+     2×3 $(sprint(show, Matrix{Float64})):
+       1.0  0.0  0.0
+       0.0  0.0  0.0
+    and 6 basis vectors.
+    Basis vectors:
+     E1 =
+      2×3 $(sprint(show, Matrix{Float64})):
+       1.0  0.0  0.0
+       0.0  0.0  0.0
+     E2 =
+      2×3 $(sprint(show, Matrix{Float64})):
+       0.0  0.0  0.0
+       1.0  0.0  0.0
+     ⋮
+     E5 =
+      2×3 $(sprint(show, Matrix{Float64})):
+       0.0  0.0  1.0
+       0.0  0.0  0.0
+     E6 =
+      2×3 $(sprint(show, Matrix{Float64})):
+       0.0  0.0  0.0
+       0.0  0.0  1.0
+    Eigenvalues:
+     6-element $(sprint(show, Vector{Float64})):
+      1.0
+      2.0
+      3.0
+      4.0
+      5.0
+      6.0"""
 
     M = DefaultManifold(1, 1, 1)
     x = reshape(Float64[1], (1, 1, 1))
@@ -479,7 +411,7 @@ DiagonalizingOrthonormalBasis(ℝ) with eigenvalue 0 in direction:
     @test sprint(show, "text/plain", pb) == """
     DefaultOrthonormalBasis(ℝ) with 1 basis vector:
      E1 =
-      1×1×1 Array{Float64,3}:
+      1×1×1 $(sprint(show, Array{Float64,3})):
       [:, :, 1] =
        1.0"""
 
@@ -488,33 +420,18 @@ DiagonalizingOrthonormalBasis(ℝ) with eigenvalue 0 in direction:
         Float64[1],
         get_vectors(M, x, pb),
     )
-    if VERSION >= v"1.6.0-DEV.430"
-        @test sprint(show, "text/plain", dpb) == """
-        DiagonalizingOrthonormalBasis(ℝ) with eigenvalue 0 in direction:
-         1-element Vector{Array{Float64,3}}:
-           [1.0]
-        and 1 basis vector.
-        Basis vectors:
-         E1 =
-          1×1×1 Array{Float64,3}:
-          [:, :, 1] =
-           1.0
-        Eigenvalues:
-         1-element Vector{Float64}:
-          1.0"""
-    else
-        @test sprint(show, "text/plain", dpb) == """
-        DiagonalizingOrthonormalBasis(ℝ) with eigenvalue 0 in direction:
-         1-element Array{Array{Float64,3},1}:
-           [1.0]
-        and 1 basis vector.
-        Basis vectors:
-         E1 =
-          1×1×1 Array{Float64,3}:
-          [:, :, 1] =
-           1.0
-        Eigenvalues:
-         1-element Array{Float64,1}:
-          1.0"""
-    end
+
+    @test sprint(show, "text/plain", dpb) == """
+    DiagonalizingOrthonormalBasis(ℝ) with eigenvalue 0 in direction:
+     1-element $(sprint(show, Vector{Array{Float64,3}})):
+       [1.0]
+    and 1 basis vector.
+    Basis vectors:
+     E1 =
+      1×1×1 $(sprint(show, Array{Float64,3})):
+      [:, :, 1] =
+       1.0
+    Eigenvalues:
+     1-element $(sprint(show, Vector{Float64})):
+      1.0"""
 end
diff --git a/test/embedded_manifold.jl b/test/embedded_manifold.jl
index 5ea54685..6149e80c 100644
--- a/test/embedded_manifold.jl
+++ b/test/embedded_manifold.jl
@@ -1,6 +1,6 @@
 struct PlaneManifold <: AbstractEmbeddedManifold{ℝ,TransparentIsometricEmbedding} end
 
-ManifoldsBase.decorated_manifold(::PlaneManifold) = ManifoldsBase.DefaultManifold(3)
+ManifoldsBase.decorated_manifold(::PlaneManifold) = ManifoldsBase.DefaultManifold(1, 3)
 ManifoldsBase.base_manifold(::PlaneManifold) = ManifoldsBase.DefaultManifold(2)
 
 ManifoldsBase.embed!(::PlaneManifold, q, p) = copyto!(q, p)
@@ -8,6 +8,28 @@ ManifoldsBase.embed!(::PlaneManifold, Y, p, X) = copyto!(Y, X)
 ManifoldsBase.project!(::PlaneManifold, q, p) = (q .= [p[1] p[2] 0.0])
 ManifoldsBase.project!(::PlaneManifold, Y, p, X) = (Y .= [X[1] X[2] 0.0])
 
+struct AnotherPlaneManifold <: AbstractEmbeddedManifold{ℝ,DefaultIsometricEmbeddingType} end
+
+ManifoldsBase.decorated_manifold(::AnotherPlaneManifold) = ManifoldsBase.DefaultManifold(3)
+ManifoldsBase.base_manifold(::AnotherPlaneManifold) = ManifoldsBase.DefaultManifold(2)
+
+function ManifoldsBase.embed!(::AnotherPlaneManifold, q, p)
+    q[1:2] .= p
+    q[3] = 0
+    return q
+end
+function ManifoldsBase.embed!(::AnotherPlaneManifold, Y, p, X)
+    Y[1:2] .= X
+    Y[3] = 0
+    return Y
+end
+function ManifoldsBase.project!(::AnotherPlaneManifold, q, p)
+    return q .= [p[1], p[2]]
+end
+function ManifoldsBase.project!(::AnotherPlaneManifold, Y, p, X)
+    return Y .= [X[1], X[2]]
+end
+
 struct NotImplementedEmbeddedManifold <:
        AbstractEmbeddedManifold{ℝ,TransparentIsometricEmbedding} end
 function ManifoldsBase.decorated_manifold(::NotImplementedEmbeddedManifold)
@@ -29,7 +51,7 @@ struct NotImplementedEmbeddedManifold3 <: AbstractEmbeddedManifold{ℝ,DefaultEm
             ManifoldsBase.DefaultManifold(3),
         )
         @test repr(M) ==
-              "EmbeddedManifold(DefaultManifold{Tuple{2},ℝ}(), DefaultManifold{Tuple{3},ℝ}(), TransparentIsometricEmbedding())"
+              "EmbeddedManifold($(sprint(show, M.manifold)), $(sprint(show, M.embedding)), TransparentIsometricEmbedding())"
         @test base_manifold(M) == ManifoldsBase.DefaultManifold(2)
         @test ManifoldsBase.decorated_manifold(M) == ManifoldsBase.DefaultManifold(3)
         @test ManifoldsBase.default_embedding_dispatch(M) === Val{false}()
@@ -40,11 +62,11 @@ struct NotImplementedEmbeddedManifold3 <: AbstractEmbeddedManifold{ℝ,DefaultEm
         M = PlaneManifold()
         @test repr(M) == "PlaneManifold()"
         @test ManifoldsBase.default_decorator_dispatch(M) === Val{false}()
-        @test get_embedding(M) == ManifoldsBase.DefaultManifold(3)
+        @test get_embedding(M) == ManifoldsBase.DefaultManifold(1, 3)
         # Check fallbacks to check embed->check_manifoldpoint Defaults
-        @test_throws DomainError is_manifold_point(M, [1, 0], true)
-        @test_throws DomainError is_tangent_vector(M, [1, 0], [1, 0, 0], true)
-        @test_throws DomainError is_tangent_vector(M, [1, 0, 0], [0, 0], true)
+        @test is_manifold_point(M, [1, 0], true)
+        @test is_tangent_vector(M, [1, 0], [1, 0, 0], true)
+        @test is_tangent_vector(M, [1, 0, 0], [0, 0], true)
         p = [1.0 1.0 0.0]
         q = [1.0 0.0 0.0]
         X = q - p
@@ -67,6 +89,18 @@ struct NotImplementedEmbeddedManifold3 <: AbstractEmbeddedManifold{ℝ,DefaultEm
         @test r == q
     end
 
+    @testset "AnotherPlaneManifold" begin
+        M = AnotherPlaneManifold()
+        p = [1.0, 2.0]
+        pe = embed(M, p)
+        @test pe == [1.0, 2.0, 0.0]
+        X = [2.0, 3.0]
+        Xe = embed(M, pe, X)
+        @test Xe == [2.0, 3.0, 0.0]
+        @test project(M, pe) == p
+        @test project(M, pe, Xe) == X
+    end
+
     @testset "Test nonimplemented fallbacks" begin
         @testset "Default Isometric Embedding Fallback Error Tests" begin
             M = NotImplementedEmbeddedManifold()