Opened 7 months ago

Closed 4 months ago

#31609 closed enhancement (fixed)

Add method tangent_vector to differentiable manifolds

Reported by: egourgoulhon Owned by:
Priority: major Milestone: sage-9.4
Component: manifolds Keywords: tangent vector
Cc: slelievre Merged in:
Authors: Eric Gourgoulhon Reviewers: Michael Jung
Report Upstream: N/A Work issues:
Branch: b5d7a8f (Commits, GitHub, GitLab) Commit: b5d7a8f800fb496de28a5e89cabac2250b124c6f
Dependencies: Stopgaps:

Status badges

Description (last modified by egourgoulhon)

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:

sage: E.<x,y> = EuclideanSpace()
sage: p = E((1, 2), name='p')
sage: Tp = E.tangent_space(p)   # step 1
sage: v = Tp((-1, 3)); v        # step 2
Vector at Point p on the Euclidean plane E^2

This ticket endows the class DifferentiableManifold with a method tangent_vector, with vector as an alias, to make it a 1-step process. We can now write:

sage: E.<x,y> = EuclideanSpace()
sage: p = E((1, 2), name='p')
sage: v = E.vector(p, -1, 3); v
Vector at Point p on the Euclidean plane E^2

This feature is motivated by this ask.sagemath question.

Change History (20)

comment:1 Changed 7 months ago by egourgoulhon

  • Branch set to public/manifolds/tangent_vector-31609
  • Cc slelievre added
  • Commit set to 5b225961f67c6a01b238e657549cf6a9e7e11d05
  • Status changed from new to needs_review

New commits:

5b22596Add method vector() to DifferentiableManifold

comment:2 Changed 7 months ago by gh-mjungmath

Sweet! I have just one comment. Since Python 3.6, we have f-strings which are way more convenient and easier to read than format (and also faster). I'd propose to use f-strings more often in the future. For this ticket:

- raise ValueError("{} components must be provided".format(dim))
+ raise ValueError(f"{dim} components must be provided")
Last edited 7 months ago by gh-mjungmath (previous) (diff)

comment:3 follow-up: Changed 7 months ago by gh-mjungmath

Here is an explanation also elaborating on more advantages of f-strings over format: https://realpython.com/python-f-strings/.

Last edited 7 months ago by gh-mjungmath (previous) (diff)

comment:4 Changed 7 months ago by git

  • Commit changed from 5b225961f67c6a01b238e657549cf6a9e7e11d05 to c4a76e92ff90733bf484265c1d0421152db252c8

Branch pushed to git repo; I updated commit sha1. New commits:

c4a76e9Use f-string in DifferentialManifold.vector

comment:5 in reply to: ↑ 3 Changed 7 months ago by egourgoulhon

Replying to gh-mjungmath:

Here is an explanation also elaborating on more advantages of f-strings over format: https://realpython.com/python-f-strings/.

Thanks for the tip! The change to f-string is performed in the last commit.

comment:6 follow-up: Changed 7 months ago by gh-mjungmath

Is there a reason why you chose vector over tangent_vector for the method's name? The latter would sound more intuitive to me.

comment:7 in reply to: ↑ 6 Changed 7 months ago by egourgoulhon

Replying to gh-mjungmath:

Is there a reason why you chose vector over tangent_vector for the method's name? The latter would sound more intuitive to me.

vector is more adapted to elementary use, like in vector calculus in the Euclidean space, when the user might not know what "tangent space" means. Of course, we can make an alias for tangent_vector if you feel it necessary.

comment:8 follow-up: Changed 7 months ago by gh-mjungmath

Mh, I don't know. Manifolds usually don't constitute of vectors. So a vector method wouldn't make much sense there as it sounds to me more like a method that should be reserved for vector spaces only. I'd advocate to rename vector to tangent_vector because mathematically precise.

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.

Last edited 7 months ago by gh-mjungmath (previous) (diff)

comment:9 follow-up: Changed 7 months ago by gh-mjungmath

Alternatively the Euclidean space, as a special case, can be endowed with an alias vector whereas general (differentiable) manifolds only supposed to have tangent_vector. I think that is the best solution to maintain preciseness and having that alias in the elementary case at the same time.

Last edited 7 months ago by gh-mjungmath (previous) (diff)

comment:10 Changed 7 months ago by git

  • Commit changed from c4a76e92ff90733bf484265c1d0421152db252c8 to b5d7a8f800fb496de28a5e89cabac2250b124c6f

Branch pushed to git repo; I updated commit sha1. New commits:

b5d7a8fRename vector to tangent_vector and make vector an alias to it

comment:11 in reply to: ↑ 8 ; follow-up: Changed 7 months ago by egourgoulhon

  • Description modified (diff)
  • Summary changed from Add method vector to differentiable manifolds to Add method tangent_vector to differentiable manifolds

Replying to gh-mjungmath:

Mh, I don't know. Manifolds usually don't constitute of vectors. So a vector method wouldn't make much sense there as it sounds to me more like a method that should be reserved for vector spaces only. I'd advocate to rename vector to tangent_vector because mathematically precise.

OK, this is done in the last commit. Note however that saying simply "vector at point p of manifold M" is quite unambiguous and is in line with the terminology "vector field": we do not say "tangent vector field", do we?

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.

Done. vector is now an alias for tangent_vector.

comment:12 in reply to: ↑ 9 Changed 7 months ago by egourgoulhon

Replying to gh-mjungmath:

Alternatively the Euclidean space, as a special case, can be endowed with an alias vector whereas general (differentiable) manifolds only supposed to have tangent_vector. I think that is the best solution to maintain preciseness and having that alias in the elementary case at the same time.

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 follow-up: Changed 7 months ago by slelievre

How about requiring

v = E.vector(p, (-1, 3))

instead of

v = E.vector(p, -1, 3)

I don't mind either way but there's a choice to make.

comment:14 in reply to: ↑ 13 Changed 7 months ago by egourgoulhon

Replying to slelievre:

How about requiring

v = E.vector(p, (-1, 3))

instead of

v = E.vector(p, -1, 3)

I don't mind either way but there's a choice to make.

There is no choice to make ;-) Both work in the current implementation (cf. the examples in the doctests).

Last edited 7 months ago by egourgoulhon (previous) (diff)

comment:15 in reply to: ↑ 11 Changed 6 months ago by gh-mjungmath

Replying to egourgoulhon:

OK, this is done in the last commit. Note however that saying simply "vector at point p of manifold M" is quite unambiguous and is in line with the terminology "vector field": we do not say "tangent vector field", do we?

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 Changed 6 months ago by gh-mjungmath

  • Status changed from needs_review to positive_review

LGTM

comment:17 Changed 6 months ago by gh-mjungmath

  • Reviewers set to Michael Jung

comment:18 Changed 6 months ago by egourgoulhon

Thanks for the review and suggestions!

comment:19 Changed 6 months ago by mkoeppe

  • Milestone changed from sage-9.3 to sage-9.4

Sage development has entered the release candidate phase for 9.3. Setting a new milestone for this ticket based on a cursory review.

comment:20 Changed 4 months ago by vbraun

  • Branch changed from public/manifolds/tangent_vector-31609 to b5d7a8f800fb496de28a5e89cabac2250b124c6f
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.