Opened 2 years ago

Closed 2 years ago

# Apply a function to all components of a tensor field

Reported by: Owned by: egourgoulhon major sage-9.1 geometry manifolds, tensor, components, sd107 tscrim Eric Gourgoulhon Vincent Delecroix, Travis Scrimshaw N/A 5cf7fc5 5cf7fc53f523cb3a1a7c8c75df8b2f0774203df1

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/basic-work-on-tensor-components/ and was discussed again during Sage Days 107.

### comment:1 Changed 2 years ago by egourgoulhon

• Branch set to public/manifolds/map_tensor_components-29244
• Commit set to 14937a88e33fee39ec4a6873ee55808b2165ea17

New commits:

 ​14937a8 `Add method map_components to class TensorField`

### comment:2 Changed 2 years ago by egourgoulhon

• Status changed from new to needs_review

### comment:3 follow-up: ↓ 4 Changed 2 years ago by vdelecroix

• 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 ; follow-up: ↓ 5 Changed 2 years ago by egourgoulhon

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 ; follow-up: ↓ 6 Changed 2 years ago by 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?

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 2 years ago by 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?

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 2 years ago by tscrim

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 follow-up: ↓ 9 Changed 2 years ago by 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?

### comment:9 in reply to: ↑ 8 Changed 2 years ago by vdelecroix

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 2 years ago by gh-mwageringel

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 follow-up: ↓ 12 Changed 2 years ago by gh-PatrickMassot

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 non-destructive like factor, but substituting a parameter in some frame but not the other is clearly evil.

### comment:12 in reply to: ↑ 11 Changed 2 years ago by egourgoulhon

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 non-destructive 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 `non-destructive=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 change-of-frame 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 2 years ago by git

• 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 2 years ago by egourgoulhon

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 2 years ago by egourgoulhon

• Status changed from needs_info to needs_review

### comment:16 Changed 2 years ago by egourgoulhon

• Description modified (diff)

### comment:17 Changed 2 years ago by egourgoulhon

Anybody to perform the final review? If we want this in 9.1, this could be the good time...

### comment:18 Changed 2 years ago by tscrim

• 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 2 years ago by egourgoulhon

Thank you Travis!

### comment:20 Changed 2 years ago by vbraun

• Branch changed from public/manifolds/map_tensor_components-29244 to 5cf7fc53f523cb3a1a7c8c75df8b2f0774203df1
• Resolution set to fixed
• Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.