Opened 3 years ago

Closed 3 years ago

# Automorphism Fields: Notation of Inverse

Reported by: Owned by: gh-DeRhamSource major sage-9.1 geometry automorphisms egourgoulhon, tscrim Michael Jung Eric Gourgoulhon N/A 8403049 840304928e17abefa37418de9d702090850e06fd

At this stage we have the following problem with the notation of inverses:

```sage: M = Manifold(2, 'M')
sage: X.<x,y> = M.chart()
sage: aib = M.automorphism_field([[0, 2], [-1, 0]], name='a^(-1)*b')
sage: c = aib.inverse(); c
Field of tangent-space automorphisms a^(-1)*b^(-1) on the 2-dimensional differentiable manifold M
```

That is definitely misleading. In this ticket it gets fixed by adding brackets around the element before taking its inverse if the name could cause any confusion:

```sage: c = aib.inverse(); c
Field of tangent-space automorphisms (a^(-1)*b)^(-1) on the 2-dimensional differentiable manifold M
```

To solve this, the ticket provides a modification of `is_atomic` in `sage/tensor/modules/format_utilities.py`.

### comment:1 Changed 3 years ago by gh-DeRhamSource

• Branch set to u/gh-DeRhamSource/automorphism_inverse_notation

### comment:2 Changed 3 years ago by gh-DeRhamSource

• Commit set to 8b9ac9a24a29930ae84cac2e575ce821171bd4bb
• Status changed from new to needs_review

New commits:

 ​8b9ac9a `Trac #28973: Brackets around element before inverse`

### comment:3 Changed 3 years ago by gh-DeRhamSource

• Description modified (diff)

### comment:4 Changed 3 years ago by egourgoulhon

Thanks for taking care of this! However, for simple names like `a`, having `(a)^(-1)` for the inverse is not desirable. One should maintain `a^(-1)` in this case. To cover all cases, you should use something like `is_atomic()` (defined in `src/sage/tensor/modules/format_utilities.py`); see the handling of names in `FreeModuleAltForm.wedge()`. More precisely, you should define, in `format_utilities.py`, a new function similar to `is_atomic` and adapted to the inverse naming.

### comment:5 Changed 3 years ago by gh-DeRhamSource

You're right. That's definitely preferrable.

This was the fastest solution I had in mind so far. I see what I can do.

### comment:6 Changed 3 years ago by gh-DeRhamSource

• Status changed from needs_review to needs_work

### comment:7 Changed 3 years ago by gh-DeRhamSource

• Description modified (diff)
• Status changed from needs_work to needs_review

### comment:8 Changed 3 years ago by git

• Commit changed from 8b9ac9a24a29930ae84cac2e575ce821171bd4bb to 15067c152449793de4e1ca567313ed10b06de245

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

 ​15067c1 `Trac #28973: is_atomic adapted and used`

### comment:9 Changed 3 years ago by git

• Commit changed from 15067c152449793de4e1ca567313ed10b06de245 to c7fbb72cab8da51db5c461b0b51b2c686a232d75

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

 ​c7fbb72 `Trac #28973: Small improvement of is_atomic and Python3 compatibility in doctest`

### comment:10 Changed 3 years ago by git

• Commit changed from c7fbb72cab8da51db5c461b0b51b2c686a232d75 to 69ee5289141605e9aa6a1c3683d76b59efa50a7d

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

 ​69ee528 `Trac #28973: No raw string needed`

### comment:11 Changed 3 years ago by gh-DeRhamSource

• Status changed from needs_review to needs_work

### comment:12 Changed 3 years ago by git

• Commit changed from 69ee5289141605e9aa6a1c3683d76b59efa50a7d to 48d84fe66ffa84b4976faf6141813aa27f8aaf80

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

 ​48d84fe `Trac #28973: Automorphism fields adapted to new code`

### comment:13 Changed 3 years ago by gh-DeRhamSource

• Status changed from needs_work to needs_review

### comment:14 Changed 3 years ago by egourgoulhon

Thanks for the useful enhancement of the function `is_atomic()`.

The patchbot complains about the change introduced in the commit of comment:10

```+inside file:  b/src/sage/tensor/modules/format_utilities.py
+@@ -52,16 +55,31 @@ def is_atomic(expression):
++        sage: is_atomic("a<>b", sep=['<>'])
++        sage: is_atomic("(a<>b)", sep=['<>'])
+Python3 incompatible code inserted on 2 non-empty lines
```

Could you revert this change?

### comment:15 Changed 3 years ago by egourgoulhon

There is also a doctest error reported by one of the patchbots:

```File "src/sage/manifolds/differentiable/automorphismfield.py", line 951, in sage.manifolds.differentiable.automorphismfield.AutomorphismFieldParal
Failed example:
latex(inv)
Expected:
R
Got:
R^{-1}
**********************************************************************
1 of  12 in sage.manifolds.differentiable.automorphismfield.AutomorphismFieldParal
```

### comment:16 Changed 3 years ago by git

• Commit changed from 48d84fe66ffa84b4976faf6141813aa27f8aaf80 to db09dbc682b259e619d0019ce7c5ca56ccb01916

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

 ​dce291f `Trac #28973: Python3 compatibility and doctest` ​db09dbc `Trac #28973: Merge branch 'develop' into automorphism_inverse_notation`

Like this?

### comment:18 in reply to: ↑ 17 Changed 3 years ago by egourgoulhon

Like this?

As you can see from the latest patchbot report, `<>` still causes some trouble:

```-Python3 incompatible code inserted on 0 non-empty lines
+inside file:  b/src/sage/tensor/modules/format_utilities.py
+@@ -52,16 +55,31 @@ def is_atomic(expression):
++        sage: is_atomic(r"a<>b", sep=[r'<>'])
++        sage: is_atomic(r"(a<>b)", sep=[r'<>'])
+Python3 incompatible code inserted on 2 non-empty lines
```

Apparently, the operator `<>` has been removed in Python 3: it does not appear in the list https://docs.python.org/3/library/operator.html. So please remove this doctest.

### comment:19 Changed 3 years ago by gh-DeRhamSource

I see. Well, this is on the symbolic level, so no actual Python code is involved here. Even though another operator as example would be certainly more convenient.

### comment:20 Changed 3 years ago by git

• Commit changed from db09dbc682b259e619d0019ce7c5ca56ccb01916 to 840304928e17abefa37418de9d702090850e06fd

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

 ​8403049 `Trac #28973: Doctest improved`

### comment:21 Changed 3 years ago by gh-DeRhamSource

Perhaps this is a good example because the separator consists of more than one letter.

Last edited 3 years ago by gh-DeRhamSource (previous) (diff)

### comment:22 Changed 3 years ago by egourgoulhon

• Reviewers set to Eric Gourgoulhon
• Status changed from needs_review to positive_review

Thanks for the change. The patchbot is now full green ==> good to go.

### comment:23 Changed 3 years ago by gh-DeRhamSource

• Description modified (diff)

### comment:24 Changed 3 years ago by vbraun

• Branch changed from u/gh-DeRhamSource/automorphism_inverse_notation to 840304928e17abefa37418de9d702090850e06fd
• Resolution set to fixed
• Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.