Skip to content
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

Fix apply_diff_group for some actions #673

Merged
merged 29 commits into from
Nov 6, 2023
Merged
Changes from 1 commit
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
598d5a3
Inverse of differential, fix for apply_diff_group issue
mateuszbaran Oct 27, 2023
48eab50
Merge branch 'master' into mbaran/fix-apply-diff-goa
mateuszbaran Oct 27, 2023
f130856
news entry
mateuszbaran Oct 27, 2023
6a941f8
`inverse_translate_diff` fix
mateuszbaran Oct 27, 2023
8abdea7
Fixes
mateuszbaran Oct 27, 2023
cbb7537
Another fix
mateuszbaran Oct 27, 2023
5eadc7a
test for issue #669
mateuszbaran Oct 27, 2023
137a39d
improve coverage
mateuszbaran Oct 28, 2023
54ea696
more tests
mateuszbaran Oct 28, 2023
e729f03
Merge branch 'master' into mbaran/fix-apply-diff-goa
mateuszbaran Oct 28, 2023
46680df
some tests and figuring out
mateuszbaran Oct 28, 2023
cb05433
fix inv_diff
mateuszbaran Nov 3, 2023
d6e3381
fix translate_diff on general unitary
mateuszbaran Nov 3, 2023
8e4d1e0
remove some tests that don't apply to all actions
mateuszbaran Nov 4, 2023
4fcfd91
why fail there?
mateuszbaran Nov 4, 2023
7fff4cf
tests
mateuszbaran Nov 4, 2023
e934b63
add some tests
mateuszbaran Nov 4, 2023
f2b4a68
this was right
mateuszbaran Nov 4, 2023
66175b5
Giles is helpful
mateuszbaran Nov 4, 2023
cbd8de9
fix circle group inv_diff
mateuszbaran Nov 4, 2023
5ca782c
Merge branch 'master' into mbaran/fix-apply-diff-goa
mateuszbaran Nov 4, 2023
248e87d
add some more tests
mateuszbaran Nov 5, 2023
2a060c5
add pullbacks for group inverse
mateuszbaran Nov 5, 2023
f4120ff
test for translations
mateuszbaran Nov 5, 2023
2f323ac
update news
mateuszbaran Nov 5, 2023
193828c
fix tests
mateuszbaran Nov 5, 2023
ed9818e
fix the other test
mateuszbaran Nov 5, 2023
0ad3fad
coverage false positives
mateuszbaran Nov 5, 2023
6714cff
update version
mateuszbaran Nov 6, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Giles is helpful
mateuszbaran committed Nov 4, 2023

Verified

This commit was signed with the committer’s verified signature.
skrashevich Sergey Krashevich
commit 66175b5d49e100e3c2a95408d2d6ffcf920d705c
13 changes: 13 additions & 0 deletions docs/src/references.bib
Original file line number Diff line number Diff line change
@@ -338,6 +338,19 @@ @article{GaoSonAbsilStykel:2021
TITLE = {Riemannian Optimization on the Symplectic Stiefel Manifold},
JOURNAL = {SIAM Journal on Optimization}
}
@inproceedings{Giles:2008,
address = {Berlin, Heidelberg},
series = {Lecture {Notes} in {Computational} {Science} and {Engineering}},
title = {Collected {Matrix} {Derivative} {Results} for {Forward} and {Reverse} {Mode} {Algorithmic} {Differentiation}},
isbn = {978-3-540-68942-3},
doi = {10.1007/978-3-540-68942-3_4},
booktitle = {Advances in {Automatic} {Differentiation}},
publisher = {Springer},
author = {Giles, Mike B.},
editor = {Bischof, Christian H. and Bücker, H. Martin and Hovland, Paul and Naumann, Uwe and Utke, Jean},
year = {2008},
pages = {35--44},
}
@incollection{GuiguiMaignantTroouvePennec:2021,
DOI = {10.1007/978-3-030-80209-7_12},
YEAR = {2021},
10 changes: 6 additions & 4 deletions src/groups/multiplication_operation.jl
Original file line number Diff line number Diff line change
@@ -23,7 +23,7 @@
Base.:\(::Identity{MultiplicationOperation}, p) = p
Base.:\(e::Identity{MultiplicationOperation}, ::Identity{MultiplicationOperation}) = e

Base.inv(e::Identity{MultiplicationOperation}) = e

Check warning on line 26 in src/groups/multiplication_operation.jl

Codecov / codecov/patch

src/groups/multiplication_operation.jl#L26

Added line #L26 was not covered by tests

LinearAlgebra.det(::Identity{MultiplicationOperation}) = true
LinearAlgebra.adjoint(e::Identity{MultiplicationOperation}) = e
@@ -131,16 +131,18 @@
Compute the value of differential of matrix inversion ``p ↦ p^{-1}`` at ``X``.
When tangent vectors are represented in Lie algebra in a left-invariant way, the formula
reads ``-X``. For matrix groups with ambient space tangent vectors, the formula would
read ``-p^{-1}Xp^{-1}``.
reads ``-pXp^{-1}``. For matrix groups with ambient space tangent vectors, the formula would
read ``-p^{-1}Xp^{-1}``. See the section about matrix inverse in [Giles:2008](@cite).
"""
function inv_diff(::MultiplicationGroupTrait, G::AbstractDecoratorManifold, p, X)
return -X
return -(p * X * inv(G, p))

Check warning on line 138 in src/groups/multiplication_operation.jl

Codecov / codecov/patch

src/groups/multiplication_operation.jl#L137-L138

Added lines #L137 - L138 were not covered by tests
end
function inv_diff!(::MultiplicationGroupTrait, G::AbstractDecoratorManifold, Y, p, X)
copyto!(Y, X)
p_inv = inv(p)
Z = X * p_inv
mul!(Y, p, Z)
Y .*= -1
return Y

Check warning on line 145 in src/groups/multiplication_operation.jl

Codecov / codecov/patch

src/groups/multiplication_operation.jl#L140-L145

Added lines #L140 - L145 were not covered by tests
end

compose(::MultiplicationGroupTrait, G::AbstractDecoratorManifold, p, q) = p * q
13 changes: 10 additions & 3 deletions test/groups/group_operation_action.jl
Original file line number Diff line number Diff line change
@@ -158,9 +158,16 @@ using Manifolds:

Y = similar(X)
adjoint_apply_diff_group!(A_right_fwd, Y, a_pts[1], X, m)
@test Y inverse_translate_diff(G, m, a_pts[1], -X, RightBackwardAction())
@test Y inverse_translate_diff(
G,
m,
a_pts[1],
-a_pts[1] * X / a_pts[1],
RightBackwardAction(),
)

@test apply_diff_group(A_right_fwd, a_pts[1], X, m) -m \ X * m
@test apply_diff_group(A_left_back, a_pts[1], X, m) -X
@test apply_diff_group(A_right_fwd, a_pts[1], X, m)
-m \ (a_pts[1] * X / a_pts[1]) * m
@test apply_diff_group(A_left_back, a_pts[1], X, m) -a_pts[1] * X / a_pts[1]
end
end

Unchanged files with check annotations Beta

end
test_inv_diff && Test.@testset "Differential of inverse" begin # COV_EXCL_LINE
Test.@test isapprox(inv_diff(G, e, Xe_pts[1]), -Xe_pts[1]; atol=atol)
Test.@test isapprox(

Check warning on line 307 in ext/ManifoldsTestExt/tests_group.jl

Codecov / codecov/patch

ext/ManifoldsTestExt/tests_group.jl#L306-L307

Added lines #L306 - L307 were not covered by tests
inv_diff(G, inv(G, g_pts[1]), inv_diff(G, g_pts[1], X_pts[1])),
X_pts[1];
atol=atol,
Compute the value of differential of additive matrix inversion ``p ↦ -p`` at ``X``, i.e. ``-X``.
"""
function inv_diff(::AdditionGroupTrait, G::AbstractDecoratorManifold, p, X)
return -X

Check warning on line 64 in src/groups/addition_operation.jl

Codecov / codecov/patch

src/groups/addition_operation.jl#L63-L64

Added lines #L63 - L64 were not covered by tests
end
function inv_diff!(::AdditionGroupTrait, G::AbstractDecoratorManifold, Y, p, X)
Y .= X
Y .*= -1
return Y

Check warning on line 69 in src/groups/addition_operation.jl

Codecov / codecov/patch

src/groups/addition_operation.jl#L66-L69

Added lines #L66 - L69 were not covered by tests
end
function is_identity(::AdditionGroupTrait, G::AbstractDecoratorManifold, q; kwargs...)
X,
::RightForwardAction,
)
copyto!(G, Y, X)
return Y

Check warning on line 310 in src/groups/general_unitary_groups.jl

Codecov / codecov/patch

src/groups/general_unitary_groups.jl#L309-L310

Added lines #L309 - L310 were not covered by tests
end
function translate_diff!(
G::GeneralUnitaryMultiplicationGroup,
X,
::LeftBackwardAction,
)
copyto!(G, Y, p * X * inv(G, p))
return Y

Check warning on line 321 in src/groups/general_unitary_groups.jl

Codecov / codecov/patch

src/groups/general_unitary_groups.jl#L320-L321

Added lines #L320 - L321 were not covered by tests
end
function translate_diff!(
G::GeneralUnitaryMultiplicationGroup,
inv_diff(G::AbstractDecoratorManifold, p)
@trait_function inv_diff(G::AbstractDecoratorManifold, p, X)
function inv_diff(::TraitList{<:IsGroupManifold}, G::AbstractDecoratorManifold, p, X)
Y = allocate_result(G, inv_diff, X, p)
return inv_diff!(G, Y, p, X)

Check warning on line 543 in src/groups/group.jl

Codecov / codecov/patch

src/groups/group.jl#L541-L543

Added lines #L541 - L543 were not covered by tests
end
@trait_function inv_diff!(G::AbstractDecoratorManifold, Y, p, X)
function adjoint_apply_diff_group(A::GroupOperationAction, a, X, p)
G = base_group(A)
if direction_and_side(A) === LeftForwardAction() ||

Check warning on line 68 in src/groups/group_operation_action.jl

Codecov / codecov/patch

src/groups/group_operation_action.jl#L68

Added line #L68 was not covered by tests
direction_and_side(A) === RightBackwardAction()
return inverse_translate_diff(G, a, p, X, reverse_direction_and_side(A))

Check warning on line 70 in src/groups/group_operation_action.jl

Codecov / codecov/patch

src/groups/group_operation_action.jl#L70

Added line #L70 was not covered by tests
else
return inverse_translate_diff(

Check warning on line 72 in src/groups/group_operation_action.jl

Codecov / codecov/patch

src/groups/group_operation_action.jl#L72

Added line #L72 was not covered by tests
G,
p,
a,
function adjoint_apply_diff_group!(A::GroupOperationAction, Y, a, X, p)
G = base_group(A)
if direction_and_side(A) === LeftForwardAction() ||

Check warning on line 84 in src/groups/group_operation_action.jl

Codecov / codecov/patch

src/groups/group_operation_action.jl#L84

Added line #L84 was not covered by tests
direction_and_side(A) === RightBackwardAction()
return inverse_translate_diff!(G, Y, a, p, X, reverse_direction_and_side(A))

Check warning on line 86 in src/groups/group_operation_action.jl

Codecov / codecov/patch

src/groups/group_operation_action.jl#L86

Added line #L86 was not covered by tests
else
return inverse_translate_diff!(

Check warning on line 88 in src/groups/group_operation_action.jl

Codecov / codecov/patch

src/groups/group_operation_action.jl#L88

Added line #L88 was not covered by tests
G,
Y,
p,
"""
function apply_diff_group(A::GroupOperationAction, a, X, p)
G = base_group(A)
if direction_and_side(A) === LeftForwardAction() ||

Check warning on line 152 in src/groups/group_operation_action.jl

Codecov / codecov/patch

src/groups/group_operation_action.jl#L152

Added line #L152 was not covered by tests
direction_and_side(A) === RightBackwardAction()
return translate_diff(G, p, a, X, reverse_direction_and_side(A))

Check warning on line 154 in src/groups/group_operation_action.jl

Codecov / codecov/patch

src/groups/group_operation_action.jl#L154

Added line #L154 was not covered by tests
else
return translate_diff(

Check warning on line 156 in src/groups/group_operation_action.jl

Codecov / codecov/patch

src/groups/group_operation_action.jl#L156

Added line #L156 was not covered by tests
G,
p,
a,
end
function apply_diff_group!(A::GroupOperationAction, Y, a, X, p)
G = base_group(A)
if direction_and_side(A) === LeftForwardAction() ||

Check warning on line 167 in src/groups/group_operation_action.jl

Codecov / codecov/patch

src/groups/group_operation_action.jl#L167

Added line #L167 was not covered by tests
direction_and_side(A) === RightBackwardAction()
return translate_diff!(G, Y, p, a, X, reverse_direction_and_side(A))

Check warning on line 169 in src/groups/group_operation_action.jl

Codecov / codecov/patch

src/groups/group_operation_action.jl#L169

Added line #L169 was not covered by tests
else
return translate_diff!(

Check warning on line 171 in src/groups/group_operation_action.jl

Codecov / codecov/patch

src/groups/group_operation_action.jl#L171

Added line #L171 was not covered by tests
G,
Y,
p,
return q
end
function inv_diff!(G::PowerGroup, Y, p, X)
GM = G.manifold
rep_size = representation_size(GM.manifold)
for i in get_iterator(GM)
inv_diff!(

Check warning on line 135 in src/groups/power_group.jl

Codecov / codecov/patch

src/groups/power_group.jl#L131-L135

Added lines #L131 - L135 were not covered by tests
GM.manifold,
_write(GM, rep_size, Y, i),
_read(GM, rep_size, p, i),
_read(GM, rep_size, X, i),
)
end
return Y

Check warning on line 142 in src/groups/power_group.jl

Codecov / codecov/patch

src/groups/power_group.jl#L141-L142

Added lines #L141 - L142 were not covered by tests
end
function inv_diff!(G::PowerGroupNestedReplacing, Y, p, X)
GM = G.manifold
N = GM.manifold
rep_size = representation_size(N)
for i in get_iterator(GM)
Y[i...] = inv_diff(N, _read(GM, rep_size, p, i), _read(GM, rep_size, X, i))
end
return Y

Check warning on line 151 in src/groups/power_group.jl

Codecov / codecov/patch

src/groups/power_group.jl#L144-L151

Added lines #L144 - L151 were not covered by tests
end
# lower level methods are added instead of top level ones to not have to deal
end
inv!(::ProductGroup, q::Identity{ProductOperation}, ::Identity{ProductOperation}) = q
function inv_diff!(G::ProductGroup, Y, p, X)
M = G.manifold
map(

Check warning on line 107 in src/groups/product_group.jl

Codecov / codecov/patch

src/groups/product_group.jl#L105-L107

Added lines #L105 - L107 were not covered by tests
inv_diff!,
M.manifolds,
submanifold_components(G, Y),
submanifold_components(G, p),
submanifold_components(G, X),
)
return Y

Check warning on line 114 in src/groups/product_group.jl

Codecov / codecov/patch

src/groups/product_group.jl#L114

Added line #L114 was not covered by tests
end
_compose(G::ProductGroup, p, q) = _compose(G.manifold, p, q)
p,
X,
)
mul!(Y, a.x[2], X)
return Y

Check warning on line 190 in src/groups/rotation_translation_action.jl

Codecov / codecov/patch

src/groups/rotation_translation_action.jl#L189-L190

Added lines #L189 - L190 were not covered by tests
end
function apply_diff!(
::RotationTranslationActionOnVector{LeftAction},