sage.tensor: Canonicalize sym, antisym

[mkoeppe]

for #30229.

The new static method CompWithSym._canonicalize_sym_antisym brings sym and antisym into a canonical form as sorted tuples of sorted tuples, with trivial symmetries and antisymmetries dropped.

Using it also removes some code duplication.

CC: @egourgoulhon

Component: linear algebra

Author: Matthias Koeppe

Branch/Commit: 66009e7

Reviewer: Eric Gourgoulhon

Issue created by migration from https://trac.sagemath.org/ticket/34451

[mkoeppe]

Author: Matthias Koeppe

[mkoeppe]

Branch: u/mkoeppe/sage_tensor__canonicalize_sym__antisym

[mkoeppe]

New commits:

3a65d2d	CompWithSym._canonicalize_sym_antisym: Factor out from __init__
07ab991	src/sage/tensor: Allow _sym, _antisym attributes to be tuples
3619b4c	src/sage/tensor, src/sage/manifolds: Sort _sym, _antisym, store as tuples of tuples
970c6c8	src/sage/tensor, src/sage/manifolds: Sort _sym, _antisym, store as tuples of tuples (fixup)
9f15e00	src/sage/tensor: Allow _sym, _antisym attributes to be tuples (fixup)

[mkoeppe]

Commit: 9f15e00

[egourgoulhon]

comment:4

Thanks for introducing this and removing the code duplication!

It would useful to add INPUT and OUTPUT fields in the docstring of _canonicalize_sym_antisym.
Besides, I don't understand this change:

-                if len(isym) < 2:
-                    raise IndexError("at least two index positions must be " +
-                                     "provided to define a symmetry")
+                if len(isym) < 2:
+                    # Drop trivial symmetry
+                    continue

Shouldn't an error be raised if the input pretends to define a symmetry/antisymmetry with less than 2 index positions?

[mkoeppe]

comment:5

In this version of this code:

diff --git a/src/sage/tensor/modules/free_module_tensor.py b/src/sage/tensor/modules/free_module_tensor.py
index bbcc1ec..4d6d05c 100644
--- a/src/sage/tensor/modules/free_module_tensor.py
+++ b/src/sage/tensor/modules/free_module_tensor.py
@@ -309,41 +309,8 @@ class FreeModuleTensor(ModuleElementWithMutability):
                               # bases, with the bases as keys (initially empty)
 
         # Treatment of symmetry declarations:
-        self._sym = []
-        if sym is not None and sym != []:
-            if isinstance(sym[0], (int, Integer)):
-                # a single symmetry is provided as a tuple -> 1-item list:
-                sym = [tuple(sym)]
-            for isym in sym:
-                if len(isym) > 1:
-                    for i in isym:

... the trivial symmetries were dropped silently.

I suppose I can add a parameter trivial_symmetries='error' for the cases where you'd like to see the errors.

[sagetrac-git]

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

d1a92e5	Merge tag '9.7.rc0' into t/34451/sage_tensor__canonicalize_sym__antisym
319ecfa	FiniteRankFreeModule.tensor: Use CompWithSym._canonicalize_sym_antisym
8e93626	CompWithSym._canonicalize_sym_antisym: Add documentation
564be35	FiniteRankFreeModule.tensor: Restore error when trivial symmetries are passed
ffea896	CompWithSym._canonicalize_sym_antisym: Code optimization

[sagetrac-git]

Changed commit from 9f15e00 to ffea896

[sagetrac-git]

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

fed09d5

CompWithSym._canonicalize_sym_antisym: Sort earlier to validate indices faster

[sagetrac-git]

Changed commit from ffea896 to fed09d5

[sagetrac-git]

Changed commit from fed09d5 to 421c660

[sagetrac-git]

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

421c660

src/sage/tensor/modules/finite_rank_free_module.py: Add doctest

[egourgoulhon]

comment:9

Replying to @mkoeppe:

In this version of this code:

... the trivial symmetries were dropped silently.

True...

I suppose I can add a parameter trivial_symmetries='error' for the cases where you'd like to see the errors.

I am wondering about what should be meant by trivial symmetry, especially for an antisymmetry: if only a single index position is provided, is there any meaning in stating that the components are antisymmetric with respect to this position?

---

New commits:

421c660

src/sage/tensor/modules/finite_rank_free_module.py: Add doctest

[sagetrac-git]

Changed commit from 421c660 to a1a3f36

[sagetrac-git]

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

a1a3f36

CompWithSym.__init__: Accept keyword 'trivial_symmetries', add tests

[sagetrac-git]

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

8a2c71a

VectorFieldModule.tensor, VectorFieldFreeModule.tensor: Use CompWithSym._canonicalize_sym_antisym

[sagetrac-git]

Changed commit from a1a3f36 to 8a2c71a

[mkoeppe]

comment:12

Replying to @egourgoulhon:

I am wondering about what should be meant by trivial symmetry, especially for an antisymmetry: if only a single index position is provided, is there any meaning in stating that the components are antisymmetric with respect to this position?

One index (whether symmetry or antisymmetry) just means that there's a symmetry group of 1 element acting. This element is the identity

[mkoeppe]

comment:13

I ran into this, by the way, using dual_ext_power of order 1: It is passing antisym=range(1).

[mkoeppe]

comment:14

I've worked a bit more on the branch and have removed some more code duplication. Ready for another look

[egourgoulhon]

comment:15

Replying to @mkoeppe:

Replying to @egourgoulhon:

I am wondering about what should be meant by trivial symmetry, especially for an antisymmetry: if only a single index position is provided, is there any meaning in stating that the components are antisymmetric with respect to this position?

One index (whether symmetry or antisymmetry) just means that there's a symmetry group of 1 element acting. This element is the identity

Thanks for the explanation. I see now: since the signature of the identity is +1, this makes sense.
Then, I would vote for reverting to the original version, i.e. that without the extra argument trivial_symmetries: this adds an extra test and we might want a rather fast method here; moreover, I don't see any use case of trivial_symmetries='error'. Sorry for the noise... Anyway, I leave it to you...

[egourgoulhon]

comment:16

Replying to @mkoeppe:

I ran into this, by the way, using dual_ext_power of order 1: It is passing antisym=range(1).

Indeed!

[mkoeppe]

comment:17

OK, I'll revert it

[sagetrac-git]

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

e34306b	Revert "FiniteRankFreeModule.tensor: Restore error when trivial symmetries are passed"
c6b6b2e	src/sage/tensor, src/sage/manifolds: Remove parameter 'trivial_symmetries' again

[sagetrac-git]

Changed commit from 8a2c71a to c6b6b2e

[mkoeppe]

comment:19

Done

[egourgoulhon]

comment:20

Replying to @mkoeppe:

Done

Thanks!

[egourgoulhon]

Reviewer: Eric Gourgoulhon

[egourgoulhon]

comment:21

There is actually an error (sequel of an old piece of code) in _canonicalize_sym_antisym, which is revealed by the pyflakes plugin of the patchbot running on #30229 (unfortunately, the patchbot did not visit this ticket after the last commit): the name i in

                if isym[0] < 0 or isym[-1] > nb_indices - 1:
                    raise IndexError("invalid index position: " + str(i) +
                                     " not in [0," + str(nb_indices-1) + "]")

is undefined. Same issue a few lines lower.

[sagetrac-git]

Changed commit from c6b6b2e to bec2f65

[sagetrac-git]

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

bec2f65

CompWithSym._canonicalize_sym_antisym: Refactor, fix index validation, add tests

[sagetrac-git]

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

66009e7

src/sage/tensor/modules/comp.py: Fix docstring

[sagetrac-git]

Changed commit from bec2f65 to 66009e7

[egourgoulhon]

comment:25

Nice refactoring!

[mkoeppe]

comment:26

Thanks!

[vbraun]

Changed branch from u/mkoeppe/sage_tensor__canonicalize_sym__antisym to 66009e7