-
-
Notifications
You must be signed in to change notification settings - Fork 505
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
Add method tangent_vector to differentiable manifolds #31609
Comments
Commit: |
New commits:
|
comment:2
Sweet! I have just one comment. Since Python 3.6, we have f-strings which are way more convenient and easier to read than - raise ValueError("{} components must be provided".format(dim))
+ raise ValueError(f"{dim} components must be provided") |
comment:3
Here is an explanation also elaborating on more advantages of f-strings over |
Branch pushed to git repo; I updated commit sha1. New commits:
|
comment:5
Replying to @mjungmath:
Thanks for the tip! The change to f-string is performed in the last commit. |
comment:6
Is there a reason why you chose |
comment:7
Replying to @mjungmath:
|
comment:8
Mh, I don't know. Manifolds usually don't constitute of vectors. So a This is a neat feature though, and I don't want to block it just because of my pedantry. An alias sounds like a good compromise. |
comment:9
Alternatively the Euclidean space, as a special case, can be endowed with an alias |
Branch pushed to git repo; I updated commit sha1. New commits:
|
This comment has been minimized.
This comment has been minimized.
comment:11
Replying to @mjungmath:
OK, this is done in the last commit.
Done. |
comment:12
Replying to @mjungmath:
I oppose to this: we shall not introduce on Euclidean spaces method names that break for more general manifolds, while the functionality is exactly the same. |
comment:13
How about requiring
instead of
I don't mind either way but there's a choice to make. |
comment:14
Replying to @slel:
There is no choice to make ;-) Both work in the current implementation (cf. the examples in the doctests). |
comment:15
Replying to @egourgoulhon:
No, we don't say "tangent vector field". But I haven't heard or read the use of "vector" over "tangent vector" either. Historical burden I suppose. It should be unambiguous though and having both is fine with me. Someone who seeks for (tangent) vectors will find them now. |
comment:16
LGTM |
Reviewer: Michael Jung |
comment:18
Thanks for the review and suggestions! |
comment:19
Sage development has entered the release candidate phase for 9.3. Setting a new milestone for this ticket based on a cursory review. |
Changed branch from public/manifolds/tangent_vector-31609 to |
Currently, constructing a tangent vector v at some point p of a manifold M requires two steps: first construct the tangent space at p, TpM, and then construct v as an element of TpM. For instance:
This ticket endows the class
DifferentiableManifold
with a methodtangent_vector
, withvector
as an alias, to make it a 1-step process. We can now write:This feature is motivated by this ask.sagemath question.
CC: @slel
Component: manifolds
Keywords: tangent vector
Author: Eric Gourgoulhon
Branch/Commit:
b5d7a8f
Reviewer: Michael Jung
Issue created by migration from https://trac.sagemath.org/ticket/31609
The text was updated successfully, but these errors were encountered: