Skip to content

Add jacobian_exp implementations for SE(2/3) and SGal(3) - #103

Open
Affie wants to merge 13 commits into
mainfrom
jt/more_jacobian_exp
Open

Add jacobian_exp implementations for SE(2/3) and SGal(3)#103
Affie wants to merge 13 commits into
mainfrom
jt/more_jacobian_exp

Conversation

@Affie

@Affie Affie commented Jul 13, 2026

Copy link
Copy Markdown
Collaborator

Hi, this PR adds jacobian_exp for SE(2/3), SGal(3), ProductLieGroups, and Abelian Groups.
I did use claude quite a bit especially for the documentation and tests.

We use it here (this file also contains some fall-backs and matrix group and algebra adjoints that might be worth upstreaming):
https://github.com/JuliaRobotics/ApproxManifoldProducts.jl/blob/b16ea75a69e051374b58064159ad732d9c74f18b/src/services/ManifoldsOverloads.jl#L88

@Affie Affie added enhancement New feature or request preview docs Add this label if you want to see a PR-preview of the documentation labels Jul 13, 2026
@Affie

Affie commented Jul 13, 2026

Copy link
Copy Markdown
Collaborator Author

Locally I only got 6 ambiguities:

Number of LieGroups.jl ambiguities: 6
Test Summary: | Pass  Total     Time
LieGroups.jl  | 5463   5463  3m12.4s
     Testing LieGroups tests passed 

@kellertuer

kellertuer commented Jul 13, 2026

Copy link
Copy Markdown
Member

Thanks for the PR!
We had 6 ambiguities before, those are fine, see our bound at

LG_LIMIT = 6

Thanks for the transparency on AI, then I will need more time to check the PR more closely – at lest the reference does exist 😄
In order for the @cite to work, though, you have add the corresponding entry also to the https://github.com/JuliaManifolds/LieGroups.jl/blob/main/docs/src/references.bib

edit: Oh I See, on CI it raised to 20; that we should carefully check, why that is the case; It was still 6 on last run of CI here, so I fear it is something in this PR then – and most probably interaction with the Identity https://juliamanifolds.github.io/LieGroups.jl/stable/interface/operations/#LieGroups.Identity

@Affie

Affie commented Jul 13, 2026

Copy link
Copy Markdown
Collaborator Author

Thanks for the transparency on AI, then I will need more time to check the PR more closely – at lest the reference does exist 😄

I really suck with writing documentation, but did explicitly tell AI what references to use for the docs and know them well, (they were already in use as well).

@kellertuer

Copy link
Copy Markdown
Member

(they were already in use as well).

Oh sorry! I was a bit quick there, since I only checked that it is not added in this PR, I did (sadly) not check whether it was already in there.

Yeah I am not a fan of AI, but since I get told at least 3 times a day by now I should do everything with AI and spend all my personal money (no support by university there) on some mega-token plan – I gave up arguing against AI and am just fine when people are honest by now.
Usually I have to compensate other peoples use of AI and fixing their mistakes then, that is what I am a bit tired of – but also nothing I can change. It is what life is by now probably, we are all slaves of AI.
And here it just means it might take a while until I get around to check the formulae against the source and such. At least I see the docs do include formulae, which is what we aim for in our packages.

@Affie

Affie commented Jul 13, 2026

Copy link
Copy Markdown
Collaborator Author

Ahh, I know why I only got 6 ambiguities, my local environment was stil on ManifoldsBase v2.4.0:

(LieGroups) pkg> st
Project LieGroups v0.1.12
Status `~/.julia/dev/LieGroups/Project.toml`
  [1cead3c2] Manifolds v0.11.28
⌃ [3362f125] ManifoldsBase v2.4.0
  [94ee1d12] Quaternions v0.7.7
  [90137ffa] StaticArrays v1.9.18
  [37e2e46d] LinearAlgebra v1.12.0
  [9a3f8284] Random v1.11.0
Info Packages marked with ⌃ have new versions available and may be upgradable.

If I update to ManifoldsBase v2.5.0 I also get the same failure:

Ambiguities: Test Failed at /home/johan/.julia/dev/LieGroups/test/test_aqua.jl:28
  Expression: length(ms) <= LG_LIMIT
   Evaluated: 10 <= 6

@Affie

Affie commented Jul 13, 2026

Copy link
Copy Markdown
Collaborator Author

The maths part should be fine, that was mostly me, but I did make some mistakes that was picked up. You will see the most complicated is for SGal and i referenced the equations from https://arxiv.org/pdf/2312.07555v11 directly.
I also checked it using monte carlo against the truncated series of the jacobian (test/groups/jacobian_exp_series_reference.jl) and spot checks are included in the tests.

Edit: Objective look would be very welcome as I tested it against my own understanding, so not very objective.

@kellertuer

Copy link
Copy Markdown
Member

Oh! I remembered that we print the ambiguities, see https://github.com/JuliaManifolds/LieGroups.jl/actions/runs/29268851175/job/86881148111#step:6:316

And my guess was not wrong – it's always the identity – just that this one is on us, it's the new + and - ambiguities with the new ZeroVector type, so its not your fault and you can keep the ambiguity on 10 for now. I hope to remember an issue for that tomorrow then.

@Affie

Affie commented Jul 13, 2026

Copy link
Copy Markdown
Collaborator Author

I think it is complaining about methods like `+(::Identity{AdditionGroupOperation}, Y::ManifoldsBase.ZeroVector) which is not valid operations. Should we ignore it or maybe throw an ArgumentError?

edit: ok I'll keep on 10 for now.

@Affie
Affie marked this pull request as ready for review July 13, 2026 18:48
@kellertuer

kellertuer commented Jul 13, 2026

Copy link
Copy Markdown
Member

Actually it is valid:
Identity{Addition} is a valid point on a Lie algebra (not only on the Lie group with group op +) and indeed that identity is the same as the zero vector (because that is the identity in a vector space).

We can really just define those 4 functions to return the identity argument of the two and we are fine.

Indeed both types have exactly the same reason: we want to avoid allocating zeros.

@Affie

Affie commented Jul 13, 2026

Copy link
Copy Markdown
Collaborator Author

You are correct, I didn't think that far. I'll add those 4 methods tomorrow.

@Affie

Affie commented Jul 13, 2026

Copy link
Copy Markdown
Collaborator Author

Like this:

Base.:+(g::Identity{AdditionGroupOperation}, ::ManifoldsBase.ZeroVector) = g
Base.:+(::ManifoldsBase.ZeroVector, g::Identity{AdditionGroupOperation}) = g
Base.:-(g::Identity{AdditionGroupOperation}, ::ManifoldsBase.ZeroVector) = g
Base.:-(::ManifoldsBase.ZeroVector, g::Identity{AdditionGroupOperation}) = g

@kellertuer

Copy link
Copy Markdown
Member

Yes. Exactly that. Perfect :)
And sure, it is two languages meeting basically, the zero vector was even inspired by the identity, so I think its just fair to resolve that here.

Thanks for adding them even here (and fixing our problem there) – maybe even add a short note to the changelog just for completeness :)

@kellertuer kellertuer left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here is a first few comments. I did not find the time yet to check/render the docs locally,
but the code in general looks fine to me

Comment thread src/group_operations/addition_operation.jl
Comment thread src/groups/special_euclidean_group.jl Outdated
Compute the Jacobian of the Lie group exponential in a basis of the Lie algebra on the
[`SpecialEuclideanGroup`](@ref)`(2)`.

For ``X ∈ 𝔰𝔢(2)`` with translation component ``v = (x, y)`` and rotation angle ``θ``,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you like to use the non raw string somewhere, we have some nice _tex(...) functions, especially to keep notation consistent for things like special Lie algebras and groups.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll have to learn how to use that a bit better. Looking at the other doc-strings, i think this should be 𝔤

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That would be the generic Lie algebra, for SE(e) the one you use is right. As long as you use a raw string, your code is also fine. I see where you have the g from, there we were not consistent and it should also be se.

The reason why we usually use this definition

define!(:Math, :se, :symbol, _tex(:frak, "se"))

to interpolate into a string with $(_math(:se)) is that this is a semantic description. If we ever decide to change the way se looks like in the docs, we just have to change that line 179 above.

But I do see that means one has to change several things in your doc string so if you prefer to leave it as it is now (please stay with se then not g) – that is also fine. I can refactor that later somewhen.

Comment thread src/groups/special_euclidean_group.jl Outdated
Comment thread src/groups/special_euclidean_group.jl Outdated
@mateuszbaran

Copy link
Copy Markdown
Member

I'm a bit confused here. Is this the Jacobian of the group exponential or special cases of jacobian_exp_argument or jacobian_exp_basepoint? You do specify the g but don't specify with respect to what the Jacobian is calculated.

@kellertuer

kellertuer commented Jul 14, 2026

Copy link
Copy Markdown
Member

Oh I was confused by that as well, but wanted to first check more closely whether that is on me ;)

To provide detail here: the base point one is the differential with respect to g (X fixed) and the arg one is the one with respect to X (g fixed).
Since the Lie group exponential has a fixed g=e (the identity), the Lie group one only has the argument one – but then no g. Or to even make it more precise, X is then also always from the Lie algebra.

@Affie

Affie commented Jul 14, 2026

Copy link
Copy Markdown
Collaborator Author

.... Since the Lie group exponential has a fixed g=e (the identity), the Lie group one only has the argument one – but then no g. Or to even make it more precise, X is then also always from the Lie algebra.

I saw this and followed the existing pattern, eg:

function jacobian_exp!(
M::SpecialOrthogonalGroup{ManifoldsBase.TypeParameter{Tuple{3}}}, J::AbstractMatrix, p, X, ::DefaultLieAlgebraOrthogonalBasis
)
θ = norm(M, p, X) / sqrt(2)
copyto!(J, I)
if θ 0
a = (cos(θ) - 1) / θ^2
b =- sin(θ)) / θ^3
J .+= a .* X .+ b .* (X^2)
end
return J
end

I wasn't too sure about the g argument though.
If g is not at the identity will this be equivalent to jacobian_exp_argument?

@codecov

codecov Bot commented Jul 14, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (1a25650) to head (6c11ff5).

Additional details and impacted files
@@            Coverage Diff             @@
##              main      #103    +/-   ##
==========================================
  Coverage   100.00%   100.00%            
==========================================
  Files           36        36            
  Lines         2899      3066   +167     
==========================================
+ Hits          2899      3066   +167     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@mateuszbaran

Copy link
Copy Markdown
Member

I see, I forgot that already exists. I've checked the discussion in #65 and I have no clue why the _argument suffix was dropped but the point argument was not. Either both should be dropped or both kept. Maybe this is a good time to make this straight. @kellertuer , is there a reason behind this convention or was that an oversight? If it's the second case, do you prefer adding _argument or dropping p/g?

@kellertuer

Copy link
Copy Markdown
Member

Hm, I mean this #65 (comment) definitely says that we only have exp_arg here, so there is no need to do the base one – and to then pass on the the manifold (arg) jac exp?.

I think what we could do, is the same as the default described herre
https://juliamanifolds.github.io/LieGroups.jl/stable/interface/group/#Base.exp-Tuple{AbstractLieGroup,%20Any,%20Any}
compared to here
https://juliamanifolds.github.io/LieGroups.jl/stable/interface/group/#Base.exp-Tuple{AbstractLieGroup,%20Any}

and do kind of both – but one for each.
For the first – with g – that defaults to the second anyways, we introduce the arg one – pushing forward the second case – allowing for a nice consistency with manifolds/tangent-spacey X
For the second case we can introduce the drop-g/p case.

I am just not sure which should be the default to implement, but I think the second one is probably nicer to implement and the first push-forward-defaults.

I hope this is – given the early evening time – not written in too confusing words.

@mateuszbaran

Copy link
Copy Markdown
Member

Yes, that comment explains that it's essentially jacobian_exp_argument, and it even comments that g may not be necessary, so I think it was just an oversight that g wasn't removed in that PR.

and do kind of both – but one for each.
For the first – with g – that defaults to the second anyways, we introduce the arg one – pushing forward the second case – allowing for a nice consistency with manifolds/tangent-spacey X
For the second case we can introduce the drop-g/p case.

That makes sense to me.

I am just not sure which should be the default to implement, but I think the second one is probably nicer to implement and the first push-forward-defaults.

The "no g" case as the default to implement sounds good. So basically all this PR needs to do is drop g and in the next PR we can fix the other groups I think.

@Affie

Affie commented Jul 15, 2026

Copy link
Copy Markdown
Collaborator Author

The "no g" case as the default to implement sounds good. So basically all this PR needs to do is drop g and in the next PR we can fix the other groups I think.

The interface and tests are also affected and it looks like only one group was implemented (SO2/3). So it looks easier to drop and deprecate the g parameter in this branch. I'm busy removing it for you to look at.

Comment thread src/interface.jl
Comment thread src/interface.jl
Comment on lines +894 to +919
Compute the Jacobian of the [Lie group exponential function](@ref exp(::AbstractLieGroup, ::Identity, :Any))
``$(_tex(:exp))_{$(_math(:G))}: $(_math(:𝔤)) → $(_math(:G))`` at ``X ∈ $(_math(:𝔤))``,
represented in an [`AbstractBasis`](@extref `ManifoldsBase.AbstractBasis`) ``b`` of the [`LieAlgebra`](@ref) ``$(_math(:𝔤))``.

The (classical) differential ``$(_math(:D))$(_tex(:exp))_{$(_math(:G))}(X): $(_math(:𝔤)) → T_{$(_tex(:exp))_{$(_math(:G))}(X)}$(_math(:G))``
maps a tangent vector of the Lie algebra to a tangent vector at the point ``$(_tex(:exp))_{$(_math(:G))}(X)``.
To turn this into a map ``$(_math(:𝔤)) → $(_math(:𝔤))``, that is representable as a matrix in a basis of ``$(_math(:𝔤))``,
we _left-trivialize_ it: analogous to [`diff_right_compose`](@ref) we “pull back” the resulting tangent vector
by multiplying with ``$(_tex(:exp))_{$(_math(:G))}(X)^{-1}`` from the left.
The resulting left-trivialized differential ``$(_math(:d))$(_tex(:exp))_{$(_math(:G))}(X): $(_math(:𝔤)) → $(_math(:𝔤))``
has the series representation

```math
$(_math(:d))$(_tex(:exp))_{$(_math(:G))}(X) = $(_tex(:sum))_{k ≥ 0} $(_tex(:frac, "(-$(_tex(:rm, "ad"))_X)^k", "(k+1)!")),
```

where ``$(_tex(:rm, "ad"))_X = [X, ⋅]`` denotes the adjoint of the [`LieAlgebra`](@ref), see [`lie_bracket`](@ref).
The Jacobian ``J`` is the matrix of this map with respect to the basis ``b``: its ``j``th column contains the
coordinates of ``$(_math(:d))$(_tex(:exp))_{$(_math(:G))}(X)[X_j]``, where ``X_j`` is the ``j``th basis vector of ``b``.
Since it only depends on ``X``, this Jacobian is independent of a base point, which is why no point is passed.

!!! note
In the robotics and state-estimation literature this left-trivialized differential is often called the
_right Jacobian_ ``J_r``, for example in [SolaDerayAtchuthan:2021](@cite) and [Chirikjian:2012](@cite).
It is related to the _left Jacobian_ ``J_ℓ`` by ``J_r(X) = J_ℓ(-X)``, which is the right-trivialized
differential ``$(_tex(:sum))_{k ≥ 0} $(_tex(:frac, "($(_tex(:rm, "ad"))_X)^k", "(k+1)!"))``.

@Affie Affie Jul 15, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kellertuer You helped me figure this out when we strugled with the left and right confusion in diff_*. This follows that explanation you wrote in the diff_* docstrings, so hopefully it clears it up for users and future me.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would prefer to not use

that is representable as a matrix in a basis of g

A linear map is always representable as a matrix, there is nothing to “turn”

nor

left-trivialize

at least not in doc strings, if so only somewhere in a footnote of the docs page.

In order for Future-Johannes to clear that up, present-Ronny will need some time to read the rest very carefully. All your left/right stuff always confuses me. In short: To get form some TgG back to the Lie group, you just have to “pull back” - or in practice multiply by inv(G,g)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

on another note, I would not repeat the whole docs / formula for dexp - that has its own implementation and you can refer to the corresponding doc string.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, the left/right stuff is very confusing, but definitely not mine, that is why I tried to write the comments to trace if the specific literature calls the same thing left or right. I think it is correct for LieGroups.jl to be fixed in one way (left-trivialized) and that is it, but time will tell.

or in practice multiply by inv(G,g)

Exactly, but we have 2 options, LieGroups.jl multiplies this "pull back" on the left side. We could have chosen the right side throughout LieGroups.jl as well.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could have chosen the right side throughout LieGroups.jl as well.

No. LieGroups.jl is left side land ;)

I personally do not like the term left-trivialised. If at all that is terminology-complification. Lets not use that term here.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I’m open to anything. Just tried to follow the sources, but I’m not a mathematician.
I can delete those lines. What do you call this “LieGroups.jl is left side land”.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

zou have a lot of left/right choices actually, I think 3 or 4. The approach in Manifolds.jl had those all in I think. Here we simplified things to mainly always choose the left one, since Hilgert & Neeb did as well.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A linear map is always representable as a matrix, there is nothing to “turn”

I think it's worth mentioning that this is only trivial in the finite-dimensional case 😉 .

We could have chosen the right side throughout LieGroups.jl as well.

No. LieGroups.jl is left side land ;)

We tried not sticking to the left-side convention in Manifolds.jl before and it's a nightmare to keep it consistent. Left-side it slightly more performance-friendly in some cases so I think it makes sense to go with it, even if the right side would be more consistent with what Manifolds.jl is doing outside of Lie groups.

Comment thread src/interface.jl
Comment on lines +900 to +904
To turn this into a map ``$(_math(:𝔤)) → $(_math(:𝔤))``, that is representable as a matrix in a basis of ``$(_math(:𝔤))``,
we _left-trivialize_ it: analogous to [`diff_right_compose`](@ref) we “pull back” the resulting tangent vector
by multiplying with ``$(_tex(:exp))_{$(_math(:G))}(X)^{-1}`` from the left.
The resulting left-trivialized differential ``$(_math(:d))$(_tex(:exp))_{$(_math(:G))}(X): $(_math(:𝔤)) → $(_math(:𝔤))``
has the series representation

@Affie Affie Jul 15, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
To turn this into a map ``$(_math(:𝔤)) $(_math(:𝔤))``, that is representable as a matrix in a basis of ``$(_math(:𝔤))``,
we _left-trivialize_ it: analogous to [`diff_right_compose`](@ref) we “pull backthe resulting tangent vector
by multiplying with ``$(_tex(:exp))_{$(_math(:G))}(X)^{-1}`` from the left.
The resulting left-trivialized differential ``$(_math(:d))$(_tex(:exp))_{$(_math(:G))}(X): $(_math(:𝔤)) $(_math(:𝔤))``
has the series representation
To express this as a map on the Lie algebra ``$(_math(:𝔤)) $(_math(:𝔤))``, which (in the finite-dimensional case) can be represented as a matrix in a basis of ``$(_math(:𝔤))``,
we push forward the output tangent vector from the tangent space back to the Lie algebra via the differential of the left translation by ``$(_tex(:exp))_{$(_math(:G))}(X)^{-1}``.
For a matrix Lie group, this action corresponds to multiplying the tangent vector from the left.
The resulting map ``$(_math(:d))$(_tex(:exp))_{$(_math(:G))}(X): $(_math(:𝔤)) $(_math(:𝔤))``
has the convergent series representation

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, that is fine with me. I would use pull back as the actual term personally and put “left translate” in quotes instead, but maybe that is just personal favour

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think i see the issue with left translate; translate is on the group and we are doing a push-forward from TₚG to 𝔤. Then pull back might be a bit misleading as well as pull-back is normally for differential forms, correct?
So, I've updated this part again, what do you think now?
I'm not a mathematician and suck with documentation, so feel free to change anything in this PR.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok.

Comment thread Project.toml
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request preview docs Add this label if you want to see a PR-preview of the documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants