Opened 20 months ago
Closed 20 months ago
#29244 closed enhancement (fixed)
Apply a function to all components of a tensor field
Reported by:  egourgoulhon  Owned by:  

Priority:  major  Milestone:  sage9.1 
Component:  geometry  Keywords:  manifolds, tensor, components, sd107 
Cc:  tscrim  Merged in:  
Authors:  Eric Gourgoulhon  Reviewers:  Vincent Delecroix, Travis Scrimshaw 
Report Upstream:  N/A  Work issues:  
Branch:  5cf7fc5 (Commits, GitHub, GitLab)  Commit:  5cf7fc53f523cb3a1a7c8c75df8b2f0774203df1 
Dependencies:  Stopgaps: 
Description (last modified by )
This ticket adds the method apply_map
to the class TensorField
, which applies a function to the coordinate expressions of all components of a tensor field in a given vector frame.
This allows operations like factorization, expansion, simplification or substitution to be performed on the components.
Such a method has been requested in e.g. https://ask.sagemath.org/question/42107/basicworkontensorcomponents/ and was discussed again during Sage Days 107.
Change History (20)
comment:1 Changed 20 months ago by
 Branch set to public/manifolds/map_tensor_components29244
 Commit set to 14937a88e33fee39ec4a6873ee55808b2165ea17
comment:2 Changed 20 months ago by
 Cc tscrim added
 Status changed from new to needs_review
comment:3 followup: ↓ 4 Changed 20 months ago by
 Status changed from needs_review to needs_info
For vectors and matrices, such method already exists and is called apply_map
sage: v = vector(range(4)) sage: v.apply_map(lambda x: x^2) (0, 1, 4, 9) sage: m = matrix(3,range(9)) sage: m.apply_map(lambda x: x^2) [ 0 1 4] [ 9 16 25] [36 49 64]
Is the difference in terminology intended?
comment:4 in reply to: ↑ 3 ; followup: ↓ 5 Changed 20 months ago by
Replying to vdelecroix:
For vectors and matrices, such method already exists and is called
apply_map
Is the difference in terminology intended?
No, not at all. Thanks for pointing out this.
In the context of tensor fields, apply_map
may sound a little bit too vague (for instance, it may evoke a pull back operation associated with a map between manifolds). Would apply_map_components
be more appropriate?
comment:5 in reply to: ↑ 4 ; followup: ↓ 6 Changed 20 months ago by
Replying to egourgoulhon:
Replying to vdelecroix:
For vectors and matrices, such method already exists and is called
apply_map
Is the difference in terminology intended?
No, not at all. Thanks for pointing out this. In the context of tensor fields,
apply_map
may sound a little bit too vague (for instance, it may evoke a pull back operation associated with a map between manifolds). Wouldapply_map_components
be more appropriate?
The vagueness also applies to vectors/matrices where map can also refer to a morphism between vector spaces. I am not a big fan of the terminology but to my mind, consistency is the best option for the Sage interface as a whole. This consistency didcatorship includes as an option changing apply_map
to something better.
comment:6 in reply to: ↑ 5 Changed 20 months ago by
Replying to vdelecroix:
Replying to egourgoulhon:
Replying to vdelecroix:
For vectors and matrices, such method already exists and is called
apply_map
Is the difference in terminology intended?
No, not at all. Thanks for pointing out this. In the context of tensor fields,
apply_map
may sound a little bit too vague (for instance, it may evoke a pull back operation associated with a map between manifolds). Wouldapply_map_components
be more appropriate?The vagueness also applies to vectors/matrices where map can also refer to a morphism between vector spaces. I am not a big fan of the terminology but to my mind, consistency is the best option for the Sage interface as a whole. This consistency didcatorship includes as an option changing
apply_map
to something better.
apply_map
is redundant. I would just have called it map because of the already existing Python function
sage: list(map(lambda x: x^2, range(4))) [0, 1, 4, 9]
comment:7 Changed 20 months ago by
I am 1 on calling the method map
as I would first think it should return a map
either from or based on the object or a map form of the object (such as for a matrix).
comment:8 followup: ↓ 9 Changed 20 months ago by
Thank you Vincent and Travis for the discussion. I am pretty sensitive to the consistency argument put forward in comment:5, so I would lean toward apply_map
. Do we all agree?
comment:9 in reply to: ↑ 8 Changed 20 months ago by
Replying to egourgoulhon:
Thank you Vincent and Travis for the discussion. I am pretty sensitive to the consistency argument put forward in comment:5, so I would lean toward
apply_map
. Do we all agree?
Dear Eric, that is of course ok for me!
comment:10 Changed 20 months ago by
I just want to mention that matrices also have a method map_coefficients
which is very similar, coming from modules with basis. It is supposed to work with an endofunction on the coefficient ring, however it seems to be broken for matrices:
sage: matrix.identity(2).map_coefficients(lambda x: x+1) ... KeyError: 1
As apply_map
does not require an endofunction, it has a more general notion, though.
comment:11 followup: ↓ 12 Changed 20 months ago by
I'm the one who asked for this function, so I'm very happy to see this patch. But I'm uneasy about the way it can be used to introduce inconsistencies between frames. Was it already easy to abuse SageManifolds? like this before?
At least I would phrase that as a warning in the docstrings, instead of making it sound like a feature. I mean, it can be a feature where the function being mapped is nondestructive like factor, but substituting a parameter in some frame but not the other is clearly evil.
comment:12 in reply to: ↑ 11 Changed 20 months ago by
Replying to ghPatrickMassot:
I'm the one who asked for this function, so I'm very happy to see this patch. But I'm uneasy about the way it can be used to introduce inconsistencies between frames. Was it already easy to abuse SageManifolds? like this before?
At least I would phrase that as a warning in the docstrings, instead of making it sound like a feature. I mean, it can be a feature where the function being mapped is nondestructive like factor, but substituting a parameter in some frame but not the other is clearly evil.
That's a good point. It should be easy to add a keyword argument such as keep_other_components=False
or nondestructive=False
to enforce the consistency between the sets of components in various frames. The components in frames different from the requested one will then be deleted. When requested, they can be recomputed via changeofframe formulas. However, this can be time consuming and, in some cases, may generate a loss of information due to some lack of simplifications.
comment:13 Changed 20 months ago by
 Commit changed from 14937a88e33fee39ec4a6873ee55808b2165ea17 to 5cf7fc53f523cb3a1a7c8c75df8b2f0774203df1
Branch pushed to git repo; I updated commit sha1. New commits:
5cf7fc5  #29244: rename map_components to apply_map and add keyword argument keep_other_components

comment:14 Changed 20 months ago by
In the above commit, the method has been renamed apply_map
(cf. comment:9) and the consistency issue pointed out in comment:11 has been delt with by adding the keyword argument keep_other_components
. Its default value (False
) enforces the consistency among the components in various vector frames.
comment:15 Changed 20 months ago by
 Status changed from needs_info to needs_review
comment:16 Changed 20 months ago by
 Description modified (diff)
comment:17 Changed 20 months ago by
Anybody to perform the final review? If we want this in 9.1, this could be the good time...
comment:18 Changed 20 months ago by
 Reviewers set to Vincent Delecroix, Travis Scrimshaw
 Status changed from needs_review to positive_review
I will give this the green light. I added Vincent as a reviewer considering the name change was based off his suggestion. If anyone else feels like they want to be a reviewer, please add your name.
comment:19 Changed 20 months ago by
Thank you Travis!
comment:20 Changed 20 months ago by
 Branch changed from public/manifolds/map_tensor_components29244 to 5cf7fc53f523cb3a1a7c8c75df8b2f0774203df1
 Resolution set to fixed
 Status changed from positive_review to closed
New commits:
Add method map_components to class TensorField