-
Notifications
You must be signed in to change notification settings - Fork 2
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
Introduce the general linear group and the multiplication group operation. #12
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #12 +/- ##
========================================
Coverage ? 100.00%
========================================
Files ? 9
Lines ? 397
Branches ? 0
========================================
Hits ? 397
Misses ? 0
Partials ? 0 ☔ View full report in Codecov by Sentry. |
to run on main not master
there is now only text coverage missing, so I already mark this as “Ready-to-Review”. For testing the test suite has to be extended slightly, but besides that this PR is feature-complete. |
Good work 👍 . One idea to discuss: should |
Sounds reasonable. Do we want to stick to calling that “embed/project”? For me that is more of a change of a representer. We have the same And in practice it is also just a call to the correct So I am very much for introducing that here, but maybe we can have some better fitting names? Especially project does not fit so well for a change of a representer. |
Now this is really finished. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few comments
One more thing we might want to fix: G = GeneralLinearGroup(3)
@test_throws ManifoldDomainError is_point(G, randn(ComplexF64, 3, 3); error=:error) This used to work in Manifolds.jl but doesn't throw an error here. |
That might ne more something for Invertible matrices? |
I'm not really sure what goes wrong at the moment. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
🛣️🗺️
GeneralLinearGroup
check_point
,check_vector
,distance
,embed
,norm
(inherited fromInvertibleMatrices
)get_coordinates
,get_vector
in theLieAlgebraOrthonormalBasis
exp
,exp!
,log
,log!
(will be inherited from matrix group operationAbstractMultiplicationGroupOperation
MatrixMultiplicationGroupOperation
exp
,exp!
log
,log!
*
,/
,\
compose
inv
diff_inv
(formerlyinv_diff
)diff_conjugate
diff_right_compose
anddiff_left_compose
(formerlytranslate_diff
)identity_element
,identity_element!
lie_bracket
,lie_bracket!
inverse_translate_diff
vee
hat
get_vector
get_coordinates
rand
andrand!