-
Notifications
You must be signed in to change notification settings - Fork 56
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
apply_diff_group
gives wrong sign?
#669
Comments
This indeed looks like a bug. I will look for the best solution and see if there could be other similar issues. |
I've tried to figure out the math that led to the confusion here. Introduce these notations for the translations (where This induces the following relations between |
Yes, those relations were used to derive the original code which wasn't carefully enough updated later. |
After giving it some thought I think +R and -L actions should use |
My guess is that it is going to be hard to avoid differentiating Maybe it's an idea to introduce a function The solution would then just be (for apply_diff_group(G, a, X, p, conv) = translate_diff(G, p, a, inv_diff(G, a, X), switch_side(conv)) |
I see, |
I've made a bunch of tests for both The important functions are
translate_diff_id(G, χ, ξ, conv) = translate_diff(G, χ, identity_element(G), ξ, conv)
"""
The left translation ``T_L(g,ξ) = gξ``.
"""
move(G, χ, ξ, ::LeftSide) = translate_diff_id(G, χ, ξ, (LeftAction(), LeftSide()))
"""
The right translation ``T_R(g,ξ) = ξg``.
"""
move(G, χ, ξ, ::RightSide) = translate_diff_id(G, χ, ξ, (RightAction(), RightSide()))
"""
Test the differential of the inverse on a Lie group.
Denote this inverse by ``I(g) := g^{-1}``.
If the left and right transports are ``T_L(g,ξ) := gξ``
and ``T_R(g,ξ) := ξg`` respectively, then
```math
⟨DI(g), T_L(g,ξ)⟩ = -T_R(g^{-1}, ξ)
```
and
``` math
⟨DI(g), T_R(g,ξ)⟩ = -T_L(g^{-1}, ξ)
```
"""
test_inv_diff(G, χ, ξ, conv) = begin
χ_ = inv(G, χ)
@test isapprox(TangentSpace(G, χ_), inv_diff(G, χ, move(G, χ, ξ, conv)), -move(G, χ_, ξ, switch_side(conv)))
end
test_inv_diff_set(G, χ, ξ) = @testset "inv_diff" for side in [LeftSide(), RightSide()]
test_inv_diff(G, χ, ξ, side)
end
_get_side(::LeftAction) = RightSide()
_get_side(::RightAction) = LeftSide()
_transporter(G, χ, ξ, dir) = move(G, χ, ξ, _get_side(dir))
"""
This should hold for *any* group action ``A`` on any manifold.
If you define ``π_x(g) := A(g, x)`` for ``g ∈ G`` and ``x ∈ M``,
and define, for ``ξ in Alg(G)``,
``T_R(g, ξ) := ξg`` (the right translation),
and ``T_L(g, ξ) := gξ`` (the left translation), then we have the identity:
```math
⟨Dπ_{x}(g), T(g, ξ)⟩ = ⟨Dπ_{A(g,x)}(1), ξ⟩
```
where, for a *left* action, ``T`` is the *right* translation,
and for a *right* action, ``T`` is the *left* translation.
"""
test_apply_diff_group(A::AbstractGroupAction{TAD}, χ, ξ, p) where {TAD} = begin
G = base_group(A)
p_ = apply(A, χ, p)
v1 = apply_diff_group(A, χ, _transporter(G, χ, ξ, TAD()), p)
v2 = apply_diff_group(A, identity_element(G), ξ, p_)
@test isapprox(TangentSpace(G, p_), v1, v2)
end
test_apply_diff_group_set(G, χ, ξ, χ_) = begin
@testset "$dir, $side" for side in [LeftSide(), RightSide()], dir in [LeftAction(), RightAction()]
test_apply_diff_group(GroupOperationAction(G, (dir, side)), χ, ξ, χ_)
end
end Here is an example of how to use the tests on a particular group: dim = 4
G = SpecialOrthogonal(dim)
# G = TranslationGroup(dim)
rng = Random.default_rng()
χ = rand(rng, G)
ξ = rand(rng, TangentSpace(G, identity_element(G)))
test_inv_diff_set(G, χ, ξ)
χ_ = rand(rng, G)
test_apply_diff_group_set(G, χ, ξ, χ_)
# Doesn't work, as `apply_diff_group` is not implemented for that action, but should work if it were implemented:
# V = Euclidean(dim)
# p = rand(V)
# A = RotationAction(V, G, LeftAction())
# test_apply_diff_group(A, χ, ξ, p) |
Thank you, I will add it to our test set ❤️ |
* Inverse of differential, fix for apply_diff_group issue * news entry * `inverse_translate_diff` fix * Fixes * Another fix * test for issue #669 * improve coverage * more tests * some tests and figuring out * fix inv_diff * fix translate_diff on general unitary * remove some tests that don't apply to all actions * why fail there? * tests * add some tests * this was right * Giles is helpful * fix circle group inv_diff * add some more tests * add pullbacks for group inverse * test for translations * update news * fix tests * fix the other test * coverage false positives * update version
* BoundaryValueDiffEq v5 in docs * this can still be a part of 0.9.4
As far as I understand how
apply_diff_group
is supposed to work, when applied at the identity, it should sometimes reverse the sign, sometimes not (prior tov0.9.0
, this was not the case, which might explain why the error cropped up).Now run
test_apply_diff_group(G, rand(TangentSpace(G, identity_element(G))))
. It should printtrue
four times but doesn't.I think
apply_diff_group
should be updated in light of the new implementation around group actions. It's a pretty serious bug, in my opinion. 😢The text was updated successfully, but these errors were encountered: