Skip to content

Commit

Permalink
Improve Test style and coverage (#319)
Browse files Browse the repository at this point in the history
* refactor runtests to work ina single testset and still print the interims tests via @info.

* increase test covaerage and resolve an error for projection on the PoincareBallTVectors

* Increase test coverage further.

* runs formatter.

* adds another small test for utils.

* Fix a test on group general

* trying to cover two things

* removing eigvals and some old version tests

* updates for distributions and formatting

* add support of NormalRotationDistribution

* metric test updates

* run formatter

* remove one more version check

* typo in docs

* fixes a small error that always compared SArrays to SArrays

such that the Array variant was never used and never checked/compared.

* removes a line that is never reached (note the injectivity atop before the loop)

* Revert "removes a line that is never reached (note the injectivity atop before the loop)"

This reverts commit 6e71c6c.

Co-authored-by: Mateusz Baran <[email protected]>
  • Loading branch information
kellertuer and mateuszbaran authored Jan 8, 2021
1 parent 2b3da3a commit 85e3940
Show file tree
Hide file tree
Showing 50 changed files with 581 additions and 450 deletions.
7 changes: 4 additions & 3 deletions docs/src/features/distributions.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@ Pages = ["distributions.jl"]
Order = [:type, :function]
```

```@docs
Manifolds.ProjectedPointDistribution
Manifolds.ProjectedFVectorDistribution
```@autodocs
Modules = [Manifolds]
Pages = ["projected_distribution.jl"]
Order = [:type, :function]
```
9 changes: 0 additions & 9 deletions src/distributions.jl
Original file line number Diff line number Diff line change
Expand Up @@ -62,12 +62,3 @@ Get the object of type `FVectorSupport` for the distribution `d`.
function Distributions.support(::T) where {T<:FVectorDistribution}
return error("support not implemented for type $T")
end

@decorator_transparent_signature normal_tvector_distribution(
M::AbstractDecoratorManifold,
p,
σ,
)

@decorator_transparent_signature projected_distribution(M::AbstractDecoratorManifold, d, p)
@decorator_transparent_signature projected_distribution(M::AbstractDecoratorManifold, d)
26 changes: 12 additions & 14 deletions src/groups/array_manifold.jl
Original file line number Diff line number Diff line change
Expand Up @@ -91,13 +91,9 @@ function translate_diff(M::ValidationManifold, p, q, X, conv::ActionDirection; k
is_manifold_point(M, p, true; kwargs...)
is_manifold_point(M, q, true; kwargs...)
is_tangent_vector(M, q, X, true; kwargs...)
Y = ValidationTVector(translate_diff(
M.manifold,
array_value(p),
array_value(q),
array_value(X),
conv,
))
Y = ValidationTVector(
translate_diff(M.manifold, array_value(p), array_value(q), array_value(X), conv),
)
pq = translate(M, p, q, conv)
is_tangent_vector(M, pq, Y, true; kwargs...)
return Y
Expand Down Expand Up @@ -139,13 +135,15 @@ function inverse_translate_diff(
is_manifold_point(M, p, true; kwargs...)
is_manifold_point(M, q, true; kwargs...)
is_tangent_vector(M, q, X, true; kwargs...)
Y = ValidationTVector(inverse_translate_diff(
M.manifold,
array_value(p),
array_value(q),
array_value(X),
conv,
))
Y = ValidationTVector(
inverse_translate_diff(
M.manifold,
array_value(p),
array_value(q),
array_value(X),
conv,
),
)
pinvq = inverse_translate(M, p, q, conv)
is_tangent_vector(M, pinvq, Y, true; kwargs...)
return Y
Expand Down
24 changes: 18 additions & 6 deletions src/groups/group.jl
Original file line number Diff line number Diff line change
Expand Up @@ -194,10 +194,14 @@ function decorator_transparent_dispatch(
end
function allocate_result(M::Manifold, f::typeof(get_vector), e::Identity, Xⁱ)
is_group_decorator(M) && return allocate_result(base_group(M), f, e, Xⁱ)
return error("allocate_result not implemented for manifold $(M), function $(f), point $(e), and vector $(Xⁱ).")
return error(
"allocate_result not implemented for manifold $(M), function $(f), point $(e), and vector $(Xⁱ).",
)
end
function allocate_result(M::AbstractGroupManifold, f::typeof(get_vector), e::Identity, Xⁱ)
return error("allocate_result not implemented for group manifold $(M), function $(f), $(e), and vector $(Xⁱ).")
return error(
"allocate_result not implemented for group manifold $(M), function $(f), $(e), and vector $(Xⁱ).",
)
end
function allocate_result(
G::GT,
Expand All @@ -223,15 +227,19 @@ function allocate_result(
X,
)
is_group_decorator(M) && return allocate_result(base_group(M), f, e, X)
return error("allocate_result not implemented for manifold $(M), function $(f), point $(e), and vector $(X).")
return error(
"allocate_result not implemented for manifold $(M), function $(f), point $(e), and vector $(X).",
)
end
function allocate_result(
M::AbstractGroupManifold,
f::typeof(get_coordinates),
e::Identity,
X,
)
return error("allocate_result not implemented for group manifold $(M), function $(f), $(e), and vector $(X).")
return error(
"allocate_result not implemented for group manifold $(M), function $(f), $(e), and vector $(X).",
)
end
function allocate_result(
G::GT,
Expand Down Expand Up @@ -1025,7 +1033,9 @@ Base.identity(::MultiplicationGroup, p) = one(p)

function identity!(G::GT, q, p) where {GT<:MultiplicationGroup}
isa(p, Identity{GT}) || return copyto!(q, one(p))
return error("identity! not implemented on $(typeof(G)) for points $(typeof(q)) and $(typeof(p))")
return error(
"identity! not implemented on $(typeof(G)) for points $(typeof(q)) and $(typeof(p))",
)
end
identity!(::MultiplicationGroup, q::AbstractMatrix, p) = copyto!(q, I)

Expand All @@ -1047,5 +1057,7 @@ end

function group_exp!(G::MultiplicationGroup, q, X)
X isa Union{Number,AbstractMatrix} && return copyto!(q, exp(X))
return error("group_exp! not implemented on $(typeof(G)) for vector $(typeof(X)) and element $(typeof(q)).")
return error(
"group_exp! not implemented on $(typeof(G)) for vector $(typeof(X)) and element $(typeof(q)).",
)
end
16 changes: 12 additions & 4 deletions src/groups/group_action.jl
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,9 @@ Apply action `a` to the point `p` with the rule specified by `A`.
The result is saved in `q`.
"""
function apply!(A::AbstractGroupAction{LeftAction}, q, a, p)
return error("apply! not implemented for action $(typeof(A)) and points $(typeof(q)), $(typeof(p)) and $(typeof(a)).")
return error(
"apply! not implemented for action $(typeof(A)) and points $(typeof(q)), $(typeof(p)) and $(typeof(a)).",
)
end
function apply!(A::AbstractGroupAction{RightAction}, q, a, p)
ainv = inv(base_group(A), a)
Expand Down Expand Up @@ -91,11 +93,15 @@ differential transports vectors
````
"""
function apply_diff(A::AbstractGroupAction, a, p, X)
return error("apply_diff not implemented for action $(typeof(A)), points $(typeof(a)) and $(typeof(p)), and vector $(typeof(X))")
return error(
"apply_diff not implemented for action $(typeof(A)), points $(typeof(a)) and $(typeof(p)), and vector $(typeof(X))",
)
end

function apply_diff!(A::AbstractGroupAction, Y, a, p, X)
return error("apply_diff! not implemented for action $(typeof(A)), points $(typeof(a)) and $(typeof(p)), vectors $(typeof(Y)) and $(typeof(X))")
return error(
"apply_diff! not implemented for action $(typeof(A)), points $(typeof(a)) and $(typeof(p)), vectors $(typeof(Y)) and $(typeof(X))",
)
end

@doc raw"""
Expand Down Expand Up @@ -135,7 +141,9 @@ the element closest to `q` in the metric of the G-manifold:
where $\mathcal{G}$ is the group that acts on the G-manifold $\mathcal M$.
"""
function optimal_alignment(A::AbstractGroupAction, p, q)
return error("optimal_alignment not implemented for $(typeof(A)) and points $(typeof(p)) and $(typeof(q)).")
return error(
"optimal_alignment not implemented for $(typeof(A)) and points $(typeof(p)) and $(typeof(q)).",
)
end

"""
Expand Down
82 changes: 46 additions & 36 deletions src/groups/product_group.jl
Original file line number Diff line number Diff line change
Expand Up @@ -100,12 +100,14 @@ compose(G::GT, ::Identity{GT}, p) where {GT<:ProductGroup} = p
compose(G::GT, p, ::Identity{GT}) where {GT<:ProductGroup} = p
compose(G::GT, e::E, ::E) where {GT<:ProductGroup,E<:Identity{GT}} = e
function compose(M::ProductManifold, p::ProductRepr, q::ProductRepr)
return ProductRepr(map(
compose,
M.manifolds,
submanifold_components(M, p),
submanifold_components(M, q),
)...)
return ProductRepr(
map(
compose,
M.manifolds,
submanifold_components(M, p),
submanifold_components(M, q),
)...,
)
end
function compose(M::ProductManifold, p, q)
x = allocate_result(M, compose, p, q)
Expand All @@ -131,13 +133,15 @@ function translate(
q::ProductRepr,
conv::ActionDirection,
)
return ProductRepr(map(
translate,
M.manifolds,
submanifold_components(M, p),
submanifold_components(M, q),
repeated(conv),
)...)
return ProductRepr(
map(
translate,
M.manifolds,
submanifold_components(M, p),
submanifold_components(M, q),
repeated(conv),
)...,
)
end
function translate(M::ProductManifold, p, q, conv::ActionDirection)
x = allocate_result(M, translate, p, q)
Expand Down Expand Up @@ -168,13 +172,15 @@ function inverse_translate(
q::ProductRepr,
conv::ActionDirection,
)
return ProductRepr(map(
inverse_translate,
M.manifolds,
submanifold_components(M, p),
submanifold_components(M, q),
repeated(conv),
)...)
return ProductRepr(
map(
inverse_translate,
M.manifolds,
submanifold_components(M, p),
submanifold_components(M, q),
repeated(conv),
)...,
)
end
function inverse_translate(M::ProductManifold, p, q, conv::ActionDirection)
x = allocate_result(M, inverse_translate, p, q)
Expand Down Expand Up @@ -206,14 +212,16 @@ function translate_diff(
X::ProductRepr,
conv::ActionDirection,
)
return ProductRepr(map(
translate_diff,
M.manifolds,
submanifold_components(M, p),
submanifold_components(M, q),
submanifold_components(M, X),
repeated(conv),
)...)
return ProductRepr(
map(
translate_diff,
M.manifolds,
submanifold_components(M, p),
submanifold_components(M, q),
submanifold_components(M, X),
repeated(conv),
)...,
)
end
function translate_diff(M::ProductManifold, p, q, X, conv::ActionDirection)
Y = allocate_result(M, translate_diff, X, p, q)
Expand Down Expand Up @@ -246,14 +254,16 @@ function inverse_translate_diff(
X::ProductRepr,
conv::ActionDirection,
)
return ProductRepr(map(
inverse_translate_diff,
M.manifolds,
submanifold_components(M, p),
submanifold_components(M, q),
submanifold_components(M, X),
repeated(conv),
)...)
return ProductRepr(
map(
inverse_translate_diff,
M.manifolds,
submanifold_components(M, p),
submanifold_components(M, q),
submanifold_components(M, X),
repeated(conv),
)...,
)
end
function inverse_translate_diff(M::ProductManifold, p, q, X, conv::ActionDirection)
Y = allocate_result(M, inverse_translate_diff, X, p, q)
Expand Down
4 changes: 3 additions & 1 deletion src/manifolds/CenteredMatrices.jl
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,9 @@ function check_manifold_point(M::CenteredMatrices{m,n,𝔽}, p; kwargs...) where
if !isapprox(sum(p, dims=1), zeros(1, n); kwargs...)
return DomainError(
p,
string("The point $(p) does not lie on $(M), since its columns do not sum to zero."),
string(
"The point $(p) does not lie on $(M), since its columns do not sum to zero.",
),
)
end
return nothing
Expand Down
10 changes: 6 additions & 4 deletions src/manifolds/Circle.jl
Original file line number Diff line number Diff line change
Expand Up @@ -356,8 +356,9 @@ angles.
mean(::Circle{ℂ}, ::Any)
function Statistics.mean(M::Circle{ℂ}, x::AbstractVector{<:Complex}; kwargs...)
s = sum(x)
abs(s) == 0 &&
return error("The mean for $(x) on $(M) is not defined/unique, since the sum of the complex numbers is zero")
abs(s) == 0 && return error(
"The mean for $(x) on $(M) is not defined/unique, since the sum of the complex numbers is zero",
)
return s / abs(s)
end
function Statistics.mean(
Expand All @@ -367,8 +368,9 @@ function Statistics.mean(
kwargs...,
)
s = sum(w .* x)
abs(s) == 0 &&
error("The mean for $(x) on $(M) is not defined/unique, since the sum of the complex numbers is zero")
abs(s) == 0 && error(
"The mean for $(x) on $(M) is not defined/unique, since the sum of the complex numbers is zero",
)
return s /= abs(s)
end

Expand Down
52 changes: 20 additions & 32 deletions src/manifolds/Euclidean.jl
Original file line number Diff line number Diff line change
Expand Up @@ -149,10 +149,16 @@ function embed!(
ln = length(n)
m = size(q)
lm = length(m)
(length(n) > length(m)) &&
throw(DomainError("Invalid embedding, since Euclidean dimension ($(n)) is longer than embedding dimension $(m)."))
any(n .> m[1:ln]) &&
throw(DomainError("Invalid embedding, since Euclidean dimension ($(n)) has entry larger than embedding dimensions ($(m))."))
(length(n) > length(m)) && throw(
DomainError(
"Invalid embedding, since Euclidean dimension ($(n)) is longer than embedding dimension $(m).",
),
)
any(n .> m[1:ln]) && throw(
DomainError(
"Invalid embedding, since Euclidean dimension ($(n)) has entry larger than embedding dimensions ($(m)).",
),
)
# put p into q
fill!(q, 0)
# fill „top left edge“ of q with p.
Expand Down Expand Up @@ -384,17 +390,6 @@ in this case, just the (Frobenius) norm of `X`.
LinearAlgebra.norm(::Euclidean, p, X) = norm(X)
LinearAlgebra.norm(::MetricManifold{ℝ,<:Manifold,EuclideanMetric}, p, X) = norm(X)

"""
normal_tvector_distribution(M::Euclidean, p, σ)
Normal distribution in ambient space with standard deviation `σ`
projected to tangent space at `p`.
"""
function normal_tvector_distribution(M::Euclidean{Tuple{N}}, p, σ) where {N}
d = Distributions.MvNormal(zero(p), σ)
return ProjectedFVectorDistribution(TangentBundleFibers(M), p, d, project!, p)
end

function project!(
::EmbeddedManifold{𝔽,Euclidean{nL,𝔽},Euclidean{mL,𝔽2}},
q,
Expand All @@ -404,10 +399,16 @@ function project!(
ln = length(n)
m = size(q)
lm = length(m)
(length(n) < length(m)) &&
throw(DomainError("Invalid embedding, since Euclidean dimension ($(n)) is longer than embedding dimension $(m)."))
any(n .< m[1:ln]) &&
throw(DomainError("Invalid embedding, since Euclidean dimension ($(n)) has entry larger than embedding dimensions ($(m))."))
(length(n) < length(m)) && throw(
DomainError(
"Invalid embedding, since Euclidean dimension ($(n)) is longer than embedding dimension $(m).",
),
)
any(n .< m[1:ln]) && throw(
DomainError(
"Invalid embedding, since Euclidean dimension ($(n)) has entry larger than embedding dimensions ($(m)).",
),
)
# fill q with the „top left edge“ of p.
q .= p[map(i -> Base.OneTo(i), m)..., ntuple(_ -> 1, lm - ln)...]
return q
Expand All @@ -434,19 +435,6 @@ project(::Euclidean, ::Any, ::Any)

project!(::Euclidean, Y, p, X) = copyto!(Y, X)

"""
projected_distribution(M::Euclidean, d, [p])
Wrap the standard distribution `d` into a manifold-valued distribution. Generated
points will be of similar type to `p`. By default, the type is not changed.
"""
function projected_distribution(M::Euclidean, d, p)
return ProjectedPointDistribution(M, d, project!, p)
end
function projected_distribution(M::Euclidean, d)
return ProjectedPointDistribution(M, d, project!, rand(d))
end

"""
representation_size(M::Euclidean)
Expand Down
Loading

0 comments on commit 85e3940

Please sign in to comment.