ManifoldsBase.jl icon indicating copy to clipboard operation
ManifoldsBase.jl copied to clipboard

Move GroupManifold interface to ManifoldsBase.

Open kellertuer opened this issue 4 years ago • 38 comments
trafficstars

This PR aims to provide the Lie group functionality already in ManifoldsBase.

While transferring the code (ok, in the end it was just copying 974 lines) I noticed two things:

  • direction not documented (GroupManifold, line 896)
  • GROUP_MANIFOLD_BASIS_DISAMBIGUATION currently unused

Also I would like to use this to propose to add a few small features

Homogeneous space

We could introduce an Homogneous Space wikipedia as a more generic case? This might be nice for some ManifoldDiffEq.jl solvers. I was thinking that again we have an abstract type and a concrete one; the abstract one would be a supertype of the `AbstractGroupManifold (where then M=G) and a concrete subtype with a manifold a group and an action. What do you think?

Lie Groups

Currently we use the technical term GroupManifold which I think we should keep, but we could introduce LieGroup either as an const or as an easy constructor with defaults LieGroup(Rotations(3)) could then just return the default Lie group on the rotations, i.e. SpecialOrthogonal(3) (though this specific case would have to be defined in Manifolds for sure). This would be kind of a higher level (or semantic) way to use GroupManifolds. Maybe that would be easier to a few end users. What do you think?

Still missing

  • [ ] Test coverage
  • [ ] Check documentation docstrings thoroughly (since they will only be rendered when adapting Manifolds.jl interface by a new section.)

I think this PR is nonbreaking though technically now Manifolds (when bumped to this without changes) would redefine a few things.

kellertuer avatar Aug 20 '21 07:08 kellertuer

Codecov Report

Merging #84 (53a7b8c) into master (4d8e6de) will decrease coverage by 12.08%. The diff coverage is 0.00%.

:exclamation: Current head 53a7b8c differs from pull request most recent head 892fd23. Consider uploading reports for the commit 892fd23 to get more accurate results Impacted file tree graph

@@             Coverage Diff             @@
##           master      #84       +/-   ##
===========================================
- Coverage   99.85%   87.76%   -12.09%     
===========================================
  Files          14       15        +1     
  Lines        1339     1504      +165     
===========================================
- Hits         1337     1320       -17     
- Misses          2      184      +182     
Impacted Files Coverage Δ
src/GroupManifold.jl 0.00% <0.00%> (ø)
src/ManifoldsBase.jl 97.43% <ø> (-1.72%) :arrow_down:
src/bases.jl 100.00% <0.00%> (ø)
src/PowerManifold.jl 100.00% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 4d8e6de...892fd23. Read the comment docs.

codecov[bot] avatar Aug 20 '21 07:08 codecov[bot]

We could introduce an Homogneous Space wikipedia as a more generic case? This might be nice for some ManifoldDiffEq.jl solvers. I was thinking that again we have an abstract type and a concrete one; the abstract one would be a supertype of the `AbstractGroupManifold (where then M=G) and a concrete subtype with a manifold a group and an action. What do you think?

That's an interesting idea. I think, though, that it should be developed in Manifolds.jl first. What operations would actually be simplified by having homogeneous spaces?

Currently we use the technical term GroupManifold which I think we should keep, but we could introduce LieGroup either as an const or as an easy constructor with defaults LieGroup(Rotations(3)) could then just return the default Lie group on the rotations, i.e. SpecialOrthogonal(3) (though this specific case would have to be defined in Manifolds for sure). This would be kind of a higher level (or semantic) way to use GroupManifolds. Maybe that would be easier to a few end users. What do you think?

I think having dedicated constructors for each important Lie group may be actually more convenient.

I think this PR is nonbreaking though technically now Manifolds (when bumped to this without changes) would redefine a few things.

It should be non-breaking for everything except Manifolds.jl but IIRC redefinitions are a bad idea, so I'd prefer to make a breaking release here.

BTW, why did you only partially move group actions here?

mateuszbaran avatar Aug 20 '21 12:08 mateuszbaran

We could introduce an Homogneous Space wikipedia as a more generic case? This might be nice for some ManifoldDiffEq.jl solvers. I was thinking that again we have an abstract type and a concrete one; the abstract one would be a supertype of the `AbstractGroupManifold (where then M=G) and a concrete subtype with a manifold a group and an action. What do you think?

That's an interesting idea. I think, though, that it should be developed in Manifolds.jl first. What operations would actually be simplified by having homogeneous spaces?

Ok, then lets not include it in this PR. My idea is that some operations can already be defined on homogeneous spaces (and then inherited by Lie groups). So it would not be necessarily a simplification but make the “coupling” Group&Manifold better/more clear.

Currently we use the technical term GroupManifold which I think we should keep, but we could introduce LieGroup either as an const or as an easy constructor with defaults LieGroup(Rotations(3)) could then just return the default Lie group on the rotations, i.e. SpecialOrthogonal(3) (though this specific case would have to be defined in Manifolds for sure). This would be kind of a higher level (or semantic) way to use GroupManifolds. Maybe that would be easier to a few end users. What do you think?

I think having dedicated constructors for each important Lie group may be actually more convenient.

Sure we have those already, I was thinking here about “getting” to the (default) Lie group for rotations that way (in the sense of decorating). But if you think that is not so interesting, we can also leave that out.

BTW, why did you only partially move group actions here?

I hope I only moved the abstract stuff here and not the concrete (addition/multiplication) parts? And the abstract parts can surely be in ManifoldsBase. Did I do that wrongly somewhere?

kellertuer avatar Aug 20 '21 12:08 kellertuer

Ok, then lets not include it in this PR. My idea is that some operations can already be defined on homogeneous spaces (and then inherited by Lie groups). So it would not be necessarily a simplification but make the “coupling” Group&Manifold better/more clear.

Which operations do you have in mind? I think I don't quite see what you want to do but maybe let's discuss that in a separate issue.

Sure we have those already, I was thinking here about “getting” to the (default) Lie group for rotations that way (in the sense of decorating). But if you think that is not so interesting, we can also leave that out.

I'm not against, I just don't see the benefit here.

BTW, why did you only partially move group actions here?

I hope I only moved the abstract stuff here and not the concrete (addition/multiplication) parts? And the abstract parts can surely be in ManifoldsBase. Did I do that wrongly somewhere?

Maybe it's fine but it feels weird to have LeftAction and RightAction here without any actions. Also, translate is basically apply for GroupOperationAction -- that's what I mean by partially moving group actions. We could have GroupOperationAction here as well.

mateuszbaran avatar Aug 20 '21 14:08 mateuszbaran

Ok, then lets not include it in this PR. My idea is that some operations can already be defined on homogeneous spaces (and then inherited by Lie groups). So it would not be necessarily a simplification but make the “coupling” Group&Manifold better/more clear.

Which operations do you have in mind? I think I don't quite see what you want to do but maybe let's discuss that in a separate issue.

Since we do not do it here, sure lets do that in a separate issue. My main idea is to have a HomogeneousSpace{G,M} type that might make some functions easier to read? It couples the manifold and the group (but has not as many properties as if that would yield a Lie group, but a subset of those)

Sure we have those already, I was thinking here about “getting” to the (default) Lie group for rotations that way (in the sense of decorating). But if you think that is not so interesting, we can also leave that out.

I'm not against, I just don't see the benefit here.

It would just get easier to use/read; whether that is super useful, I am also not sure.

BTW, why did you only partially move group actions here?

I hope I only moved the abstract stuff here and not the concrete (addition/multiplication) parts? And the abstract parts can surely be in ManifoldsBase. Did I do that wrongly somewhere?

Maybe it's fine but it feels weird to have LeftAction and RightAction here without any actions. Also, translate is basically apply for GroupOperationAction -- that's what I mean by partially moving group actions. We could have GroupOperationAction here as well.

Did I miss to move GroupOperationAction? I meant to exactly move all abstract stuff here.

kellertuer avatar Aug 20 '21 14:08 kellertuer

Did I miss to move GroupOperationAction? I meant to exactly move all abstract stuff here.

I don't see GroupOperationAction here. It's not abstract but I think it should be moved here as well.

mateuszbaran avatar Aug 21 '21 09:08 mateuszbaran

Ah, I read it the other way around, sorry (that it was here and should not be). I will check for the tests and the error with Julia 1.0 later today anyways and then also look for that.

kellertuer avatar Aug 21 '21 09:08 kellertuer

While I checked where I missed GroupOperationAction, I noticed that that type is in principle what I meant with the Homogeneous Space, i.e. couple a manifold and an action/other space (most prominently translation & rotation actions then).

Edit: Since it seems really to be the case that GroupOperationAction does not model an Action but a Homogeneous Space (with a transitive group operation) I would (since we are breaking already anyways) like to rename this to HomogeneousSpace – that would also make it easier / more logical to distinguish group and manifold (i.e. rename base_group() to ``get_groupoandg_manifoldtoget_manifold`. (and use both a little more consistently). What do you think @sethaxen (since you designed this initially)?

kellertuer avatar Aug 21 '21 14:08 kellertuer

Hm, are you sure? GroupOperationAction doesn't store the action explicitly, and is not a manifold. Things like RotationAction or TranslationAction should be closer I think? Anyway, we may also introduce arbitrary quotients and make homogeneous spaces a special case. See also https://github.com/JuliaManifolds/Manifolds.jl/issues/205 .

mateuszbaran avatar Aug 21 '21 14:08 mateuszbaran

But exactly that makes it currently a little strange, because the GroupOperationAction currently seems to claim to be an action, but as a first argument to apply seems to ge GroupManifold-like. I think it should act like an AbstractGroupManifold there (and similar to the embedded manifold in some sense it also decorates the manifold that is also in there. The current model makes it necessary to check the actual action, whether it first the group stored – why not directly use a group manifold then? And then – ”adding” a group and its action to a manifold – we have a homogeneous space.

kellertuer avatar Aug 21 '21 14:08 kellertuer

That's a good point -- the division between actions and their respective quotients is somewhat arbitrary and rearranging things a bit may be a good idea.

The current model makes it necessary to check the actual action, whether it first the group stored – why not directly use a group manifold then?

For which operation? apply for example needs to know the action to apply it. We can either leave it as is, replace action with a quotient space as the first argument, or put the action as an additional argument.

And then – ”adding” a group and its action to a manifold – we have a homogeneous space.

I also want to handle non-transitive actions so not all quotient spaces are going to be homogeneous spaces.

mateuszbaran avatar Aug 21 '21 14:08 mateuszbaran

Well for me an action storing its group feels the wrong way around. I am not yet finished with modelling the new types, but I do not like this line for example

https://github.com/JuliaManifolds/Manifolds.jl/blob/272f2581f0629a89150a000c1540f0feade961d2/src/groups/group_operation_action.jl#L29

Since in principle the action (operation?) would be a type of the group and here its the other way around – what if the group operation within the action is different form the action you have in mind? I think that also action and operation refer to nearly the same thing?

And I am far away from quotient spaces here, but as a sketch

"""
    AbstractHomogeneousSpace{𝔽, GO, T <: AbstractDecoratorType} <: AbstractDecoratorManifold{𝔽,T}

An abstract homogeneous space, i.e. an [`AbstractManifold`](@ref) `M`
together with an [`AbstractGroupManifold`](@ref) `G` (to be precise its group action)
or a [`AbstractGroupOperation`](@ref) `O` that acts transitively on the Manifold.

Note that `M` equal to `G` yields the case of just a [`GroupManifold`](@ref) `G`.
"""
abstract type AbstractHomogeneousSpace{𝔽, O, T<:AbstractDecoratorType} <:
              AbstractDecoratorManifold{𝔽,T} end

would render the g_manifold (https://github.com/JuliaManifolds/Manifolds.jl/blob/272f2581f0629a89150a000c1540f0feade961d2/src/groups/group_operation_action.jl#L23 ) obsolete since that is then just the base manifold.

kellertuer avatar Aug 21 '21 15:08 kellertuer

Ah here https://github.com/JuliaManifolds/Manifolds.jl/blob/272f2581f0629a89150a000c1540f0feade961d2/src/groups/semidirect_product_group.jl#L43-L53 was the lines that feel strange.

You take a group (that has an action) and a manifold and an action (that internally has a group and a manifold) and then those have to match. Why then store everything double if that only yields that one has to check that the fit?

kellertuer avatar Aug 21 '21 15:08 kellertuer

I mean, maybe you both think all this is bogus – but then someone else has to move the Lie group stuff to Base, because then I seem to have a major problem in understanding the modelling here, but for me the order is a little mixed up in the current modelling (operation in group but group and manifold in action...)

kellertuer avatar Aug 21 '21 15:08 kellertuer

It would be nice to improve the current design but it's a hard problem. Take apply as an example. It may need to know the manifold it acts on, the group that acts, and the action itself. It was easy to just put all these things in the action. But it causes weird issues with semidirect products as you noticed. If we separate action from the group and manifold, then we need to provide them to apply through separate arguments. Which order would be right?

mateuszbaran avatar Aug 21 '21 15:08 mateuszbaran

I currently get confused that apply and an order rotates the order sometimes. And currently I am confusing Action and operation. Maybe I should stop here, because I seem to have a major misunderstanding somewhere. Also there seems to be a direction function that is nowhere documented bur implemented twice?

My understanding of the action (until your last post) was that it is the group operation of the group? Maybe its not? Then I am missing some major documentation what is what here and why the action (which is an operation?) internally stores group and manifold. Would and Addition internally store the Rn? And the Rn again?

So Main Question: What is the difference between an action and an operation? I got lost somewhere there since this is also nowhere documented as far as I see. And then – is the action direction the decision wherther the group operation acts transitively from the right or left?

(I stop here, until both of you have clarified this, since I am now confused and someone else might have to take over or clarify).

kellertuer avatar Aug 21 '21 15:08 kellertuer

Apply would habe as its first argument the homogeneous space – that has both the group and the manifold. That would not be a problem. I think often what was the AbstractOperationAction would now be the homogeneous space – up to the difference between operation and action (see above) where I got confused.

Also what confuses me, that we have the apply documentation

Apply action a to the point p with the rule specified by A. The result is saved in q.

Which for me sounds like a is an action (operation?) so maybe +? but then in TranslationAciton we have apply!(::TranslationAction, q, a, p) = (q .= p .+ a) so a is not an action but a vector? and since action is nowhere documented what it actually is (but it can have a direction maybe? left or right? When has a vector a left or right direction?) – I get confused here.

kellertuer avatar Aug 21 '21 15:08 kellertuer

rom rotation it seems that left action means multiplication from the left. See https://github.com/JuliaManifolds/Manifolds.jl/blob/272f2581f0629a89150a000c1540f0feade961d2/src/groups/rotation_action.jl#L43-L46

And right action means – multiplication from the left just with the inverse of the action (rotation matrix). So somehow elements of the group are called actions (why?) and their direction is a question whether we invert or not but right is still application from the left.

But for the default (abstract) case, this does not seem to matter https://github.com/JuliaManifolds/Manifolds.jl/blob/272f2581f0629a89150a000c1540f0feade961d2/src/groups/group_operation_action.jl#L29-L31 ?

I think I am missing something here.

kellertuer avatar Aug 21 '21 15:08 kellertuer

Sorry for all these posts – I really seem to miss a lot in the theory with actions operations and directions, sometimes actions seem operations (+ / *), then the are vectors/rotations, sometimes directions matter, sometimes not, but its not documented and for the last link I maybe would have expected compose instead to translate?

I think we might have missed to document this in a little more detail, so its easier to follow the code flow.

kellertuer avatar Aug 21 '21 15:08 kellertuer

I currently get confused that apply and an order rotates the order sometimes. And currently I am confusing Action and operation.

Operation is internal to the group (a part of its definition). For example matrix multiplication on matrix Lie groups. Action is how a group acts on a different manifold. I think a good example may be actions of SO(2) on R^3 -- you have a group, a manifold, and additionally need to specify the axis of rotation somewhere. Then it's rotation about that axis by an angle specified by an element of SO(2).

Apply action a to the point p with the rule specified by A. The result is saved in q.

Which for me sounds like a is an action (operation?) so maybe +? but then in TranslationAciton we have apply!(::TranslationAction, q, a, p) = (q .= p .+ a) so a is not an action but a vector? and since action is nowhere documented what it actually is (but it can have a direction maybe? left or right? When has a vector a left or right direction?) – I get confused here.

a is just an element of the group, and how it's used depends on the action.

Action direction may be a bit unclear, right. I'm not even quite sure why it's defined this way for the rotation action...

Sorry for all these posts – I really seem to miss a lot in the theory with actions operations and directions, sometimes actions seem operations (+ / *), then the are vectors/rotations, sometimes directions matter, sometimes not, but its not documented and for the last link I maybe would have expected compose instead to translate?

I think we might have missed to document this in a little more detail, so its easier to follow the code flow.

No problem, it really is not properly documented. I'll have to take a deeper look to answer all questions.

mateuszbaran avatar Aug 21 '21 15:08 mateuszbaran

Sorry for all these posts – I really seem to miss a lot in the theory with actions operations and directions, sometimes actions seem operations (+ / *), then the are vectors/rotations, sometimes directions matter, sometimes not, but its not documented and for the last link I maybe would have expected compose instead to translate?

apply, translate and compose are almost the same for GroupOperationAction. Here the direction only tells whether apply(A::GroupOperationAction, a, p) is compose(A.group, a, p) or compose(A.group, p, a). It just goes through translate, I think mostly because translate has important differentials that we need.

mateuszbaran avatar Aug 21 '21 15:08 mateuszbaran

I currently get confused that apply and an order rotates the order sometimes. And currently I am confusing Action and operation.

Operation is internal to the group (a part of its definition). For example matrix multiplication on matrix Lie groups. Action is how a group acts on a different manifold. I think a good example may be actions of SO(2) on R^3 -- you have a group, a manifold, and additionally need to specify the axis of rotation somewhere. Then it's rotation about that axis by an angle specified by an element of SO(2).

So operation is matrix multiplication. But if I have a manifold and a group, then still a from SO(3) and p from the manifold, then a is an action? That is a very strange name for a point on a manifold (Lie group). For me it would then be a operates on p and not an action that operates on p? For me that is two verbs on a point p where I am missing that one verb (action) should be the point a from SO(3).

Apply action a to the point p with the rule specified by A. The result is saved in q.

Which for me sounds like a is an action (operation?) so maybe +? but then in TranslationAciton we have apply!(::TranslationAction, q, a, p) = (q .= p .+ a) so a is not an action but a vector? and since action is nowhere documented what it actually is (but it can have a direction maybe? left or right? When has a vector a left or right direction?) – I get confused here.

a is just an element of the group, and how it's used depends on the action.

Above you said a is the action? That is exactly what confuses me. a is a point from the group and an action and depends on itself? I still don't get it. Is the action a now an operation or a point from the group? Or both? a point can not have a direction, though.

kellertuer avatar Aug 21 '21 16:08 kellertuer

Sorry for all these posts – I really seem to miss a lot in the theory with actions operations and directions, sometimes actions seem operations (+ / *), then the are vectors/rotations, sometimes directions matter, sometimes not, but its not documented and for the last link I maybe would have expected compose instead to translate?

apply, translate and compose are almost the same for GroupOperationAction. Here the direction only tells whether apply(A::GroupOperationAction, a, p) is compose(A.group, a, p) or compose(A.group, p, a). It just goes through translate, I think mostly because translate has important differentials that we need.

Ok, that is a little confusing, but I think my main problem is an action indeed.

My understanding would be as follows:

We take a homogeneous space H that is

  • a group G (or at least its operation)
  • a manifold M
  • a direction (left/right) which indicates in which direction the operation acts transitively.

then we do not have an action. we would just have

g * p (or p * g for right) where * is the operation from G. Whether the space stores the group or the operation – might be a matter of taste I would prefer to store G instead of just the MultiplicationOperation for example, but one can manage both.

But we can still take all g and p

and this text does not include actions. So I must miss something. Is * the action? or g? It can not be g because g does not have a direction... so its *? but why store that extra , the group should know its operation?

kellertuer avatar Aug 21 '21 16:08 kellertuer

So operation is matrix multiplication. But if I have a manifold and a group, then still a from SO(3) and p from the manifold, then a is an action? That is a very strange name for a point on a manifold (Lie group). For me it would then be a operates on p and not an action that operates on p? For me that is two verbs on a point p where I am missing that one verb (action) should be the point a from SO(3).

Action is the way elements on group operate on the manifold. a is not an action, it is an element of a Lie group. The same group may operate on the same manifold in many different ways, and the action tells which way is currently used.

Above you said a is the action?

Where did I say that? I didn't mean that.

mateuszbaran avatar Aug 21 '21 16:08 mateuszbaran

and this text does not include actions. So I must miss something. Is * the action? or g? It can not be g because g does not have a direction... so its *? but why store that extra , the group should know its operation?

I think my above example with SO(2) and R^3 demonstrates well what an action is -- and that the group, manifold, and direction don't fully specify apply.

mateuszbaran avatar Aug 21 '21 16:08 mateuszbaran

Action is the way elements on group operate on the manifold. a is not an action, it is an element of a Lie group. The same group may operate on the same manifold in many different ways, and the action tells which way is currently used.

But that means that the action is not the Lie group operation? What is the action then actually? In my had it is more like a magic hat now... it is not the group operation? What is it then and how is it defined? And why is it not the group operation? I thought that was the whole idea of taking a group :D

Where did I say that? I didn't mean that.

Oh, sorry, maybe only the docs said that a is the action.

kellertuer avatar Aug 21 '21 16:08 kellertuer

But that means that the action is not the Lie group operation? What is the action then actually? In my had it is more like a magic hat now... it is not the group operation? What is it then and how is it defined? And why is it not the group operation? I thought that was the whole idea of taking a group :D

The action is the \alpha function described here: https://en.wikipedia.org/wiki/Group_action . Does this make things more clear?

mateuszbaran avatar Aug 21 '21 16:08 mateuszbaran

and this text does not include actions. So I must miss something. Is * the action? or g? It can not be g because g does not have a direction... so its *? but why store that extra , the group should know its operation?

I think my above example with SO(2) and R^3 demonstrates well what an action is -- and that the group, manifold, and direction don't fully specify apply.

But how would one model that? Axis of rotation – so for a general combination of a manifold and a group this is really some magic hat? I had hoped that for R3 only SO(3) is allowed, since then matrix multiplication (or the group action to be precise) would just do the job – but it can be anything? How would one model that?

kellertuer avatar Aug 21 '21 16:08 kellertuer

...but well. Then I can basically only conclude that all ideas in this PR are bogus and this will lead nowhere and we have to close this :(

I am not able to model actions properly in these sketches I fear.

kellertuer avatar Aug 21 '21 16:08 kellertuer

But how would one model that? Axis of rotation – so for a general combination of a manifold and a group this is really some magic hat? I had hoped that for R3 only SO(3) is allowed, since then matrix multiplication (or the group action to be precise) would just do the job – but it can be anything? How would one model that?

I can write that example, but the key thing is making a concrete subtype of AbstractGroupAction that stores the rotation axis, which can be then accessed in apply and other methods.

mateuszbaran avatar Aug 21 '21 16:08 mateuszbaran