Move ManifoldDiff extension here

[mateuszbaran]

See here: JuliaManifolds/ManifoldDiff.jl#20

[codecov]

Codecov Report

Merging #623 (b70bf1b) into master (c6ec0a5) will increase coverage by 0.04%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #623      +/-   ##
==========================================
+ Coverage   98.96%   99.01%   +0.04%     
==========================================
  Files         104      104              
  Lines       10046    10102      +56     
==========================================
+ Hits         9942    10002      +60     
+ Misses        104      100       -4     

Impacted Files	Coverage Δ
src/Manifolds.jl	86.66% <ø> (ø)
src/groups/group.jl	100.00% <100.00%> (ø)
src/manifolds/Circle.jl	100.00% <100.00%> (ø)
src/manifolds/Euclidean.jl	99.54% <100.00%> (+0.01%)	⬆️
src/manifolds/Hyperbolic.jl	100.00% <100.00%> (ø)
src/manifolds/ProductManifold.jl	96.59% <100.00%> (+0.13%)	⬆️
src/manifolds/Sphere.jl	100.00% <100.00%> (ø)

... and 5 files with indirect coverage changes

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

[mateuszbaran]

These new lines most likely won't be cover until ManifoldDiff is merged, and tests in that PR likely won't pass until this is merged. Which one do you prefer to merge and tag first?

[kellertuer]

For me its ore reasonable to remove it from ManifoldDiff first and then add it here, since otherwise there is a version where the extension exists twice.

[mateuszbaran]

I have no idea why Manopt gradient tests fail here. Locally they pass fine.

[kellertuer]

The error message seems to be quite Diff-related though

[mateuszbaran]

Yes, it doesn't see the adjoint_Jacobi_field method for Circle. That method was in the extension of ManifoldDiff. We have it in this PR now and it's covered by a test so this makes no sense to me.

[kellertuer]

Ah, maybe Manopt still checks our some old version? Because currently Manopt/ManifoldDiff/Manifolds in their most recent version do not play well together, since the extension is nowhere (reverted to 0.3.2 to get it working again). So I am not sure how to resolve this. Register this version and hope for the best?

[kellertuer]

The same happens for now on Manopt CI, I think the choice of having a version combination without this extension was not that good and a next release here should fix that.

On the other hand it means that there is now combinations of Manifolds and ManifoldDiff that do not work well together (one or two combinations where the extension is nowhere to be found), but I think this can not be avoided.

[kellertuer]

I think I found a solution. I will restrict ManifoldDiff in Manopt to 0.3.2, then it should work fine; we register the stochastic PR (maybe also together with the new tutorial update?) as a new version (with the 0.3.2 bound). Then this PR defines a few functions double for Manopt, but that should be fine; afterwards bumping version to 0.3.3 and Manifolds <= this PR should also make Manopt work again.

[mateuszbaran]

OK, that sounds like a good plan.

[kellertuer]

It worked :) After this is merged and registered I can use the version number from here and Diff 0.3.3 as lower bounds in Manopt, for the next version.

[mateuszbaran]

Great! So I think this PR can be merged now?

[kellertuer]

Yes :)