Add group action param to Product Lie Group

[rohan-bansal]

I tried adding the group action parameter by way of a sibling class to ProductLieGroup, but the extra re-implementation felt unnecessary.

The version I ran with is implemented directly into the ProductLieGroup, keeping all the branches/if-checking in the internals, and exposing a lean public template/API.

[dellaert]

Awesome start! Some quick comments:

1. I remember making some changes to this class template in the other PR. I think they were only formatting changes, but please make sure. Note, we format with clang-format, Google style.
2. I don't think we need DirectProductAction. The code does not actually use it now in the "direct" branches. Please check.
3. So, we can do this: if no action is given, i.e. it's a dummy argument, it's assumed to be a direct group.

[jenniferoum]

Looks good to me. I was able to pull the code and run a unit test with the SD group.

[dellaert]

Left some comments. And also love to see a little bit more comment in the code on how semi product is different. And it suspicious that in many cases the semi branch seems simpler.

[dellaert]

@rohan-bansal ping

[rohan-bansal]

Sorry for the delay @dellaert , have been feeling a bit under the weather.

Changes:

• Used the simpler generic function implementations where possible instead of having if-else branches
• Commented the direct/semi-direct paths to explain better
• Addressed misc. comments

The one thing I was not able to do that I would appreciate some guidance on is making the Expmap/Logmap generic for the semi-direct product.

For a semidirect product G ⋉ H, the exponential map is Expmap(ξ, v) = (Expmap_G(ξ), J_left(ξ) · v) where J_left(ξ) = ∫₀¹ D_h φ(Expmap_G(tξ), e_H) dt. From what I understand after talking a bit with Claude, there is no closed form solution for this integral that can be derived generically, it would need a group-specific formula or a very computationally-expensive numerical quadrature based method. I think the simplest thing to do is have the action supply Expmap/Logmap, but let me know if you think I should do it differently.

[dellaert]

I’ll email - I might have just the thing.

[dellaert]

Good progress!!! 2 comments

[dellaert]

Pretty cool. But is this cheaper?
Also - maybe in a next PR - we need to think about a mechanism to special-case Rot3 and other G groups we have closed forms for.

[rohan-bansal]

Good point, this is not intended to be cheaper than closed forms like the Rot3 kernels, and updated the comment to make that explicit. I agree that we should special-case these groups in a separate PR.

[dellaert]

Status?

[rohan-bansal]

From my end it looks ready for re-review. I did my best to be faithful to the PDFs, hopefully there are no mistakes; I would hope that the tests catch all of them. Just added another generality test with Rot2 x R2.

[dellaert]

My comment on phi01Kernel was that it does an expm on a larger matrix. We also don't always need both. So I'd prefer two separate phi0 and phi1 kernels.

[rohan-bansal]

Oh I see, I misinterpreted. Will make this change

[rohan-bansal]

Updated.

[rohan-bansal]

Replaced numerical code with analytical.