JuliaManifolds / ManifoldsBase.jl

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

Introduce a `RetractionWithKeywords` to specify / fix keywords on Layer 1 #130

Closed kellertuer closed 1 year ago

kellertuer commented 1 year ago

This is a continuation of https://github.com/JuliaManifolds/Manifolds.jl/issues/547, to introduce retractions (and inverse retractions and vector transports) where you can fix keywords in the retraction types.

This is necessary since keywords only act on layer 3 on the actual implementation – and with this step they are “added” in layer two before “resolving” the retraction type and passing them down to layer 3.

codecov[bot] commented 1 year ago

Codecov Report

Merging #130 (d6bddf6) into master (437f341) will increase coverage by 0.00%. The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master     #130   +/-   ##
=======================================
  Coverage   99.81%   99.82%           
=======================================
  Files          18       18           
  Lines        2200     2247   +47     
=======================================
+ Hits         2196     2243   +47     
  Misses          4        4           
Impacted Files Coverage Δ
src/ManifoldsBase.jl 99.31% <ø> (ø)
src/parallel_transport.jl 100.00% <100.00%> (ø)
src/retractions.jl 100.00% <100.00%> (ø)
src/vector_transport.jl 100.00% <100.00%> (ø)

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

kellertuer commented 1 year ago

The retraction case should be finished (including tests); If this is a way we want to go, the other two are not much work to do, but I would like to be sure that this is a good approach.

mateuszbaran commented 1 year ago

I think this new retraction looks fine, this is a good approach.

mateuszbaran commented 1 year ago

Sorry for closing, I've accidentally clicked the wrong button.

kellertuer commented 1 year ago

So this should be feature complete, but I have to take a look how to test the final variant, the VectorTransportWithKeywords.

kellertuer commented 1 year ago

I checked the missing lines - the one case (mid point on <=1.1) is called so that is a false positive; where I am not sure whether maybe the 1.1 just did not report properly.

The other – on ValidationManifolds, I checked and hopefully fixed.