Opened 10 months ago
Closed 10 months ago
#28784 closed enhancement (fixed)
Adding multiple symmetries and multiple contractions to tensors
Reported by:  ghLBrunswic  Owned by:  ghLBrunswic 

Priority:  major  Milestone:  sage9.0 
Component:  linear algebra  Keywords:  tensor, contraction, symmetries, manifolds 
Cc:  egourgoulhon  Merged in:  
Authors:  Léo Brunswic  Reviewers:  Eric Gourgoulhon 
Report Upstream:  N/A  Work issues:  
Branch:  3013354 (Commits)  Commit:  3013354903be7147e51c9dd7957428a281c801df 
Dependencies:  Stopgaps: 
Description (last modified by )
As of sage9.0.beta6, applying multiple contractions or multiple symmetries to tensors in indices notation raises a NotImplementedError?.
This tickets aims at removing this error by implementing the adequate behavior as well as adding a convention check on the index notation.
The index notation should allow :
* Multiple contraction
* Multiple symmetries
* indices denoted by a nonaccentuated latin caracter {a,...,z,A,...,Z} and wild card "."
* covariant indices first notation as well as contravariant indices first
* Latex notations '{' and '}'
* if indices do not begin by ^
nor _
then contravariant indices first is assumed
The index notation should not allow :
* Repeated indices of the same type
* indices denoted by any other caracter
* nested symmetries
* unbalanced parentheses/brackets
NB : Usual index notations allows greek indices but their implementation seems more difficult and is not the goal of the ticket.
Change History (22)
comment:1 Changed 10 months ago by
 Cc egourgoulhon added
 Component changed from PLEASE CHANGE to linear algebra
 Description modified (diff)
 Keywords tensor contraction symmetries manifolds added
 Type changed from PLEASE CHANGE to enhancement
comment:2 Changed 10 months ago by
 Branch set to u/ghLBrunswic/adding_multiple_symmetries_and_multiple_contractions_to_tensors
comment:3 Changed 10 months ago by
 Commit set to 1d1e4eaee5ee42864bed95a9ce10f38f2a7a95d3
 Owner changed from (none) to ghLBrunswic
comment:4 Changed 10 months ago by
 Description modified (diff)
comment:5 Changed 10 months ago by
 Commit changed from 1d1e4eaee5ee42864bed95a9ce10f38f2a7a95d3 to 59c86e3ed784136ff1b9026a6b89b5625ec14052
comment:6 Changed 10 months ago by
 Status changed from new to needs_review
comment:7 followup: ↓ 9 Changed 10 months ago by
 Status changed from needs_review to needs_work
Thanks for this useful enhancement!
The patchbot reports some failures:
sage t long src/sage/manifolds/differentiable/tensorfield.py # 2 doctests failed sage t long src/sage/tensor/modules/free_module_tensor.py # 19 doctests failed sage t long src/sage/tensor/modules/tensor_with_indices.py # 39 doctests failed
Besides, could you add a few doctests to illustrate the new functionalities?
comment:8 Changed 10 months ago by
 Commit changed from 59c86e3ed784136ff1b9026a6b89b5625ec14052 to 4b9a565daccfb8312b00efa9a86a9c47080ffe41
Branch pushed to git repo; I updated commit sha1. New commits:
4b9a565  The convention without leading '^' for contravariant indices is now accepted as is used to. A regex corrected. doctest added to illustrate new functionalities.

comment:9 in reply to: ↑ 7 Changed 10 months ago by
@egourgoulhon
I needed it :)
I'm sorry I forgot to do a doctest :$...I corrected the features that are tested and added new doctest.
comment:10 Changed 10 months ago by
 Description modified (diff)
 Status changed from needs_work to needs_review
comment:11 Changed 10 months ago by
 Description modified (diff)
comment:12 Changed 10 months ago by
 Branch changed from u/ghLBrunswic/adding_multiple_symmetries_and_multiple_contractions_to_tensors to public/manifolds/tensor_multiple_symmetries_contractions28784
 Commit changed from 4b9a565daccfb8312b00efa9a86a9c47080ffe41 to 0823e29c5fa02815c71a23f770f8ee226eec7c76
 Reviewers set to Eric Gourgoulhon
New commits:
0823e29  Minor corrections for #28784

comment:13 Changed 10 months ago by
Thanks for making all doctest passed. I gave a more indepth look at the code and it looks very good. I've just performed some reviewer tweaks, which I pushed in the new branch. You can see the (minor) changes here. In particular
 a doctest block has been corrected by adding a missing
::
 some PEP 8 enforcement has been performed
 doctests raising an exception have been rewritten via
Traceback...
instead of atry
block  the header
# * coding: utf8 *
has been added to the filetensor_with_indices.py
, in order to get rid of the nonascii error revealed by the patchbot.
Do you agree with the above changes?
comment:14 Changed 10 months ago by
 Commit changed from 0823e29c5fa02815c71a23f770f8ee226eec7c76 to 3013354903be7147e51c9dd7957428a281c801df
Branch pushed to git repo; I updated commit sha1. New commits:
3013354  Marginal modifications

comment:15 Changed 10 months ago by
Thanks for the PEP8 enforcement and the correction of the doctest.
Of course, I agree with the changes and merged. I quickly reviewed them and checked the doctest.
Also, I corrected a typo and clarified two lines.
comment:16 Changed 10 months ago by
I would give a positive review but since I modified the code (even marginally) I think I shouldn't do it.
comment:17 Changed 10 months ago by
 Status changed from needs_review to positive_review
I've just checked your latest changes. Everything is OK. Thanks for this work.
comment:18 followup: ↓ 19 Changed 10 months ago by
It would be nice to add this functionality for multiple symmetrizations to the method symmetrize()
as well because symmetrization is not necessarily done over neighboring indices which cannot be handled by the string syntax. I assume it is more efficient to do all symmetrizations and antisymmetrizations in a single function call, rather than computing several intermediate results.
comment:19 in reply to: ↑ 18 ; followup: ↓ 20 Changed 10 months ago by
Replying to ghmwageringel:
It would be nice to add this functionality for multiple symmetrizations to the method
symmetrize()
as well because symmetrization is not necessarily done over neighboring indices which cannot be handled by the string syntax. I assume it is more efficient to do all symmetrizations and antisymmetrizations in a single function call, rather than computing several intermediate results.
Hi!
I completely agree! We should open another ticket or a discussion of devel to discuss this (I would at the very least ask Eric Gourgoulhon for validation).
To begin the discussion, I see other issues with the current implementation and methods.
For instance, it's not possible to define the symmetries of the riemann tensor. This is related to the fact that (anti)symmetrize ony takes a set of indices as argument and then consider the action of the natural action of the symmetric group this set. It would be appreciable to define the symmetrization via an action of some group on the set indices. Antisymmetrization would need also a 'signature' morphism G>{1,1}. More generally, one could consider linear group action symmetries.
comment:20 in reply to: ↑ 19 Changed 10 months ago by
Replying to ghLBrunswic:
Replying to ghmwageringel:
It would be nice to add this functionality for multiple symmetrizations to the method
symmetrize()
as well because symmetrization is not necessarily done over neighboring indices which cannot be handled by the string syntax. I assume it is more efficient to do all symmetrizations and antisymmetrizations in a single function call, rather than computing several intermediate results.Hi!
I completely agree! We should open another ticket or a discussion of devel to discuss this (I would at the very least ask Eric Gourgoulhon for validation).
Yes please open a ticket for this! (and continue the discussion there).
To begin the discussion, I see other issues with the current implementation and methods.
For instance, it's not possible to define the symmetries of the riemann tensor. This is related to the fact that (anti)symmetrize ony takes a set of indices as argument and then consider the action of the natural action of the symmetric group this set. It would be appreciable to define the symmetrization via an action of some group on the set indices. Antisymmetrization would need also a 'signature' morphism G>{1,1}. More generally, one could consider linear group action symmetries.
Indeed, this would be nice.
comment:21 Changed 10 months ago by
The new ticket is #28813.
comment:22 Changed 10 months ago by
 Branch changed from public/manifolds/tensor_multiple_symmetries_contractions28784 to 3013354903be7147e51c9dd7957428a281c801df
 Resolution set to fixed
 Status changed from positive_review to closed
Branch pushed to git repo; I updated commit sha1. New commits:
Adding check to raise exception on repeated indices of same type. Implementation of multiple contractions