Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unify use of an atol when comparing to zero #692

Merged
merged 29 commits into from
Dec 25, 2023
Merged
Show file tree
Hide file tree
Changes from 28 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
bbb6dd6
Unify atols in isapproxes.
kellertuer Dec 14, 2023
e60fb14
fix dispatch route for direction vector transport in Circle.
kellertuer Dec 14, 2023
477feae
Fix a space error.
kellertuer Dec 14, 2023
d9d83cf
Make the atol more generic/tolerant
kellertuer Dec 14, 2023
b397d2a
Fix a small typo in symplectic stiefel and update project.toml and news.
kellertuer Dec 14, 2023
8b57a5b
Fix essential manifold.
kellertuer Dec 14, 2023
c35aa83
Apply suggestions from code review
kellertuer Dec 15, 2023
1e9610b
Update our reference.
kellertuer Dec 15, 2023
5fa4eed
fix an edge case of exp! on Julia 1.6
mateuszbaran Dec 16, 2023
e7861d9
comment out a test for now
mateuszbaran Dec 16, 2023
b269660
some fixes for ManifoldsBase 0.15.6
mateuszbaran Dec 16, 2023
8aad681
Adapt imports, remove the ones that are no longer defined since Manif…
kellertuer Dec 18, 2023
a0aaa02
Adapt statistics to new scheme (part 1).
kellertuer Dec 18, 2023
0cfeca5
Improve deprecations.
kellertuer Dec 18, 2023
869f0a3
Improve formatting.
kellertuer Dec 18, 2023
9f6f544
Apply suggestions from code review
kellertuer Dec 18, 2023
b8de1d9
forgot one scaling in the mean.
kellertuer Dec 19, 2023
f3f1359
Merge branch 'kellertuer/fix-isapproxes' of github.com:JuliaManifolds…
kellertuer Dec 19, 2023
27b1bbb
Fix a typo and allow for dispatch on the method already for the alloc…
kellertuer Dec 19, 2023
c7fc451
Fix statistics tests.
kellertuer Dec 23, 2023
f181e72
reintroduce the correct default.
kellertuer Dec 23, 2023
032a5d4
remove unneccessary code cases, increase code coverage also for depre…
kellertuer Dec 24, 2023
70d5df8
re-enable BVP test, but raise its tolerance slightly.
kellertuer Dec 24, 2023
6244a6a
Remove two unnecessary fallbacks.
kellertuer Dec 24, 2023
f8080e2
Remove allocating code that is no longer necessary on rotations.
kellertuer Dec 24, 2023
4c05d8e
Apply suggestions from code review
kellertuer Dec 24, 2023
3d9c861
Apply suggestions from code review
kellertuer Dec 24, 2023
f95584f
Revert deletion of two methods of `parallel_transport_direction` and …
mateuszbaran Dec 24, 2023
a63e8ae
fix changelog.
kellertuer Dec 25, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,15 @@ 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.9] – 2023-12-14

### Fixed

* introduced a nonzero `atol` for all point and vector checks that compre to zero.
This makes those checks a bit more relaxed by default and resolves [#630](https://github.com/JuliaManifolds/Manifolds.jl/issues/630).
* `default_estimation_method(M, f)` is deprecated, use `default_approximation_method(M, f)` for your specific method `f` on the manifold `M`.
* `AbstractEstimationMethod` is deprecated, use `AbstractApproximationMethod` instead.

## [0.9.8] - 2023-11-17

### Fixed
Expand Down
6 changes: 3 additions & 3 deletions Project.toml
Original file line number Diff line number Diff line change
@@ -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.9.8"
version = "0.9.9"

[deps]
Distributions = "31c24e10-a181-5473-b8eb-7969acd0382f"
Expand Down Expand Up @@ -42,7 +42,7 @@ ManifoldsRecipesBaseExt = ["Colors", "RecipesBase"]
ManifoldsTestExt = "Test"

[compat]
BoundaryValueDiffEq = "4, 5"
BoundaryValueDiffEq = "4, 5.6.1"
Colors = "0.12"
Distributions = "0.22.6, 0.23, 0.24, 0.25"
Einsum = "0.4"
Expand All @@ -51,7 +51,7 @@ HybridArrays = "0.4"
Kronecker = "0.4, 0.5"
LinearAlgebra = "1.6"
ManifoldDiff = "0.3.7"
ManifoldsBase = "0.15.0"
ManifoldsBase = "0.15.6"
Markdown = "1.6"
MatrixEquations = "2.2"
OrdinaryDiffEq = "6.31"
Expand Down
18 changes: 10 additions & 8 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -53,17 +53,19 @@ If you have any questions regarding the Manifolds.jl ecosystem feel free to reac

## Citation

If you use `Manifolds.jl` in your work, please cite the following
If you use `Manifolds.jl` in your work, please cite the following open access article

```biblatex
@article{AxenBaranBergmannRzecki:2023,
AUTHOR = {Seth D. Axen and Mateusz Baran and Ronny Bergmann and Krzysztof Rzecki},
EPRINT = {2021.08777},
EPRINTTYPE = {arXiv},
JOURNAL = {AMS Transactions on Mathematical Software},
NOTE = {accepted for publication},
TITLE = {Manifolds.jl: An Extensible {J}ulia Framework for Data Analysis on Manifolds},
YEAR = {2023}
AUTHOR = {Axen, Seth D. and Baran, Mateusz and Bergmann, Ronny and Rzecki, Krzysztof},
ARTICLENO = {33},
DOI = {10.1145/3618296},
JOURNAL = {ACM Transactions on Mathematical Software},
MONTH = {dec},
NUMBER = {4},
TITLE = {Manifolds.Jl: An Extensible Julia Framework for Data Analysis on Manifolds},
VOLUME = {49},
YEAR = {2023}
}
```

Expand Down
16 changes: 9 additions & 7 deletions docs/src/references.bib
Original file line number Diff line number Diff line change
Expand Up @@ -89,13 +89,15 @@ @book{AyJostLeSchwachhoefer:2017
PUBLISHER = {Springer Cham}
}
@article{AxenBaranBergmannRzecki:2023,
AUTHOR = {Seth D. Axen and Mateusz Baran and Ronny Bergmann and Krzysztof Rzecki},
EPRINT = {2021.08777},
EPRINTTYPE = {arXiv},
JOURNAL = {AMS Transactions on Mathematical Software},
NOTE = {accepted for publication},
TITLE = {Manifolds.jl: An Extensible {J}ulia Framework for Data Analysis on Manifolds},
YEAR = {2023}
AUTHOR = {Axen, Seth D. and Baran, Mateusz and Bergmann, Ronny and Rzecki, Krzysztof},
ARTICLENO = {33},
DOI = {10.1145/3618296},
JOURNAL = {ACM Transactions on Mathematical Software},
MONTH = {dec},
NUMBER = {4},
TITLE = {Manifolds.Jl: An Extensible Julia Framework for Data Analysis on Manifolds},
VOLUME = {49},
YEAR = {2023}
}
#
# B
Expand Down
32 changes: 21 additions & 11 deletions src/Manifolds.jl
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ import ManifoldsBase:
check_vector,
copy,
copyto!,
default_approximation_method,
default_inverse_retraction_method,
default_retraction_method,
default_vector_transport_method,
Expand Down Expand Up @@ -149,19 +150,14 @@ import ManifoldsBase:
submanifold_component,
submanifold_components,
vector_space_dimension,
vector_transport_along, # just specified in Euclidean - the next 5 as well
vector_transport_along_diff,
vector_transport_along_project,
vector_transport_along, # just specified in Euclidean - the next 5 as well
vector_transport_along!,
vector_transport_along_diff!,
vector_transport_along_project!,
vector_transport_along_diff!, # For consistency these are imported, but for now not
vector_transport_along_project!, # overwritten with new definitons.
vector_transport_direction,
vector_transport_direction_diff,
vector_transport_direction!,
vector_transport_direction_diff!,
vector_transport_to,
vector_transport_to_diff,
vector_transport_to_project,
vector_transport_to!,
vector_transport_to_diff!,
vector_transport_to_project!, # some overwrite layer 2
Expand All @@ -186,6 +182,9 @@ import ManifoldDiff:
riemannian_Hessian,
riemannian_Hessian!

import Statistics: mean, mean!, median, median!, cov, var
import StatsBase: mean_and_var

using Base.Iterators: repeated
using Distributions
using Einsum: @einsum
Expand All @@ -198,6 +197,7 @@ using ManifoldsBase:
ℝ,
ℂ,
ℍ,
AbstractApproximationMethod,
AbstractBasis,
AbstractDecoratorManifold,
AbstractInverseRetractionMethod,
Expand Down Expand Up @@ -225,20 +225,26 @@ using ManifoldsBase:
CotangentSpaceType,
CoTFVector,
CoTVector,
CyclicProximalPointEstimation,
DefaultBasis,
DefaultOrthogonalBasis,
DefaultOrthonormalBasis,
DefaultOrDiagonalizingBasis,
DiagonalizingBasisData,
DiagonalizingOrthonormalBasis,
DifferentiatedRetractionVectorTransport,
EfficientEstimator,
EmbeddedManifold,
EmptyTrait,
EuclideanMetric,
ExponentialRetraction,
ExtrinsicEstimation,
Fiber,
FiberType,
FVector,
GeodesicInterpolation,
GeodesicInterpolationWithinRadius,
GradientDescentEstimation,
InverseProductRetraction,
IsIsometricEmbeddedManifold,
IsEmbeddedManifold,
Expand Down Expand Up @@ -295,6 +301,7 @@ using ManifoldsBase:
VectorSpaceFiber,
VectorSpaceType,
VeeOrthogonalBasis,
WeiszfeldEstimation,
@invoke_maker,
_euclidean_basis_vector,
combine_allocation_promotion_functions,
Expand Down Expand Up @@ -607,8 +614,9 @@ include("deprecated.jl")
export test_manifold
export test_group, test_action

#
# Abstract main types
export CoTVector, AbstractManifold, AbstractManifoldPoint, TVector
# Manifolds
export AbstractSphere, AbstractProjectiveSpace
export Euclidean,
ArrayProjectiveSpace,
Expand Down Expand Up @@ -748,9 +756,10 @@ export AbstractInverseRetractionMethod,
ShootingInverseRetraction,
SoftmaxInverseRetraction
# Estimation methods for median and mean
export AbstractEstimationMethod,
export AbstractApproximationMethod,
GradientDescentEstimation,
CyclicProximalPointEstimation,
EfficientEstimator,
GeodesicInterpolation,
GeodesicInterpolationWithinRadius,
ExtrinsicEstimation
Expand Down Expand Up @@ -788,9 +797,10 @@ export ×,
convert,
complex_dot,
decorated_manifold,
default_vector_transport_method,
default_approximation_method,
default_inverse_retraction_method,
default_retraction_method,
default_vector_transport_method,
det_local_metric,
differential_canonical_project,
differential_canonical_project!,
Expand Down
5 changes: 5 additions & 0 deletions src/deprecated.jl
Original file line number Diff line number Diff line change
@@ -1 +1,6 @@

@deprecate default_estimation_method(M::AbstractManifold, f) default_approximation_method(
M,
f,
)
@deprecate ExtrinsicEstimation() ExtrinsicEstimation(EfficientEstimator())
10 changes: 8 additions & 2 deletions src/groups/addition_operation.jl
Original file line number Diff line number Diff line change
Expand Up @@ -83,8 +83,14 @@ function inv_diff!(::AdditionGroupTrait, G::AbstractDecoratorManifold, Y, p, X)
return Y
end

function is_identity(::AdditionGroupTrait, G::AbstractDecoratorManifold, q; kwargs...)
return isapprox(G, q, zero(q); kwargs...)
function is_identity(
::AdditionGroupTrait,
G::AbstractDecoratorManifold,
q::T;
atol::Real=sqrt(prod(representation_size(G))) * eps(real(float(number_eltype(T)))),
kwargs...,
) where {T}
return isapprox(G, q, zero(q); atol=atol, kwargs...)
end
# resolve ambiguities
function is_identity(
Expand Down
6 changes: 3 additions & 3 deletions src/groups/group_action.jl
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ where element `a` is acting on `p`, with respect to the group element.

Let ``\mathcal G`` be the group acting on manifold ``\mathcal M`` by the action `A`.
The action is of element ``g ∈ \mathcal G`` on a point ``p ∈ \mathcal M``.
The differential transforms vector `X` from the tangent space at `a ∈ \mathcal G`,
The differential transforms vector `X` from the tangent space at `a ∈ \mathcal G`,
``X ∈ T_a \mathcal G`` into a tangent space of the manifold ``\mathcal M``.
When action on element `p` is written as ``\mathrm{d}τ^p``, with the specified left or right
convention, the differential transforms vectors
Expand Down Expand Up @@ -193,7 +193,7 @@ end
A::AbstractGroupAction,
pts,
p,
mean_method::AbstractEstimationMethod = GradientDescentEstimation(),
mean_method::AbstractApproximationMethod = GradientDescentEstimation(),
)

Calculate an action element ``a`` of action `A` that is the mean element of the orbit of `p`
Expand All @@ -210,7 +210,7 @@ function center_of_orbit(
A::AbstractGroupAction,
pts::AbstractVector,
q,
mean_method::AbstractEstimationMethod=GradientDescentEstimation(),
mean_method::AbstractApproximationMethod=GradientDescentEstimation(),
)
alignments = map(p -> optimal_alignment(A, q, p), pts)
return mean(base_group(A), alignments, mean_method)
Expand Down
2 changes: 1 addition & 1 deletion src/groups/group_operation_action.jl
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ function center_of_orbit(
A::GroupOperationAction,
pts::AbstractVector,
p,
mean_method::AbstractEstimationMethod,
mean_method::AbstractApproximationMethod,
)
μ = mean(A.group, pts, mean_method)
return inverse_apply(switch_direction_and_side(A), p, μ)
Expand Down
10 changes: 8 additions & 2 deletions src/groups/special_linear.jl
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,15 @@ function check_point(G::SpecialLinear, p; kwargs...)
return nothing
end

function check_vector(G::SpecialLinear, p, X; kwargs...)
function check_vector(
G::SpecialLinear,
p,
X::T;
atol::Real=sqrt(prod(representation_size(G))) * eps(real(float(number_eltype(T)))),
kwargs...,
) where {T}
trX = tr(inverse_translate_diff(G, p, p, X, LeftForwardAction()))
if !isapprox(trX, 0; kwargs...)
if !isapprox(trX, 0; atol=atol, kwargs...)
return DomainError(
trX,
"The matrix $(X) does not lie in the tangent space of $(G) at $(p), since " *
Expand Down
19 changes: 15 additions & 4 deletions src/manifolds/CenteredMatrices.jl
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,14 @@

The tolerance for the column sums of `p` can be set using `kwargs...`.
"""
function check_point(M::CenteredMatrices, p; kwargs...)
function check_point(

Check warning on line 38 in src/manifolds/CenteredMatrices.jl

View check run for this annotation

Codecov / codecov/patch

src/manifolds/CenteredMatrices.jl#L38

Added line #L38 was not covered by tests
M::CenteredMatrices,
p::T;
atol::Real=sqrt(prod(representation_size(M))) * eps(real(float(number_eltype(T)))),
kwargs...,
) where {T}
m, n = get_parameter(M.size)
if !isapprox(sum(p, dims=1), zeros(1, n); kwargs...)
if !isapprox(sum(p, dims=1), zeros(1, n); atol=atol, kwargs...)

Check warning on line 45 in src/manifolds/CenteredMatrices.jl

View check run for this annotation

Codecov / codecov/patch

src/manifolds/CenteredMatrices.jl#L45

Added line #L45 was not covered by tests
return DomainError(
p,
string(
Expand All @@ -56,9 +61,15 @@
sum to zero and its values are from the correct [`AbstractNumbers`](https://juliamanifolds.github.io/ManifoldsBase.jl/stable/types.html#number-system).
The tolerance for the column sums of `p` and `X` can be set using `kwargs...`.
"""
function check_vector(M::CenteredMatrices, p, X; kwargs...)
function check_vector(

Check warning on line 64 in src/manifolds/CenteredMatrices.jl

View check run for this annotation

Codecov / codecov/patch

src/manifolds/CenteredMatrices.jl#L64

Added line #L64 was not covered by tests
M::CenteredMatrices,
p,
X::T;
atol::Real=sqrt(prod(representation_size(M))) * eps(real(float(number_eltype(T)))),
kwargs...,
) where {T}
m, n = get_parameter(M.size)
if !isapprox(sum(X, dims=1), zeros(1, n); kwargs...)
if !isapprox(sum(X, dims=1), zeros(1, n); atol=atol, kwargs...)

Check warning on line 72 in src/manifolds/CenteredMatrices.jl

View check run for this annotation

Codecov / codecov/patch

src/manifolds/CenteredMatrices.jl#L72

Added line #L72 was not covered by tests
return DomainError(
X,
"The vector $(X) is not a tangent vector to $(p) on $(M), since its columns do not sum to zero.",
Expand Down
19 changes: 15 additions & 4 deletions src/manifolds/CholeskySpace.jl
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,15 @@
entries on the diagonal.
The tolerance for the tests can be set using the `kwargs...`.
"""
function check_point(M::CholeskySpace, p; kwargs...)
function check_point(

Check warning on line 31 in src/manifolds/CholeskySpace.jl

View check run for this annotation

Codecov / codecov/patch

src/manifolds/CholeskySpace.jl#L31

Added line #L31 was not covered by tests
M::CholeskySpace,
p::T;
atol::Real=sqrt(prod(representation_size(M))) * eps(real(float(number_eltype(T)))),
kwargs...,
) where {T}
cks = check_size(M, p)
cks === nothing || return cks
if !isapprox(norm(strictlyUpperTriangular(p)), 0.0; kwargs...)
if !isapprox(norm(strictlyUpperTriangular(p)), 0.0; atol=atol, kwargs...)

Check warning on line 39 in src/manifolds/CholeskySpace.jl

View check run for this annotation

Codecov / codecov/patch

src/manifolds/CholeskySpace.jl#L39

Added line #L39 was not covered by tests
return DomainError(
norm(UpperTriangular(p) - Diagonal(p)),
"The point $(p) does not lie on $(M), since it strictly upper triangular nonzero entries",
Expand All @@ -54,8 +59,14 @@
and a symmetric matrix.
The tolerance for the tests can be set using the `kwargs...`.
"""
function check_vector(M::CholeskySpace, p, X; kwargs...)
if !isapprox(norm(strictlyUpperTriangular(X)), 0.0; kwargs...)
function check_vector(

Check warning on line 62 in src/manifolds/CholeskySpace.jl

View check run for this annotation

Codecov / codecov/patch

src/manifolds/CholeskySpace.jl#L62

Added line #L62 was not covered by tests
M::CholeskySpace,
p,
X;
atol::Real=sqrt(prod(representation_size(M)) * eps(float(eltype(p)))),
kwargs...,
)
if !isapprox(norm(strictlyUpperTriangular(X)), 0.0; atol=atol, kwargs...)

Check warning on line 69 in src/manifolds/CholeskySpace.jl

View check run for this annotation

Codecov / codecov/patch

src/manifolds/CholeskySpace.jl#L69

Added line #L69 was not covered by tests
return DomainError(
norm(UpperTriangular(X) - Diagonal(X)),
"The matrix $(X) is not a tangent vector at $(p) (represented as an element of the Lie algebra) since it is not lower triangular.",
Expand Down
17 changes: 14 additions & 3 deletions src/manifolds/Circle.jl
Original file line number Diff line number Diff line change
Expand Up @@ -78,8 +78,14 @@
function check_vector(M::Circle{ℝ}, p, X; kwargs...)
return nothing
end
function check_vector(M::Circle{ℂ}, p, X; kwargs...)
if !isapprox(abs(complex_dot(p, X)), 0.0; kwargs...)
function check_vector(
M::Circle{ℂ},
p,
X::T;
atol::Real=sqrt(eps(real(float(number_eltype(T))))),
kwargs...,
) where {T}
if !isapprox(abs(complex_dot(p, X)), 0; atol=atol, kwargs...)
return DomainError(
abs(complex_dot(p, X)),
"The value $(X) is not a tangent vector to $(p) on $(M), since it is not orthogonal in the embedding.",
Expand Down Expand Up @@ -160,7 +166,7 @@
end

exp!(::Circle{ℝ}, q, p, X) = (q .= sym_rem(p + X))
exp!(::Circle{ℝ}, q, p, X, t::Number) = (q .= sym_rem(p + t * X))
exp!(::Circle{ℝ}, q, p, X, t::Number) = (q .= sym_rem(p[] + t * X[]))

Check warning on line 169 in src/manifolds/Circle.jl

View check run for this annotation

Codecov / codecov/patch

src/manifolds/Circle.jl#L169

Added line #L169 was not covered by tests
function exp!(M::Circle{ℂ}, q, p, X)
θ = norm(M, p, X)
q .= cos(θ) * p + usinc(θ) * X
Expand Down Expand Up @@ -538,6 +544,11 @@
return Y
end

# dispatch before allocation
function _vector_transport_direction(M::Circle, p, X, d, ::ParallelTransport)
return parallel_transport_to(M, p, X, exp(M, p, d))
end

"""
volume_density(::Circle, p, X)

Expand Down
Loading
Loading