JuliaManifolds / Manifolds.jl

Manifolds.jl provides a library of manifolds aiming for an easy-to-use and fast implementation.
https://juliamanifolds.github.io/Manifolds.jl
MIT License
368 stars 53 forks source link

Make `switch_direction` consistent #639

Closed mateuszbaran closed 1 year ago

mateuszbaran commented 1 year ago

This PR implements a solution that will resolve #637 by introducing four different action directions instead of just two.

TODO:

codecov[bot] commented 1 year ago

Codecov Report

Merging #639 (c7f8c9c) into master (4b80c6e) will decrease coverage by 58.39%. The diff coverage is 90.44%.

@@             Coverage Diff             @@
##           master     #639       +/-   ##
===========================================
- Coverage   98.87%   40.49%   -58.39%     
===========================================
  Files         106      106               
  Lines       10206    10081      -125     
===========================================
- Hits        10091     4082     -6009     
- Misses        115     5999     +5884     
Impacted Files Coverage Δ
src/Manifolds.jl 66.66% <ø> (-20.00%) :arrow_down:
src/groups/circle_group.jl 83.58% <ø> (-16.42%) :arrow_down:
src/groups/group_operation_action.jl 84.44% <ø> (-15.56%) :arrow_down:
src/groups/multiplication_operation.jl 83.58% <ø> (-16.42%) :arrow_down:
src/groups/rotation_action.jl 77.17% <56.25%> (-22.83%) :arrow_down:
src/groups/translation_action.jl 84.37% <75.00%> (-15.63%) :arrow_down:
src/groups/group_action.jl 85.10% <81.81%> (-5.60%) :arrow_down:
src/groups/metric.jl 65.00% <87.50%> (-35.00%) :arrow_down:
src/groups/general_unitary_groups.jl 94.69% <92.85%> (-5.31%) :arrow_down:
ext/ManifoldsTestExt/tests_group.jl 92.87% <100.00%> (-2.14%) :arrow_down:
... and 8 more

... and 80 files with indirect coverage changes

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

mateuszbaran commented 1 year ago

I think this should be more or less ready. @olivierverdier , do you think this solves the issue? I was trying to make the default close to what we currently have but now you can express all four options and switch just the left/right or forward/backward part of the action. Rotation and translation actions are all forward now.

olivierverdier commented 1 year ago

It does solve the issue, thank you!