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

Euclidean should support `local_metric` in `DefaultOrthonormalBasis` #742

Open r0uv3n opened 5 days ago

r0uv3n commented 5 days ago

Currently, local_metric only works for manifolds of type Euclidean when using an InducedBasis. For consistency I think it might be nice to have this return the same thing for the DefaultOrthonormalBasis, which seems to be used by most internal methods by default, e.g. solve_exp_ode.

kellertuer commented 5 days ago

sounds reasonable – especially since for Euclidean this is very easy (you even could consider just using DiffEq per se then). Do you feel you could do an PR for this as well (or in your PR you plan to do)?

If you are stuck somewhere we can help for sure (also with getting started with PRs and such).

mateuszbaran commented 4 days ago

Yes, support for local_metric is somewhat incomplete at the moment. It should definitely also work with DefaultOrthonormalBasis. Note though that DefaultOrthonormalBasis is in general not designed to work well with local_metric on all manifolds. Its main purpose is "I have a point, and I want to quickly transform tangent vectors into their coordinates and the other way around". As a result, not much attention is paid to the choice being continuous/differentiable/smooth between different points. For metric-related functions some level of smoothness is generally desirable and InducedBasis was introduced as a way to (locally) guarantee it.

r0uv3n commented 3 days ago

Okay I'll leave this to you