(Special) Orthogonal / Unitary / Euclidean groups

[kellertuer]

This PR aims to provide the special orthogonal group and the first concrete semidirect product: Special Euclidean, since the complex case is similar, this will also cover the special unitary case.

🛣️🗺️

• implement OrthogonalGroup
• Implement SpecialOrthogonalGroup
• Implement SpecialEuclideanGroup
• Implement UnitrayGroup
• Implement SpecialUnitaryGroup
• improve test coverage on the semidirect product groups.
• read the documentation carefully since we changed exp(G, Id, X) to exp(G,X) and similarly log(G, Ig, g) to log(G, g)

[codecov]

Codecov Report

Attention: Patch coverage is 95.66396% with 16 lines in your changes missing coverage. Please review.

Project coverage is 98.53%. Comparing base (5246f30) to head (5b86c5f).

Files with missing lines	Patch %	Lines
src/utils.jl	78.04%	9 Missing ⚠️
src/interface.jl	70.83%	7 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main      #17      +/-   ##
==========================================
- Coverage   99.20%   98.53%   -0.67%     
==========================================
  Files          13       19       +6     
  Lines         756     1095     +339     
==========================================
+ Hits          750     1079     +329     
- Misses          6       16      +10     

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

[kellertuer]

Status update 1: In theory SE(n) is defined, in practice I am not able to dispatch on the new const. Hence the constructor still falls back to calling the general show (of semidirect product) @mateuszbaran can you maybe take a look? After an hour of trying I am a bit out of ideas what is wrong with my constant SpecialEuclideanGroup definition.

[mateuszbaran]

I've fixed the issue with SpecialEuclideanGroup alias 🙂

[kellertuer]

Nice. I would not have noticed the tuple necessary during the rest of the year.

You changed one . to a map. Should I do that for the rest as well?

[kellertuer]

Oh and sorry for the capitalisation foo – Mac OS does not distinguish that on files when loading them … super annoying.

[mateuszbaran]

You changed one . to a map. Should I do that for the rest as well?

I generally prefer map over . when both are applicable because for the compiler it's much easier to optimize map.

Oh and sorry for the capitalisation foo – Mac OS does not distinguish that on files when loading them … super annoying.

No problem, that was an easy fix 🙂

[kellertuer]

The main work today was a bit of structure, since now one of my main worries already works – compose! – the remaining functions are hopefully just stuff to transfer from manifolds.jl the todo list is basically all commented our functions in the two new test files.

So I am confident that this should not be too complicated – and am quite happy with how semidirect turned out.

[kellertuer]

All dummies are there, now the rest is mainly getting all functions from the test suite implemented and tested.

[kellertuer]

I think I understand the problem of the last 3 tests failing:

the submanifold_component I implemented for AbstractArray and :Rotation for example returns a view, which usually these functions do not. The code above accesses 1 and 2 (not :Rotation and :Translation) hence does not return views and hence can not write back.

Should we maybe experiment first here in LieGroups with a submanifold_component_vew? In the long run that could maybe replace _write in ManifoldsBase.jl.

edit: For the commit below this comment I did a proof-of-concept “misusing” the submanifold_component to return views.
Previously on AbstractMatrix it would fail, now it works fine, but I seem to have missed a view on the extension (ArrayPartitioncase) by now.
Let's first decide what best to do before I try to fix both.

[mateuszbaran]

I think your submanifold_component methods are fine. They provide access to valid representations of components. I don't think we need a _read/_write distinction for submanifold_component, all access can be of the _write type.

[kellertuer]

Then we should maybe aim to get that consistent to a _write-like style?

Besides that I then just have to figure out where the ArrayPartition cases fail ;)

[mateuszbaran]

What's actually inconsistent? Which methods behave differently?

[kellertuer]

If I just “pass on” to G.manifold I think they are not views and will not write back, but I can check. Also I thought, the reason that _write exists was because subamifold_component was not meant to be “writable access”?

Feel free to experiment with the definitions in the extension yourself – some of them seem to not yet “write back”, so they might have to still return views I think.

[mateuszbaran]

If you pass on to G.manifold, those matrices are no longer valid points AFAIK.

Also I thought, the reason that _write exists was because subamifold_component was not meant to be “writable access”?

_write is for PowerManifold to deal with the case of arrays of immutable elements. We support a similar thing differently in ProductManifold (by implementing non-in-place method).

[kellertuer]

I see, then until now it did not matter whether submanifold_component was “writable” or not. I think it is at least not consistently writeable. I am also not sure we can get it consistently so.
However, that is the last point that needs to be fixed here for the tests :)

[mateuszbaran]

The failing tests are for the ArrayPartition representation; I'm not sure how it could be related to submanifold_components not working right if it just gives the component arrays which are writable. And the failing tests aren't even for the mutating methods.

[kellertuer]

Sure, the two methods called (exp/log without a point) they allocate and call the mutating ones. For me it looks like they are because the numbers are the same as for the case where AbstractMatrix did not “write back” the application of the action. (for exp/log which internally use 2 x compose and 1 x apply).
But I have a bit time on Wednesday to dive deeper into the code probably.

The tests afterwards that directly call the mutating functions just check that these give the same results as the allocating ones, to they produce the same (wrong) results currently)

[mateuszbaran]

If the output wasn't mutable, you'd get a runtime error or uninitialized memory, not a somewhat reasonably looking answer like I see. I'm 99% sure it's an unrelated logic error.

[kellertuer]

So for AbstractMatrix the problem was that there was a return A[1:n,1:n] that allocated a new matrix and returned that and the applyhence happened in place of that new matrix and not the original one which was then used in the following compose.

That does not result in an error just that the in-place computation is done “in the wrong place” and hence never written back. Maybe something like that is also currently happen, at least the numbers are the same wrong ones as in the one that I fixed before.

[mateuszbaran]

submanifold_component is already writable for ArrayPartition:

julia> using Manifolds, RecursiveArrayTools

julia> M = ProductManifold(Sphere(2), Sphere(3))
ProductManifold with 2 submanifolds:
 Sphere(2, ℝ)
 Sphere(3, ℝ)

julia> p = rand(M)
([-0.893222710741945, -0.008594280645648567, -0.4495323429465287], [0.6752061658308836, 0.005200949448816773, -0.2518961443382168, 0.6932661222188261])

julia> scp_a, scp_b = submanifold_components(M, p)
([-0.893222710741945, -0.008594280645648567, -0.4495323429465287], [0.6752061658308836, 0.005200949448816773, -0.2518961443382168, 0.6932661222188261])

julia> scp_a .= 0
3-element Vector{Float64}:
 0.0
 0.0
 0.0

julia> scp_b .= 0
4-element Vector{Float64}:
 0.0
 0.0
 0.0
 0.0

julia> p
([0.0, 0.0, 0.0], [0.0, 0.0, 0.0, 0.0])

[kellertuer]

Then I have zero idea where even to start to pin down that error. For the abstract matrix case it took me 3-4 hours to dive into the code, so I will do that then Wednesday probably again for ArrayPartition. From outside it looks like the same error because it is the same numbers as if apply was not applied. If it is not – then that is for some other reason that is looks like that.

But I can not dive into that today.

[kellertuer]

Step (1) I found a problem in compose for semidirect groups, where compose!(G, h, g, h) would “overwrite” one element im h top early (same for compose!(G, g, g, h)).

Now Left on Partition works
For Right, there seems to still be a sign error in the matrix Lie group exponential (hence also the CS one is wrong still). I could bring that down to exp(G, g, -X) would yield the same as the matrix multiplication – oh not completely, there is a mother sign error then in the T(n) component.
Not sure why, we only have one implementation for both currently (left&right); Maybe I confused exactly some left/right for them. If they need more than one unified implementation this also holds for the cases 3,4,n.
Is maybe now more a math problem than the one I solved now.

[kellertuer]

Found it. I was not clever enough to provide a correct Lie algebra tangent vector 🙄

But now finally tests run fine, so only reading the docs, and improving test coverage (once ManifoldsBase.jl 1.0 is available and Manifolds.jl has been adapted to that) are left to do.

This PR is finally feature complete.

[mateuszbaran]

Nice, it's great that you've managed to figure it out 👍

[kellertuer]

This PR only misses the code coverage check. For that we need both

• the merge of 🎆 Prepare for ManifoldsBase.jl 1.0 ManifoldsBase.jl#221 and the new version tagged (which seems nearly finished)
• the corresponding merge Adapt to ManifoldsBase 1.0 Manifolds.jl#781 to be merged and a new version registered (locally its tests already run fine)