_add_ method for tensors with indices

[LBrunswic]

One may want to add/contract tensors with indices in the following way :

T["<sup>ijklm_m]+T["</sup>mklij_m"]

This entails to implement

• _get_item_ method
• _set_item_ method
• _add_ method
  which all keep track of the indices names.

CC: @egourgoulhon

Component: linear algebra

Keywords: tensor, contraction, symmetries, tensor with indices

Author: Léo Brunswic

Branch/Commit: a1b9630

Reviewer: Eric Gourgoulhon, Travis Scrimshaw

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

[LBrunswic]

Branch: u/gh-LBrunswic/_add__method_for_tensors_with_indices

[LBrunswic]

Commit: 232ae1b

[sagetrac-git]

Changed commit from 232ae1b to 8fc152a

[sagetrac-git]

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

8fc152a

not yet done

[sagetrac-git]

Changed commit from 8fc152a to 30b92a8

[sagetrac-git]

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

1d1e4ea	Implementation of multiple symmetries for tensor with indices as well as convention checks
59c86e3	Adding check to raise exception on repeated indices of same type. Implementation of multiple contractions
4b9a565	The convention without leading '^' for contravariant indices is now accepted as is used to. A regex corrected. doctest added to illustrate new functionalities.
0823e29	Minor corrections for #28784
3013354	Marginal modifications
30b92a8	Merge branch 't/28784/public/manifolds/tensor_multiple_symmetries_contractions-28784' into t/28787/_add__method_for_tensors_with_indices

[sagetrac-git]

Changed commit from 30b92a8 to 6f9c651

[sagetrac-git]

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

e659567	__add__ method completed, doctest added, basic `__getitem__` method added, `__sub__` method added
6f9c651	Correction of the `__add__` method to have a more natural indices convention

[sagetrac-git]

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

8e29f77

_parse_indices staticmethod added ; new method permute_indices added ; `_getitem_` and `_set_item_` added ; code factorisation of `__init__` and `__add__` using _parse_indices and permute_indices.

[sagetrac-git]

Changed commit from 6f9c651 to 8e29f77

[sagetrac-git]

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

3506466

Many error corrected. Added doctest

[sagetrac-git]

Changed commit from 8e29f77 to 3506466

[sagetrac-git]

Changed commit from 3506466 to 2288249

[sagetrac-git]

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

2288249

Some PEP8 enforcement

[egourgoulhon]

comment:10

Thanks for providing this functionality! The code looks nice.
However, as revealed by the patchbot, there are a few minor issues:

• running sage -coverage src/sage/tensor/modules/tensor_with_indices.py reveals incomplete coverage
• SymmetricGroup is imported but not used
• there are invalid escape sequences in the lines

            if re.search("[()\[\]]",con) is not None:
                raise ValueError("No symmetry allowed")
            if re.search("[()\[\]]",cov) is not None:
                raise ValueError("No symmetry allowed")

Most probably, you should prefix the string "[()\[\]]" with r to make it a raw string. Side remark: the error message passed to exceptions should start with a lower case letter; so please change "No symmetry allowed" to "no symmetry allowed"

• Building the documentation via sage -docbuild reference/tensor_free_modules html yields some errors, two of them being related to the characters \] and \[ above and should be fixed with the r prefix

[sagetrac-git]

Changed commit from 2288249 to e44197b

[sagetrac-git]

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

e44197b

raw prefix 'r' added before regex. Unused import deleted. full doc Coverage.docbuild errors corrected.

[LBrunswic]

comment:12

Thx for the review. I'm sorry, I'm still familiarizing myself with the tools involved in the review process.

[egourgoulhon]

comment:13

Replying to @LBrunswic:

Thx for the review. I'm sorry, I'm still familiarizing myself with the tools involved in the review process.

No problem. I fully understand this of course. To get access to the patchbot reports, click on the button with the Sage version in the top right of the ticket description panel.

[sagetrac-git]

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

b34a712

Some docbuild error corrected

[sagetrac-git]

Changed commit from e44197b to b34a712

[sagetrac-git]

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

0fafb78

Some docbuild error corrected

[sagetrac-git]

Changed commit from b34a712 to 0fafb78

[sagetrac-git]

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

4340fc8

Merge branch 'develop' into t/28787/public/manifolds/add_tensor_with_indices-28787

[sagetrac-git]

Changed commit from cd655b1 to 4340fc8

[LBrunswic]

comment:47

It seems okay to me, I thought I had asked for review after my previous commit.

[egourgoulhon]

comment:49

It looks good to me, except that the patchbot "blocks" plugin reports this error.
This plugin forbids "Returns" at the start of a new line, see rule no. 6 in line 581 of the plugin source code. This is because, according to PEP 257, "The docstring is a phrase ending in a period. It prescribes the function or method's effect as a command ("Do this", "Return that"), not as a description; e.g. don't write "Returns the pathname ...". So the doctstring of the method _parse_indices should start with "Parse", not "Parses", etc. Could you fix this please?

[tscrim]

comment:50

This is likely a doc problem:

         - ``permutation`` -- permutation that has to be applied to the indices
-            the input should be a ``list`` containing the second line of the permutation
-            in Cauchy notation.
+          the input should be a ``list`` containing the second line of the permutation
+          in Cauchy notation

[sagetrac-git]

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

e588289

Docstring correction

[sagetrac-git]

Changed commit from 4340fc8 to e588289

[sagetrac-git]

Changed commit from e588289 to fde9245

[sagetrac-git]

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

fde9245

Docstring correction

[egourgoulhon]

comment:53

Thanks for the changes. The (relevant) patchbot in now all green.

Thanks for this nice enhancement!

[egourgoulhon]

Reviewer: Eric Gourgoulhon, Travis Scrimshaw

[LBrunswic]

comment:54

Thanks! I understand the importance of the constraints on the docstrings but it's plainful to get blocked for a whitespace.

[vbraun]

comment:55

On Python 2:

[dochtml]   File "/var/lib/buildbot/slave/sage2_git/build/local/lib/python2.7/site-packages/sage/tensor/modules/tensor_with_indices.py", line 941
[dochtml]     [swap(*param, self._tensor.tensor_rank()) for param in swap_params],
[dochtml] SyntaxError: only named arguments may follow *expression

[sagetrac-git]

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

a1b9630

python2 compatibility

[sagetrac-git]

Changed commit from fde9245 to a1b9630

[vbraun]

Changed branch from public/manifolds/add_tensor_with_indices-28787 to a1b9630