_add_ method for tensors with indices

[gh-LBrunswic]

One may want to add/contract tensors with indices in the following way : T["^ijklm_m]+T["^mklij_m"]

This entails to implement

• _get_item_ method
• _set_item_ method
• _add_ method

which all keep track of the indices names.

[git]

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

​8fc152a

not yet done

[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

[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

[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.

[git]

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

​3506466

Many error corrected. Added doctest

[git]

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

​2288249

Some PEP8 enforcement

[egourgoulhon]

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

[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.

[gh-LBrunswic]

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

[egourgoulhon]

Replying to gh-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.

[git]

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

​b34a712

Some docbuild error corrected

[git]

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

​0fafb78

Some docbuild error corrected

[git]

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

​831b050

Merge branch 'develop' into t/28787/_add__method_for_tensors_with_indices

[git]

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

​35032fa

corrected a plugin error

[git]

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

​a972fe1

corrected a plugin error

[chapoton]

Sometime the plugin is wrong:

-    EXAMPLES:
+    EXAMPLES::

was a wrong move, because this is not followed by doctests, but by more text..

[egourgoulhon]

I've performed some corrections to the docstrings, including that mentioned by Frédéric (comment:19). Now the documentation should build successfully. Note that some of the changes are mere deletions of white space characters on empty lines or at the end of lines (you should configure your editor to do that automatically when saving the file to disk).

The documentation of the new functionalities added by this ticket does not appear in the html output (i.e. in local/share/doc/sage/html/en/reference/tensor_free_modules/sage/tensor/modules/tensor_with_indices.html) because the doc of underscored methods, such as __add__, is not generated. You should therefore add relevant examples in the main docstring of TensorWithIndices.

---

New commits:

​8fcd437

Minor fixes in the documentation of tensors with indices (trac #28787)

[egourgoulhon]

PS: to see my changes, simply click on the blue 8fcd437 above.

[egourgoulhon]

Setting the ticket back to needs_review for the patchbot to run on it.

[gh-LBrunswic]

Thanks a lot, I spent an unreasonable amount of time yesterday trying to understand what was the problem.

I will consider putting some time into patchbot to see how one could improve on this. Trying to understand the issue I've already read some of the code of patchbot.

[gh-LBrunswic]

Replying to gh-LBrunswic:

Thanks a lot, I spent an unreasonable amount of time yesterday trying to understand what was the problem.

The worst part being that I was actually able to build sagemath with the doc without errors. Only patchbot was raising errors...

I will consider putting some time into patchbot to see how one could improve on this. Trying to understand the issue I've already read some of the code of patchbot.

[egourgoulhon]

Replying to gh-LBrunswic:

Replying to gh-LBrunswic:

Thanks a lot, I spent an unreasonable amount of time yesterday trying to understand what was the problem.

The worst part being that I was actually able to build sagemath with the doc without errors. Only patchbot was raising errors...

Actually, the patchbot reports are to be taken with a grain of salt. A human eye has always to take a look. Apart from the EXAMPLE: case pointed by Frédéric (maybe in that case the patchbot error was triggered by EXAMPLE: not being followed by an empty line), another case is some spurious report of unused import (see e.g. ​this example).

I will consider putting some time into patchbot to see how one could improve on this.

Thanks!

[git]

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

​5400bf7

updated doc examples illustrating __add__

[chapoton]

Doctest lines after TESTS:: should be indented, as for EXAMPLES::

+        TESTS::
+
+        sage: from sage.tensor.modules.tensor_with_indices import TensorWithIndices
+        sage: TensorWithIndices._parse_indices('([..])')  # nested symmetries
+        Traceback (most recent call last):

The patchbot plugins are not smart, but there is no simple way to make them smarter. When they give a red light, you should judge by yourself if there is an error or not.

[git]

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

​3e37ac0

indentation corrected

[gh-LBrunswic]

I'm not critisizing the work that has been put in the patchbot and I realize the work there is more than simply changing a few regex. On the contrary, I do appreciate the work that has been put into the review process of sagemath, which includes the patchbot. I feel that improving on the process (and the documentation) is improving sagemath on the long run.

[git]

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

​5e7817d

Doc corrected for __setitem__ and __getitem__

[git]

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

​a2f2383

Doc corrected (again) for __setitem__ and __getitem__

[git]

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

​4dd4a6d

PEP enforcement

[chapoton]

typo "Decompostion"

[git]

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

​1904d92

Typo correction

[git]

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

​aaf530c

#28787: small reviewer tweaks

[egourgoulhon]

I've just pushed a new commit, with very minor tweaks (mostly indentation in _parse_indices() docstring).

A last point: could you please document further the argument permutation of the method permute_indices(), in particular stating in which format it is expected, possibly providing more examples?

[gh-LBrunswic]

I then will add some overloading to accept a permutation given by an element of a symmetric group. This seems natural.

[egourgoulhon]

Replying to gh-LBrunswic:

I then will add some overloading to accept a permutation given by an element of a symmetric group. This seems natural.

I was not asking that much, but simply that you document permutation in the INPUT section by something like "tuple/list of index positions describing the permutation...". But I agree it is quite natural to pass an element of a symmetric group as well.

[gh-LBrunswic]

I struggled with adding more general permutation for several reasons :

• The word problem is only implemented for permutation group acting on [1,...,n] while we more commonly use 0-indexed notations. Therefore, one would need to translate a permutation acting on [0,...,n-1] into a permutation acting on [1,...,n]. While this is not difficult, it seems particularly unnatural : such conversion should be done in the Permutation group module.

• The overloading would depends on the overloading of PermutationGroupElement?

Therefore, I think it's better to postpone this supplementaty functionality.

[git]

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

​aa0b75d	permute_indices : Example add and more precise INPUT description
​cd655b1	Merge branch 'public/manifolds/add_tensor_with_indices-28787' of git://trac.sagemath.org/sage into t/28787/public/manifolds/add_tensor_with_indices-28787

[embray]

Ticket retargeted after milestone closed

[egourgoulhon]

May I ask about the status of this ticket? Is it again ready for review?

[gh-LBrunswic]

I haven't taken the time rework on it since the holydays (and the launch of sage9). I have to go through it again.

[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

[gh-LBrunswic]

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

[egourgoulhon]

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]

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

[git]

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

​e588289

Docstring correction

[git]

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

​fde9245

Docstring correction

[egourgoulhon]

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

Thanks for this nice enhancement!

[gh-LBrunswic]

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

[vbraun]

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

[git]

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

​a1b9630

python2 compatibility