Group retraction from invariant exp/log

[olivierverdier]

This PR will

• Reduce code duplication
• Fix Wrong parameter for GroupExponentialRetraction #682

It deprecates using GroupExponentialRetraction with a parameter. This parameter is now unused and should be scheduled for removal it in a breaking release.

The functionality of GroupExponentialRetraction is unchanged.

• Replace GroupExponentialRetraction with invariant exp/log
• Fix exp_inv/log_inv

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.36%. Comparing base (45b9cfa) to head (4a61011).

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #762      +/-   ##
==========================================
- Coverage   96.39%   96.36%   -0.03%     
==========================================
  Files         123      123              
  Lines       11445    11426      -19     
==========================================
- Hits        11032    11011      -21     
- Misses        413      415       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[olivierverdier]

Interesting, I must have made a mistake: although all the tests pass, something is wrong with this PR.

[olivierverdier]

I see, the problem is that there is a mistake in the implementations of exp_inv and/or log_inv. Those have to be fixed first.

[olivierverdier]

The tests fails because SpecialEuclidean with HybridTangentRepresentation is broken: it doesn't implement translate_diff. I can't fix it on my computer, it takes too much time, even with just one test file. 🐌

Honestly, this HybridTangentRepresentation should be removed, what is it good for? I will delay this PR until that is fixed.

[kellertuer]

Just out of curiosity: What do you actually rund and why is it slow?
Sure, we do not have a nice test suite here – tried starting it once, but only got a nice scheme now that I started on in Lie groups.

for the hybrid representation – I think we will not fix that here, but maybe introduce different representations in Lie groups then. For now LieGroups.jl only supports representation of tangents in the Lie algebra as we discussed yesterday; extending that is only a next step when a Lie group comes along that requires that. On the other hand “converting to” Manifold tangent vectors might be a nice additional function to consider/introduce there as well.

[olivierverdier]

I'm only running the test in test/group/special_euclidean.jl and it takes forever, I don't know why.

One day the HybridTangentRepresentation will be removed: it is incomplete (this is why the tests fail here), and, in my opinion, confusing to users, so it has no future. When the day of that demise comes, the tests of this PR will pass. This PR can be put on hold until then.

As a quickfix, one could add to the documentation of GroupExponentialRetraction, that using anything else that LeftForwardAction() will either do nothing or cause unexpected results, so that parameter should be fixed to LeftForwardAction() for now.

[kellertuer]

I'm only running the test in test/group/special_euclidean.jl and it takes forever, I don't know why.

Ah, yeah maybe we overcast there a bit too much.

One day the HybridTangentRepresentation will be removed: it is incomplete

One day (tm) the GroupManifold will be removed here anyways, since that will be superseded by LieGroups.jl, what we introduce there in representations is then a discussion for that package.
But I already have on my plans to add deprecations here, that will be released as soon as the corresponding functionality is released on LieGroups.jl.