-
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
Manifolds.jl v0.10 #732
Manifolds.jl v0.10 #732
Conversation
what remains to be fixed is if it is wrapped in a `MetricManifold`, or a `ConnectionManifold`, for instance. In that case, the wrong `translate_diff` methods are called.
A vanilla group has no adjoint_action defined
`translate_diff` is now always computed from adjoint_action instead
adjoint action was previously not available since not all translate_diff methods were implemented
implement for LeftAction direction, the other direction is automatic
this ensures that all group methods are now defined
This is the `exp` and `log` associated to any of the Cartan–Schouten connections. It uses the left-invariant storage of tangent vectors and the specific exp_lie/log_lie implementations.
These implementations are the one from *group product*, so they are not invariant with respect to the semidirect product. One could put them back in the product_group layer instead. The proper way to invoke them is then `exp(base_manifold(G), ...)` instead of `exp(G, ...)`. Until this is fixed, `exp` now uses more allocations, even when calling it with `base_manifold`.
The failing tests come from using matrices instead of `ArrayPartition`. Using the `ArrayPartition` type (the one returned by `identity_element`) works normally.
The invariant log fails when points are too far apart.
more adjoint action methods for CircleGroup
- at Identity - remove some unused adjoint_action! implementations
…ds.jl into mbaran/lightfolds
I think this is more or less ready for review now. I don't see any more breaking changes worth making. |
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.
Here are a few comments, mainly on the tutorial. Am I right that this is based on another PR this would close as well then?
The first part of the tutorial is copied from #355 with minor editing so we can close that PR after this is merged. |
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.
(Approved of course besides a bump in the version that is missing ;) )
List of changes:
translate_diff
andinv_diff
for all groups (#679) #683 into here and finish it.With these weak dependencies, load time of Manifolds.jl is reduced to 0.72 s on my PC. To go lower we'd have to turn both Graphs.jl and StaticArrays.jl into weak dependencies but that seems to be too much work for too little gain, and basically everything else either has marginal impact on load time or is indispensable.