JuliaManifolds / ManifoldsBase.jl

Basic interface for manifolds in Julia
https://juliamanifolds.github.io/ManifoldsBase.jl/
MIT License
87 stars 8 forks source link

Add shooting method #126

Closed sethaxen closed 1 year ago

sethaxen commented 1 year ago

This PR ports the shooting method code from https://github.com/JuliaManifolds/Manifolds.jl/pull/535. It slightly redesigns the API to better align with how the other (inverse) retractions are implemented here. The code lives in its own file, since it needs access to the AbstractVectorTransport type, which isn't available until after retractions.jl is imported.

I'm not certain the best way to test this. I could use DefaultManifold, but Euclidean is too easy of a case. Is there anything like a minimal sphere implementation in the test suite?

codecov[bot] commented 1 year ago

Codecov Report

Merging #126 (c97a191) into master (715932d) will decrease coverage by 0.04%. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #126      +/-   ##
==========================================
- Coverage   99.86%   99.81%   -0.05%     
==========================================
  Files          17       18       +1     
  Lines        2146     2188      +42     
==========================================
+ Hits         2143     2184      +41     
- Misses          3        4       +1     
Impacted Files Coverage Δ
src/ManifoldsBase.jl 99.31% <ø> (ø)
src/shooting.jl 100.00% <100.00%> (ø)
src/PowerManifold.jl 99.38% <0.00%> (-0.21%) :arrow_down:

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

kellertuer commented 1 year ago

What do you mean by “too easy”? I think we can only check against the DefaultManifold here.

Unless you define the few function you need on a DefaultSphere locally in the test suite for the shooting method?

sethaxen commented 1 year ago

Too easy because any inverse retraction is as good as the actual logarithm on Euclidean, so the initial inverse retraction will solve the problem and there are branches of the shooting code we won't hit.

I think we do need something like a sphere.

kellertuer commented 1 year ago

...otherwise you could come up with a retraction that is not exact on the default manifold.

kellertuer commented 1 year ago

Too easy because any inverse retraction is as good as the actual logarithm on Euclidean, so the initial inverse retraction will solve the problem and there are branches of the shooting code we won't hit.

I think we do need something like a sphere.

I think just the smaller amount of functions you need for a test sphere would be the best idea. edit: and maybe really just projection retraction and projection vector transport.

sethaxen commented 1 year ago

This is ready for review.