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

Adapt to ManifoldsBase 1.0 #781

Open
wants to merge 37 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 25 commits
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
ef4c4f0
Adapt new function names and naming scheme.
kellertuer Jan 24, 2025
8411b6f
Already remove two fallbacks on hat, that cause ambiguities on Groups.
kellertuer Jan 24, 2025
6d8555d
runs formatter.
kellertuer Jan 25, 2025
255623f
mostly convert to retraction_t
mateuszbaran Jan 25, 2025
f3142cf
a few more fixes
mateuszbaran Jan 25, 2025
24acd74
Adapt to removal of the _along methods.
kellertuer Jan 26, 2025
722c6fa
Merge branch 'kellertuer/adapt-to-ManifoldsBase-16' of github.com:Jul…
kellertuer Jan 26, 2025
6a67d99
Merge branch 'kellertuer/adapt-to-ManifoldsBase-16' of https://github…
mateuszbaran Jan 26, 2025
268c1dd
more retraction fixes
mateuszbaran Jan 26, 2025
372651e
one more fix
mateuszbaran Jan 26, 2025
c589e84
fix ODE retraction
mateuszbaran Jan 26, 2025
1252478
nicer compat.
kellertuer Jan 27, 2025
0aac6d9
rename exp_t -> exp_fused
mateuszbaran Jan 31, 2025
5015202
Fix elliptope
kellertuer Feb 1, 2025
a539ca1
runs formatter.
kellertuer Feb 1, 2025
81b5940
fixing stuff
mateuszbaran Feb 1, 2025
c14cbd2
Merge branch 'kellertuer/adapt-to-ManifoldsBase-16' of https://github…
mateuszbaran Feb 1, 2025
cc24fa3
Fix most fused cases.
kellertuer Feb 1, 2025
75515a2
fix a test.
kellertuer Feb 1, 2025
40e5fa9
Fix two tests in SE(n) for hat/hat! where we checked they would not w…
kellertuer Feb 1, 2025
5c75d8d
Fix sasaki.
kellertuer Feb 1, 2025
f1e121c
Fix connection.
kellertuer Feb 1, 2025
acda8b7
Don't export the fused variants.
kellertuer Feb 1, 2025
ff1b56f
Fix two further tests.
kellertuer Feb 1, 2025
c289c50
A few final touches.
kellertuer Feb 1, 2025
51363d3
Undo an accidental renaming.
kellertuer Feb 5, 2025
92baf8a
Add deprecation bindings, test and document them.
kellertuer Feb 5, 2025
25e1b76
Fix a typo.
kellertuer Feb 5, 2025
4525786
Fix compat entries in docs and tutorials.
kellertuer Feb 5, 2025
2d2a9b0
bump ambiguities.
kellertuer Feb 5, 2025
243ce62
this method is not needed
mateuszbaran Feb 5, 2025
837047a
Towards code cov.
kellertuer Feb 7, 2025
81c8c82
add a few tests towards code coverage.
kellertuer Feb 7, 2025
90c28f0
Add references.
kellertuer Feb 7, 2025
ee8530b
Remove old code, that is no longer necessary, since we are on 1.10 at…
kellertuer Feb 7, 2025
c548a64
tweak docs.
kellertuer Feb 7, 2025
bfd984f
Ups. Forgot a marker.
kellertuer Feb 7, 2025
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
6 changes: 3 additions & 3 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,8 @@ The allocating variant only needs to defined if a more efficient version
than the default is available.

Note that since the first argument is _always_ the [`AbstractManifold`](https://juliamanifolds.github.io/ManifoldsBase.jl/stable/types.html#ManifoldsBase.AbstractManifold), the mutated argument is always the second one in the signature.
In the example there are `exp(M, p, X, t)` for the exponential map that allocates
its result `q`, and `exp!(M, q, p, X, t)` for the in-place one, which computes and returns the `q`.
In the example there are `exp(M, p, X)` for the exponential map that allocates
its result `q`, and `exp!(M, q, p, X)` for the in-place one, which computes and returns the `q`.

Since a user probably looks for the documentation on the allocating variant,
we recommend to attach the documentation string to this variant, mentioning all
Expand All @@ -78,7 +78,7 @@ struct MyManifold <: AbstractManifold end

Describe the function, its input and output as well as a mathematical formula.
"""
exp(::MyManifold, ::Any...)
exp(::MyManifold, ::Any, ::Any)
```

You can also save the string to a variable, for example `_doc_myM_exp` and attach it to both functions
Expand Down
6 changes: 6 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,12 @@ 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.10.13] - unreleased

## Changed
kellertuer marked this conversation as resolved.
Show resolved Hide resolved

* bumped dependency of ManifoldsBase.jl to 0.16, split `exp` into `exp` and `exp_fused` accordingly

## [0.10.12] - 2025-01-10

### Added
Expand Down
4 changes: 2 additions & 2 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 <[email protected]>", "Mateusz Baran <[email protected]>", "Ronny Bergmann <[email protected]>", "Antoine Levitt <[email protected]>"]
version = "0.10.12"
version = "0.10.13"

[deps]
Einsum = "b7d42ee7-0b51-5a75-98ca-779d3107e4c0"
Expand Down Expand Up @@ -54,7 +54,7 @@ HybridArrays = "0.4"
Kronecker = "0.4, 0.5"
LinearAlgebra = "1.6"
ManifoldDiff = "0.4.0"
ManifoldsBase = "0.15.18"
ManifoldsBase = "1"
Markdown = "1.6"
MatrixEquations = "2.2"
NLsolve = "4"
Expand Down
2 changes: 1 addition & 1 deletion docs/src/manifolds/rotations.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ Tangent spaces at different points are different vector spaces.
Let ``L_R: \mathrm{SO}(n) → \mathrm{SO}(n)`` where ``R ∈ \mathrm{SO}(n)`` be the left-multiplication by ``R``, that is ``L_R(S) = RS``.
The tangent space at rotation ``R``, ``T_R \mathrm{SO}(n)``, is related to the tangent space at the identity rotation ``I_n`` by the differential of ``L_R`` at identity, ``(\mathrm{d}L_R)_{I_n} : T_{I_n} \mathrm{SO}(n) → T_R \mathrm{SO}(n)``.
To convert the tangent vector representation at the identity rotation ``X ∈ T_{I_n} \mathrm{SO}(n)`` (i.e., the default) to the matrix representation of the corresponding tangent vector ``Y`` at a rotation ``R`` use the [`embed`](@ref embed(::Manifolds.Rotations, :Any...)) which implements the following multiplication: ``Y = RX ∈ T_R \mathrm{SO}(n)``.
You can compare the functions [`log`](@ref log(::Manifolds.Rotations, :Any...)) and [`exp`](@ref exp(::Manifolds.Rotations, ::Any...)) to see how it works in practice.
You can compare the functions [`log`](@ref log(::Manifolds.Rotations, :Any...)) and [`exp`](@ref exp(::Manifolds.Rotations, ::Any, ::Any)) to see how it works in practice.

Several common functions are also implemented together with [orthogonal and unitary matrices](@ref generalunitarymatrices).

Expand Down
17 changes: 14 additions & 3 deletions ext/ManifoldsOrdinaryDiffEqExt.jl
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ using ManifoldsBase
using ManifoldsBase: TraitList

using Manifolds
import Manifolds: exp!, solve_exp_ode
import Manifolds: exp!, exp_fused!, solve_exp_ode
using Manifolds: @einsum

using ManifoldDiff: default_differential_backend
Expand Down Expand Up @@ -49,7 +49,7 @@ function solve_exp_ode(
return q
end
# also define exp! for metric manifold anew in this case
function exp!(
function exp_fused!(
::TraitList{IsMetricManifold},
M::AbstractDecoratorManifold,
q,
Expand All @@ -61,5 +61,16 @@ function exp!(
copyto!(M, q, solve_exp_ode(M, p, X, t; kwargs...))
return q
end

# also define exp! for metric manifold anew in this case
function exp!(
::TraitList{IsMetricManifold},
M::AbstractDecoratorManifold,
q,
p,
X;
kwargs...,
)
copyto!(M, q, solve_exp_ode(M, p, X, 1.0; kwargs...))
return q
end
end
4 changes: 2 additions & 2 deletions ext/ManifoldsRecipesBaseExt.jl
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ SURFACE_RESOLUTION_DEFAULT = 32
circle_points=CIRCLE_DEFAULT_PLOT_POINTS,
geodesic_interpolation=-1,
hyperbolic_border_color=RGBA(0.0, 0.0, 0.0, 1.0),
) where {P<:PoincareBallPoint,T<:PoincareBallTVector}
) where {P<:PoincareBallPoint,T<:PoincareBallTangentVector}
@series begin
φr = range(0, stop=2 * π, length=circle_points)
x = [cos(φ) for φ in φr]
Expand Down Expand Up @@ -83,7 +83,7 @@ end
pts::AbstractVector{P},
vecs::Union{AbstractVector{T},Nothing}=nothing;
geodesic_interpolation=-1,
) where {P<:PoincareHalfSpacePoint,T<:PoincareHalfSpaceTVector}
) where {P<:PoincareHalfSpacePoint,T<:PoincareHalfSpaceTangentVector}
aspect_ratio --> :equal
framestyle --> :origin
x = []
Expand Down
43 changes: 27 additions & 16 deletions ext/ManifoldsTestExt/tests_general.jl
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,6 @@ function test_manifold(
test_mutating_rand=false,
parallel_transport=false,
parallel_transport_to=parallel_transport,
parallel_transport_along=parallel_transport,
parallel_transport_direction=parallel_transport,
test_inner=true,
test_norm=true,
Expand Down Expand Up @@ -207,13 +206,25 @@ function test_manifold(
X1 = log(M, pts[1], pts[2])
X2 = log(M, pts[2], pts[1])
Test.@test isapprox(M, pts[2], exp(M, pts[1], X1); atol=atolp1p2, rtol=rtolp1p2)
Test.@test isapprox(M, pts[1], exp(M, pts[1], X1, 0); atol=atolp1p2, rtol=rtolp1p2)
Test.@test isapprox(M, pts[2], exp(M, pts[1], X1, 1); atol=atolp1p2, rtol=rtolp1p2)
Test.@test isapprox(
M,
pts[1],
Manifolds.exp_fused(M, pts[1], X1, 0);
atol=atolp1p2,
rtol=rtolp1p2,
)
Test.@test isapprox(
M,
pts[2],
Manifolds.exp_fused(M, pts[1], X1, 1);
atol=atolp1p2,
rtol=rtolp1p2,
)
if is_mutating
q2 = allocate(pts[1])
exp!(M, q2, pts[1], X1)
Test.@test isapprox(M, pts[2], q2; atol=atolp1p2, rtol=rtolp1p2)
exp!(M, q2, pts[1], X1, 0)
Manifolds.exp_fused!(M, q2, pts[1], X1, 0)
Test.@test isapprox(M, pts[1], q2; atol=atolp1p2, rtol=rtolp1p2)
end
if VERSION >= v"1.5" && isa(M, Union{Grassmann,GeneralizedStiefel})
Expand All @@ -230,7 +241,13 @@ function test_manifold(
Test.@test isapprox(M, pts[1], exp(M, pts[2], X2); atol=atolp1p2, rtol=rtolp1p2)
end
Test.@test is_point(M, exp(M, pts[1], X1); atol=atolp1p2, rtol=rtolp1p2)
Test.@test isapprox(M, pts[1], exp(M, pts[1], X1, 0); atol=atolp1p2, rtol=rtolp1p2)
Test.@test isapprox(
M,
pts[1],
Manifolds.exp_fused(M, pts[1], X1, 0);
atol=atolp1p2,
rtol=rtolp1p2,
)
for p in pts
epsx = find_eps(p)
Test.@test isapprox(
Expand Down Expand Up @@ -265,8 +282,8 @@ function test_manifold(
X1 = log(M, pts[1], pts[2])
end

Test.@test isapprox(M, exp(M, pts[1], X1, 1), pts[2]; atol=atolp1)
Test.@test isapprox(M, exp(M, pts[1], X1, 0), pts[1]; atol=atolp1)
Test.@test isapprox(M, Manifolds.exp_fused(M, pts[1], X1, 1), pts[2]; atol=atolp1)
Test.@test isapprox(M, Manifolds.exp_fused(M, pts[1], X1, 0), pts[1]; atol=atolp1)

if test_norm
Test.@test distance(M, pts[1], pts[2]) ≈ norm(M, pts[1], X1)
Expand Down Expand Up @@ -297,7 +314,6 @@ function test_manifold(
parallel_transport && test_parallel_transport(
M,
pts;
along=parallel_transport_along,
to=parallel_transport_to,
direction=parallel_transport_direction,
mutating=is_mutating,
Expand All @@ -312,7 +328,7 @@ function test_manifold(
Test.@test isapprox(
M,
p,
retract(M, p, X, 0, retr_method);
Manifolds.retract_fused(M, p, X, 0, retr_method);
atol=epsx * retraction_atol_multiplier,
rtol=retraction_atol_multiplier == 0 ?
sqrt(epsx) * retraction_rtol_multiplier : 0,
Expand Down Expand Up @@ -842,15 +858,12 @@ function test_manifold(
end

"""
test_parallel_transport(M,P; along=false, to=true, direction=true)
test_parallel_transport(M,P; to=true, direction=true)

Generic tests for parallel transport on `M`given at least two pointsin `P`.

The single functions to transport `along` (a curve), `to` (a point) or (towards a) `direction`
The single functions to transport `to` (a point) or (in a) `direction`
are sub-tests that can be activated by the keywords arguments

!!! Note
Since the interface to specify curves is not yet provided, the along keyword does not have an effect yet
"""
function test_parallel_transport(
M::AbstractManifold,
Expand All @@ -861,15 +874,13 @@ function test_parallel_transport(
P[2:end],
Ref(default_inverse_retraction_method(M)),
);
along=false,
to=true,
direction=true,
mutating=true,
)
length(P) < 2 &&
error("The Parallel Transport test set requires at least 2 points in P")
Test.@testset "Test Parallel Transport" begin # COV_EXCL_LINE
along && @warn "parallel transport along test not yet implemented"
Test.@testset "To (a point)" begin # COV_EXCL_LINE
# even with to =false this displays no tests
if to
Expand Down
58 changes: 26 additions & 32 deletions src/Manifolds.jl
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ import ManifoldsBase:
allocate_result,
allocate_result_type,
allocation_promotion_function,
array_value,
base_manifold,
change_basis,
change_basis!,
Expand All @@ -60,6 +59,8 @@ import ManifoldsBase:
embed!,
exp,
exp!,
exp_fused,
exp_fused!,
fiber_dimension,
get_basis,
get_basis_default,
Expand Down Expand Up @@ -96,11 +97,7 @@ import ManifoldsBase:
_injectivity_radius,
injectivity_radius_exp,
inner,
isapprox,
_isapprox,
is_flat,
is_point,
is_vector,
internal_value,
inverse_retract,
inverse_retract!,
_inverse_retract!,
Expand All @@ -112,6 +109,11 @@ import ManifoldsBase:
inverse_retract_qr!,
inverse_retract_shooting!,
inverse_retract_softmax!,
isapprox,
_isapprox,
is_flat,
is_point,
is_vector,
log,
log!,
manifold_dimension,
Expand All @@ -120,8 +122,6 @@ import ManifoldsBase:
norm,
number_eltype,
number_of_coordinates,
parallel_transport_along,
parallel_transport_along!,
parallel_transport_direction,
parallel_transport_direction!,
parallel_transport_to,
Expand All @@ -135,8 +135,6 @@ import ManifoldsBase:
representation_size,
retract,
retract!,
_retract,
retract!,
retract_cayley!,
retract_exp_ode!,
retract_pade!,
Expand All @@ -145,6 +143,8 @@ import ManifoldsBase:
retract_qr!,
retract_sasaki!,
retract_softmax!,
retract_fused,
retract_fused!,
riemann_tensor,
riemann_tensor!,
sectional_curvature,
Expand All @@ -155,10 +155,6 @@ import ManifoldsBase:
submanifold_component,
submanifold_components,
vector_space_dimension,
vector_transport_along, # just specified in Euclidean - the next 5 as well
vector_transport_along!,
vector_transport_along_diff!, # For consistency these are imported, but for now not
vector_transport_along_project!, # overwritten with new definitions.
vector_transport_direction,
vector_transport_direction!,
vector_transport_direction_diff!,
Expand Down Expand Up @@ -229,7 +225,7 @@ using ManifoldsBase:
CotangentSpace,
CotangentSpaceType,
CoTFVector,
CoTVector,
AbstractCotangentVector,
CyclicProximalPointEstimation,
DefaultBasis,
DefaultOrthogonalBasis,
Expand Down Expand Up @@ -298,12 +294,12 @@ using ManifoldsBase:
TangentSpaceType,
TCoTSpaceType,
TFVector,
TVector,
AbstractTangentVector,
TypeParameter,
ValidationCoTVector,
ValidationCotangentVector,
ValidationManifold,
ValidationMPoint,
ValidationTVector,
ValidationTangentVector,
VectorSpaceFiber,
VectorSpaceType,
VeeOrthogonalBasis,
Expand Down Expand Up @@ -501,7 +497,7 @@ include("manifolds/KendallsShapeSpace.jl")
include("manifolds/Grassmann.jl")

# Introduce Symplectic and so on manifolds only after Grassmann
# Since that defines the StiefelPoint, StiefelTVector
# Since that defines the StiefelPoint, StiefelTangentVector
include("manifolds/Symplectic.jl")
include("manifolds/Hamiltonian.jl") # Hamiltonian requires symplectic
include("manifolds/SymplecticStiefel.jl")
Expand Down Expand Up @@ -636,7 +632,8 @@ export test_manifold
export test_group, test_action

# Abstract main types
export CoTVector, AbstractManifold, AbstractManifoldPoint, TVector
export AbstractCotangentVector,
AbstractManifold, AbstractManifoldPoint, AbstractTangentVector
# Manifolds
export AbstractSphere, AbstractProjectiveSpace
export Euclidean,
Expand Down Expand Up @@ -702,20 +699,21 @@ export HyperboloidPoint,
ProjectorPoint,
SPDPoint
# Tangent vector representation types
export HyperboloidTVector,
PoincareBallTVector,
PoincareHalfSpaceTVector,
TuckerTVector,
UMVTVector,
ProjectorTVector,
StiefelTVector
export HyperboloidTangentVector,
PoincareBallTangentVector,
PoincareHalfSpaceTangentVector,
TuckerTangentVector,
UMVTangentVector,
ProjectorTangentVector,
StiefelTangentVector
export AbstractNumbers, ℝ, ℂ, ℍ
export Hamiltonian
# decorator manifolds
export AbstractDecoratorManifold
export IsIsometricEmbeddedManifold, IsEmbeddedManifold, IsEmbeddedSubmanifold
export IsDefaultMetric, IsDefaultConnection, IsMetricManifold, IsConnectionManifold
export ValidationManifold, ValidationMPoint, ValidationTVector, ValidationCoTVector
export ValidationManifold,
ValidationMPoint, ValidationTangentVector, ValidationCotangentVector
export Fiber, FiberBundle, CotangentBundle, CotangentSpace, FVector
export AbstractPowerManifold,
AbstractPowerRepresentation,
Expand Down Expand Up @@ -900,8 +898,6 @@ export ×,
number_of_coordinates,
one,
power_dimensions,
parallel_transport_along,
parallel_transport_along!,
parallel_transport_direction,
parallel_transport_direction!,
parallel_transport_to,
Expand Down Expand Up @@ -943,8 +939,6 @@ export ×,
submanifold_components,
var,
vector_space_dimension,
vector_transport_along,
vector_transport_along!,
vector_transport_direction,
vector_transport_direction!,
vector_transport_to,
Expand Down
Loading
Loading