Opened 4 years ago
Closed 3 years ago
#15916 closed enhancement (fixed)
Tensors on free modules of finite rank
Reported by:  egourgoulhon  Owned by:  

Priority:  major  Milestone:  sage6.5 
Component:  linear algebra  Keywords:  free module, tensor, tensor product, days64 
Cc:  SimonKing  Merged in:  
Authors:  Eric Gourgoulhon, Michal Bejger  Reviewers:  Travis Scrimshaw 
Report Upstream:  N/A  Work issues:  
Branch:  99f9ac5 (Commits)  Commit:  99f9ac53bd3e42dd00f5d05b7b126a632161ca55 
Dependencies:  Stopgaps: 
Description (last modified by )
Description
This ticket implements:
 tensor products of the type M\otimes ...\otimes M \otimes M* \otimes...\otimes M* where M is a free module of finite rank over a commutative ring R and M* is its dual (k factors of M and l factors of M*, say)
 the elements of the above tensor products, considered as tensors of type (k,l) on M, i.e. multilinear forms (M*)^{k} \times M^{l} > R, thanks to the canonical isomorphism (M*)* = M (which holds since M is a free module of finite rank)
 the following tensor operations:
 operations inherent to the module structure (addition, multiplication by a ring element)
 tensor product of two tensors
 tensor contraction
 symmetry / antisymmetry handling (on subset of the tensor arguments or on all arguments)
 exterior product of alternating forms
 morphisms of free modules of finite rank, with coercion of endomorphisms to/from type(1,1) tensors
No distinguished basis is assumed on the free module M; on the contrary many bases can be introduced. Each tensor has then various representations, via its components in the various bases.
Motivation and context
The ticket has been motivated by tensors on smooth manifolds over R, within the SageManifolds project. In this context, tensors on free modules appear at two levels:
 tensors on tangent spaces:
 commutative ring R: real field R
 free module M: tangent vector space at a given manifold's point
 tensor fields on a manifold:
 commutative ring R: the algebra C^{oo}(U) of smooth functions U> R, where U is a parallelizable open set of the manifold
 free module M: the set X(U) of smooth vector fields on U (since U is parallelizable, this is a free module; its rank is the manifold's dimension)
Documentation
Apart from the numerous doctests in the code, some pieces of documentation are
 the tutorial worksheet posted here (a pdf version is here)
 the "tensors on free modules" reference manual; it can also be generated via the command
sage docbuild reference/tensor_free_modules html
See also this page.
Remarks
 Although developed in the context of SageManifolds (ticket:14865), the ticket is selfcontained and does not depend on other parts of SageManifolds. It this respect, it can be viewed as some attempt to include a first subset of SageManifolds in Sage, with a moderate size: the ticket comprises 14499 lines of Python code (most of them being doctests), while at present (version 0.6) SageManifolds contains 35157 lines of code.
 The ticket follows Sage's Parent/Element pattern and the (new) category framework. In particular, the ticket's free module class (FiniteRankFreeModule) passes the module TestSuite.
 It turned out to be necessary to develop a new class to implement free modules of finite rank. Indeed, the category of free modules does not exist yet in Sage: only those of generic modules (
Modules
) or free modules with a distinguished basis (ModulesWithBasis
) are available. Now, the tangent space at a given point of a manifold is a vector space without any distinguished basis (in other words, while the tangent space is isomorphic to R^{n}, there is no canonical isomorphism, each isomorphism relying on the choice of some coordinate chart). The new class, FiniteRankFreeModule, does not rely on any distinguished basis. It inherits directly from Parent, with the category set toModules()
. In particular, it does not inherit from sage.modules.module.FreeModule_generic since the latter seems to assume a distinguished basis (cf. its method basis()).
Change History (66)
comment:1 Changed 4 years ago by
comment:2 Changed 4 years ago by
 Description modified (diff)
comment:3 Changed 4 years ago by
 Commit changed from f8e9c3e62c1b016a41d377a0ccdbcd665aa189dc to e34f29ec69a4595f52894724a5aec22a571c02ce
comment:4 Changed 4 years ago by
Is there any connection and/or overlap between this ticket and #15726?
comment:5 Changed 4 years ago by
This ticket implements tensor modules T^{(k,l)}(M) of a fixed type (k,l) over a free module M of finite rank, while #15726 implements the tensor algebra T(M), which is the direct sum of all T^{(k,l)}(M).
Another major difference is that here the base module M is a generic free module, while in #15726 M is a free module with a distinguished basis (the classes TensorModule and TensorAlgebra of #15726 inherit from class CombinatorialFreeModule, which assumes a distinguished basis). In the present ticket, an arbitrary number of bases can be introduced on the free module M, along with the changeofbasis automorphisms, and each tensor has various sets of components, one per basis.
Another difference may regard symmetry/antisymmetry handling: the tensors implemented in this ticket can have arbitrary symmetries and/or antisymmetries. The symmetries are taken into account to reduce the storage of tensor components and to optimize some computations, such as the tensor product. But maybe something similar is implemented in #15726.
comment:6 Changed 3 years ago by
 Milestone changed from sage6.2 to sage6.3
comment:7 Changed 3 years ago by
 Commit changed from e34f29ec69a4595f52894724a5aec22a571c02ce to 8b7bd605a0b41f3e428c6e67bb85d630d193065a
Branch pushed to git repo; I updated commit sha1. New commits:
8b7bd60  Updated 'tensors on free modules' to Sage 6.2

comment:8 Changed 3 years ago by
 Commit changed from 8b7bd605a0b41f3e428c6e67bb85d630d193065a to 010c31309df79e4957bb7e8daca6cd47d22b2121
Branch pushed to git repo; I updated commit sha1. New commits:
010c313  Minor improvements; coincides with SageManifolds as of 18 May 2014 (commit d488f8f807 on github)

comment:9 Changed 3 years ago by
 Commit changed from 010c31309df79e4957bb7e8daca6cd47d22b2121 to 267b6b937226357d87669f7c5c8c699d99188b5a
Branch pushed to git repo; I updated commit sha1. New commits:
267b6b9  Update to SageManifolds v0.5

comment:10 Changed 3 years ago by
This is now part of SageManifolds v0.5 (see this post on sagedevel and http://sagemanifolds.obspm.fr/changelog.html).
comment:11 Changed 3 years ago by
 Status changed from new to needs_review
comment:12 Changed 3 years ago by
 Description modified (diff)
comment:13 Changed 3 years ago by
 Description modified (diff)
comment:14 Changed 3 years ago by
 Description modified (diff)
comment:15 Changed 3 years ago by
 Cc SimonKing added
comment:16 Changed 3 years ago by
 Milestone changed from sage6.3 to sage6.4
comment:17 Changed 3 years ago by
 Commit changed from 267b6b937226357d87669f7c5c8c699d99188b5a to 8f0878d7c04e15559a76c427fac9a764fa1ecca8
Branch pushed to git repo; I updated commit sha1. New commits:
8f0878d  Added index notation for tensor contractions and symmetrizations

comment:18 Changed 3 years ago by
 Commit changed from 8f0878d7c04e15559a76c427fac9a764fa1ecca8 to d8f518ff48c8be2ea73f2b51e067af9e6c87b4bf
Branch pushed to git repo; I updated commit sha1. New commits:
d8f518f  Update to SageManifolds v0.6

comment:19 Changed 3 years ago by
The ticket has been updated to coincide with the pure algebraic part of SageManifolds v0.6. Changes are: 1/ The possibility to use index notations for denoting tensor contractions or symmetrizations: the indices have to be passed in LaTeX notations (possibly without {}) as a string inside the square bracket operator, e.g.
sage: S = A['^{ab}_{cd}']*B['^d_a'] # equivalent to S = A.contract(0, 3, B, 1, 0)
to denote the tensor S^{b}_{c} = A^{ab}_{cd} B^{d}_{a} ,
sage: S = A['^{(ab)}_{cd}'] # equivalent to S = A.symmetrize(0,1)
to denote the tensor S^{ab}_{cd} = A^{(ab)}_{cd}, and
sage: S = A['^{ab}_{[cd]}'] # equivalent to S = A.antisymmetrize(2,3)
to denote the tensor S^{ab}_{cd} = A^{ab}_{[cd]}. See this page and this one for more details.
2/ The code for tensor contractions has been completely rewritten; it is more efficient and allows now for multiple contractions (as in the first example above).
3/ The argument of methods symmetrize()
and antisymmetrize()
in the FreeModuleTensor
class is now directly a sequence of index positions (and no longer a single list/tuple encapsulating such a sequence).
Besides, new examples have been provided on http://sagemanifolds.obspm.fr/examples.html; two of them are directly relevant to this ticket:
 tensors on free modules of finite rank (tutorial)
 vectorfield modules on the 2sphere (a concrete example of use)
comment:20 followup: ↓ 21 Changed 3 years ago by
Doctest coverage: sage coverage src/sage/tensor/
I don't think the string notation is very user friendly, A['^{ab}_{cd}']*B['^d_a']
looks more like line noise. How about recognizing symbolic variables and the builtin function any
as special:
sage: var('a, d') sage: A[a, any, any, d] * B[d, a]
Or, perhaps better in the long run: Define your own index class that knows about whether indices are upper or lower. Then we can also check that you don't contract two upper indices, and/or automatically insert the metric there:
sage: a, b, c, d = A.indices() sage: a Upper index sage: A[a, b, c, d] * B[d, a]
comment:21 in reply to: ↑ 20 Changed 3 years ago by
Replying to vbraun:
Doctest coverage:
sage coverage src/sage/tensor/
OK I see: most private methods appear to have no doctest because their doctests have been put in the docstring of the class itself: this is the only way for them to appear in the generated html documentation. This is particularly true for the __init__
methods (there are actually a lot of doctests for them). Shall the doctests be repeated in the docstring of the private methods ? What is the general policy for this ?
I don't think the string notation is very user friendly,
A['^{ab}_{cd}']*B['^d_a']
looks more like line noise. How about recognizing symbolic variables and the builtin functionany
as special:sage: var('a, d') sage: A[a, any, any, d] * B[d, a]
Your suggestion for any
is quite compeling, thanks. Note that, in the present version, it is already possible to replace unrelevant indices with dots, which decreases the noise:
sage: S = A['^{a.}_{.d}']*B['^d_a']
Or, perhaps better in the long run: Define your own index class that knows about whether indices are upper or lower. Then we can also check that you don't contract two upper indices, and/or automatically insert the metric there:
sage: a, b, c, d = A.indices() sage: a Upper index sage: A[a, b, c, d] * B[d, a]
You are right: in the long run, we should probably introduce an indice class to get rid of the strings. The only advantage of the present notation is that we do not have to spoil some variable names to denote indices: they can be set on the fly, to denote an operation like contraction where index notation is more clear, most operations being performed in an indexfree setting.
comment:22 Changed 3 years ago by
The __init__
docstring is used as the class docstring if you don't define a separate class docstring, and I would recommend using that.
You can document other underscore methods by adding, say,
.. automethod:: __mul__
at the end of the class docstring (possibly derived from the __init__
docstring as above).
Not all underscore methods need to appear in the reference manual, though. The doctests are still tests, even if they are not enduser visible "documentation". And they are documentation for developers reading the source and wondering how to use that method...
comment:23 followup: ↓ 24 Changed 3 years ago by
What I generally do is put all of the important info in the class docstring, like the INPUT
block and examples showing usecases. In the __init__
method, it is just creating a common example X
and doing TestSuite(X).run()
(and possibly repeating for some boarder cases).
Also what Volker is alluding to is that all methods, including underscore ones, must have some doctest.
Some quick comments/questions:
 Please remove the autogenerated files from git tracking (as they will be autogenerated :p).
 The imports from
tensor/modules
I feel like should be done usingtensor/all.py
rather thanall.py
(at the source root).  I'm wondering if the
tensor/modules
doc should be built along with thetensor
doc (rather than it's own separate thing) astensor
is small to begin with. However, I don't think there's any drawbacks to having it be separate since it's (essentially) disjoint from the other doc.  Is there anything I can do with or you need from #15726? I'd like to make these compatible as much as possible.
 +1 to having a class for indices (once there is clear a need for it).
I really like the pictures on the Sage manifolds website and think this will be a great addition to Sage. Unfortunately I don't have time right now to do much of a review.
Best,
Travis
PS  You might also be interested in #9439.
comment:24 in reply to: ↑ 23 Changed 3 years ago by
Replying to tscrim:
What I generally do is put all of the important info in the class docstring, like the
INPUT
block and examples showing usecases. In the__init__
method, it is just creating a common exampleX
and doingTestSuite(X).run()
(and possibly repeating for some boarder cases).
Thanks to you and Volker for your advices.
Also what Volker is alluding to is that all methods, including underscore ones, must have some doctest.
OK got it. Will modify the code accordingly.
Some quick comments/questions:
 Please remove the autogenerated files from git tracking (as they will be autogenerated :p).
OK.
 The imports from
tensor/modules
I feel like should be done usingtensor/all.py
rather thanall.py
(at the source root).
Yes, this would be more consistent with the directory structure.
 I'm wondering if the
tensor/modules
doc should be built along with thetensor
doc (rather than it's own separate thing) astensor
is small to begin with. However, I don't think there's any drawbacks to having it be separate since it's (essentially) disjoint from the other doc.
Actually the current content of tensor
(algebra of differential forms defined on a coordinate patch of R^{n}) pertains more to smooth manifolds than to tensor modules over arbitrary commutative rings.
 Is there anything I can do with or you need from #15726? I'd like to make these compatible as much as possible.
Sure! There is clearly some overlap between the two and we should discuss this further. For instance, once a basis is chosen, a tensor from this ticket should be converted into an element of the tensor algebra of #15726. Let us keep in touch about this.
 +1 to having a class for indices (once there is clear a need for it).
I really like the pictures on the Sage manifolds website and think this will be a great addition to Sage. Unfortunately I don't have time right now to do much of a review.
Best,
TravisPS  You might also be interested in #9439.
Yes this has clearly some connections with SageManifolds, as well as #10132 (now integrated into Sage) and your ticket about Lie algebras (#14901).
comment:25 Changed 3 years ago by
 Commit changed from d8f518ff48c8be2ea73f2b51e067af9e6c87b4bf to 45aaa975d406ade603a15a34f34a9790936e42ab
Branch pushed to git repo; I updated commit sha1. New commits:
45aaa97  Add doctests and improve documentation in tensor/modules

comment:26 Changed 3 years ago by
Hi,
 Following Volker's comment, doctests to all underscore methods have been added. Accordingly
sage coverage src/sage/tensor/modules
results in 100% coverage.  Following Travis' one, (i) I've suppressed the spurious mention "autogenerated file" in the rst files (this resulted from some cutandpaste; actually these files are not autogenerated) and (ii) I've moved the import of tensor/modules to
sage/tensor/all.py
.
Some simple examples illustrating this ticket are on this page. Concrete examples of use in the framework of fields on a smooth manifold (modules over the algebra of scalar fields on S^{2}) are here (this requires #14865).
comment:27 Changed 3 years ago by
 Description modified (diff)
comment:28 followup: ↓ 29 Changed 3 years ago by
You shouldn't need those .rst
because there are such files autogenerated by Sage's docbuilder (although they are a little more buried in the /src/doc/en/reference/tensor/sage/
, or whatever documentation group you're considering) and you're not doing anything different as far as I can see.
Also on a quick glance through the recent commit, I noticed a colon missing on in these two places:
@@ 984,6 +1179,21 @@ class Components(SageObject):  True if ``self`` is different from ``other``, or False otherwise + EXAMPLES: + + sage: from sage.tensor.modules.comp import Components @@ 176,12 +177,46 @@ class TensorFreeModule(FiniteRankFreeModule): [snip] Element = FreeModuleTensor def __init__(self, fmodule, tensor_type, name=None, latex_name=None): + r""" + TEST: + + sage: from sage.tensor.modules.tensor_free_module import TensorFreeModule
comment:29 in reply to: ↑ 28 Changed 3 years ago by
Replying to tscrim:
Thanks for your comments.
You shouldn't need those
.rst
because there are such files autogenerated by Sage's docbuilder (although they are a little more buried in the/src/doc/en/reference/tensor/sage/
, or whatever documentation group you're considering) and you're not doing anything different as far as I can see.
Actually, I've added these .rst files because they are not autogenerated in the present case. More precisely, the command sage docbuild tensors_free_module html
does not produce any sage
subdirectory in src/doc/en/tensors_free_module
. On the contrary the command sage docbuild reference html
produces a sage
subdirectory in src/doc/en/reference/tensor_free_modules
, which contains the autogenerated .rst files. As a result, if I simply copy the file index.rst
from src/doc/en/reference/tensor_free_modules
to src/doc/en/tensors_free_module
and delete all the other .rst files (the "nonautogenerated" ones), I get the error message toctree contains reference to nonexisting document u'sage/tensor/modules/finite_rank_free_module'
. Certainly, I am not doing something properly here. Any help on this would be appreciated...
/ Also on a quick glance through the recent commit, I noticed a colon missing on in these two places:
Thanks for pointing this; I'll correct it.
comment:30 followup: ↓ 31 Changed 3 years ago by
You should only have the modules/index.rst
. The docbuilder will not always process incremental changes correctly, you might have to sage b && make docclean && make doc
Edit: added the "not"
comment:31 in reply to: ↑ 30 Changed 3 years ago by
Replying to vbraun:
You should only have the
modules/index.rst
. The docbuilder will not always process incremental changes correctly, you might have tosage b && make docclean && make doc
I tried but it did not helped. To make things clear, let me stress that the problem arises because we wanted to have a separate reference manual for tensors on free modules, in a directory that we created for this purpose: src/doc/en/tensors_free_module
, which differs from the corresponding section in the main Sage reference manual (directory src/doc/en/reference/tensor_free_modules
). For the latter, everything works well: the only .rst file under src/doc/en/reference/tensor_free_modules
is index.rst
and it suffices for the docbuilder to produce the complete documentation. The problem arises only for src/doc/en/tensors_free_module
. So the question might be: "how to produce a separate reference manual for some subpart of Sage library ?".
comment:32 followup: ↓ 33 Changed 3 years ago by
You can't and you shouldn't be able to. The toplevel sections correspond to the src/sage subdirectories, as they should. You can, however, be quite fancy in src/doc/en/reference/modules/index.rst
, see e.g. the combinat section.
comment:33 in reply to: ↑ 32 Changed 3 years ago by
Replying to vbraun:
You can't and you shouldn't be able to. The toplevel sections correspond to the src/sage subdirectories, as they should. You can, however, be quite fancy in
src/doc/en/reference/modules/index.rst
, see e.g. the combinat section.
That's not true, there are significantly more toplevel docs than toplevel sections in src/sage
, the first one that comes to mind is finite_fields
. I'm taking a look at it right now to see what's going on.
comment:34 followup: ↓ 35 Changed 3 years ago by
 Branch changed from u/egourgoulhon/tensor_modules to public/tensor_modules15916
 Commit changed from 45aaa975d406ade603a15a34f34a9790936e42ab to a07a21ae1170e224192f9fe6bae34825dae23e1d
I think the problem is you put things in src/doc/en/tensors_free_module
, as opposed to src/doc/en/reference/tensors_free_module
. So with this branch you should be able to run Volker's commands (or sage docbuild all html
or (what I did to check) sage docbuild reference/tensor_free_modules inventory
and sage docbuild reference/tensor_free_modules html
).
New commits:
93dcad2  Merge branch 'u/egourgoulhon/tensor_modules' of trac.sagemath.org:sage into public/tensor_modules15916

a07a21a  Fixing the documentation.

comment:35 in reply to: ↑ 34 Changed 3 years ago by
Replying to tscrim:
It works! Thank you very much Travis. This yields exactly what I had in mind. Moreover, it is nice that one can get rid of the directory src/doc/en/tensors_free_module
and stay only with src/doc/en/reference/tensor_free_modules
, for duplication is root of all evil ;)
comment:36 Changed 3 years ago by
 Commit changed from a07a21ae1170e224192f9fe6bae34825dae23e1d to c43321bf78adc4ac6ff324f910a879610742d383
Branch pushed to git repo; I updated commit sha1. New commits:
c43321b  Minor modifications in the documentation of tensor/modules.

comment:37 Changed 3 years ago by
 Commit changed from c43321bf78adc4ac6ff324f910a879610742d383 to 5fba412cef169dc7bbc1c32b7d254ab0bf185d4a
comment:38 followups: ↓ 39 ↓ 41 Changed 3 years ago by
 Reviewers set to Travis Scrimshaw
 Status changed from needs_review to needs_work
Okay, I've made a first pass of changes. So overall it looks really good, but there's currently some issues that need addressing. Here's what I've changed:
 Removed trailing whitespace.
 Corrected some indentations.
 Changed lowercase
repr
output into Uppercase.  Added an
__eq__
method toTensorWithIndices
.  Fixed some doc formatting (there's a likely more to do on this). However you have 2 conventions,
type`(a,b)` tensor
andtype `(a,b)` tensor
and IMO you should pick one and stick with it.  Added
TestSuite
to all classes.  Changed some
TypeError
's toValueError
's because the data type was correct but the values within were not.  Made
_an_element_
of the finite rank free module return an element with a basis.  Made the
__eq__
test of tensors check first by identity because it was really annoying to me thatt == t
raised an error.  Removed the
__hash__
and__eq__
ofFiniteRankFreeModule
sinceUniqueRepresentation
takes care of it.  Changed errors from
This is an error message.
tothis is an error message
as I believe it more standard.  Some other misc. changes on some doctests and things like
l != []
fornot l
(it's marginally faster).
Some questions:
 Do we need the class
FormattedExpansion
? It doesn't seem to be used.  Why do you have
_new_instance
, couldn't you use__copy__
?  Should we make the morphisms inherit from
Morphism
and/or have their parents be some kind ofHomset
?  Could we expand out some of the names, ex.
comp()
tocomponents()
(perhaps with aliases for the short names)?  I'm not too happy about the name
basis_change
as I would trychange_of_basis
first, but within that method, could we also check transitivity (there's a method which does this already, but I forget what it's called)?  In Sage, the
view
function outputs as a pdf. So the name of theview()
method is confusing to me. Do you think we could change the name to something else likeexpand
?
Some things to do:
 Implement a
_matrix_
method so (1,1)tensorst
can be passed asmatrix(t)
.  Fix all failures from the
TestSuite
or justify why we can skip certain tests.
There will be more to do I think, but it probably won't be too major.
comment:39 in reply to: ↑ 38 Changed 3 years ago by
Replying to tscrim:
Okay, I've made a first pass of changes. So overall it looks really good, but there's currently some issues that need addressing. Here's what I've changed:
Thank you very much for your work on this ticket! I am looking at the diff and I am impressed by the amount of things that you have changed! This is very instructive to me.
Some questions:
I shall reply later to your questions, but I can already say that I implemented morphisms last week, in the working repository of SageManifolds at https://github.com/sagemanifolds/sage. The free module morphisms inherit from Morphism
and their parent class from Homset
. Coercions have been implemented between endomorphisms and type(1,1) tensors. I shall merge this branch into the ticket branch this week.
Some things to do:
 Implement a
_matrix_
method so (1,1)tensorst
can be passed asmatrix(t)
. Fix all failures from the
TestSuite
or justify why we can skip certain tests.
OK I will work on this in the coming weeks.
comment:40 Changed 3 years ago by
 Commit changed from 5fba412cef169dc7bbc1c32b7d254ab0bf185d4a to 6e53a8d3c92d405541e2a21dff5a867d402937e6
Branch pushed to git repo; I updated commit sha1. New commits:
6e53a8d  Add free module homomorphisms, rename some classes and methods and make small improvements.

comment:41 in reply to: ↑ 38 ; followup: ↓ 44 Changed 3 years ago by
Replying to tscrim:
Some questions:
 Do we need the class
FormattedExpansion
? It doesn't seem to be used.
Yes, we need it: it is used by the method view()
of classes FreeModuleTensor
and FreeModuleAltForm
, to have a nice output, either text formatted or LaTeX formatted (notebook).
 Why do you have
_new_instance
, couldn't you use__copy__
?
Actually _new_instance
is not performing a copy: it simply creates an instance of the same type and same general characteristics (e.g. tensor type), but leave it empty (i.e. it does not perform any copy of stored data, like components).
 Should we make the morphisms inherit from
Morphism
and/or have their parents be some kind ofHomset
?
This is done in the new commit. It introduces free module homomorphisms, via
 the parent class
FreeModuleHomset
(subclass ofsage.categories.homset.Homset
)  the element class
FiniteRankFreeModuleMorphism
(subclass ofsage.categories.morphism.Morphism
)
Coercions to/from type(1,1) tensors have been introduced as well.
 Could we expand out some of the names, ex.
comp()
tocomponents()
(perhaps with aliases for the short names)?
This is done in the new commit, with comp()
as an alias to components()
(since it is really usefull to have a short name to access to components).
 I'm not too happy about the name
basis_change
as I would trychange_of_basis
first,
Yes, you are right: change_of_basis
is a better name. The change has been performed in the new commit.
but within that method, could we also check transitivity (there's a method which does this already, but I forget what it's called)?
Not done yet. Note however that transitivity is implemented in the method FreeModuleTensor.common_basis()
.
 In Sage, the
view
function outputs as a pdf. So the name of theview()
method is confusing to me. Do you think we could change the name to something else likeexpand
?
We thought quite a lot about the name of this method; here we really need a short name, because it is used very often in interactive mode (notebook). The idea is to display tensors in a nice human readable form, but not to change their state, which something like expand
might suggest. display
was too long. An alternative was show
but we eliminated it because in Sage it is used for graphic outputs. So the short name that we ended with was view
. We may also wonder why view
should be reserved for pdf output. Wouldn't view_pdf
or make_pdf
be more suggestive ?
Some things to do:
 Implement a
_matrix_
method so (1,1)tensorst
can be passed asmatrix(t)
.
This is done in the new commit, but on components instead of tensors (indeed, constructing a matrix from a tensor would have required an extra argument: the basis, to define which components are used). So currently, if t
is a type(1,1) tensor and e
is a module basis, we have matrix(t.components(e))
yielding a matrix of base ring elements representing the components of t
w.r.t. basis e
.
 Fix all failures from the
TestSuite
or justify why we can skip certain tests.
Done. Some fixes consisted in initializing the objects (e.g. by setting some components) prior to the TestSuite. Note that _test_category
had to be skipped when constructing the object by a direct call to __init__
, since the correct construction must be performed by a call to parent().element_class
. In the latter case, _test_category
passed.
comment:42 Changed 3 years ago by
 Status changed from needs_work to needs_review
comment:43 Changed 3 years ago by
 Description modified (diff)
comment:44 in reply to: ↑ 41 ; followup: ↓ 45 Changed 3 years ago by
Finally, some time to do more review. It's looking really good.
Replying to egourgoulhon:
Replying to tscrim:
 Do we need the class
FormattedExpansion
? It doesn't seem to be used.Yes, we need it: it is used by the method
view()
of classesFreeModuleTensor
andFreeModuleAltForm
, to have a nice output, either text formatted or LaTeX formatted (notebook).
Ah, I see (I never really use the notebook).
 Should we make the morphisms inherit from
Morphism
and/or have their parents be some kind ofHomset
?This is done in the new commit. It introduces free module homomorphisms, via
 the parent class
FreeModuleHomset
(subclass ofsage.categories.homset.Homset
) the element class
FiniteRankFreeModuleMorphism
(subclass ofsage.categories.morphism.Morphism
)Coercions to/from type(1,1) tensors have been introduced as well.
I'm thinking we should combine the *morphismTensor
into the *morphism
methods and the make FreeModuleTensor
a base class of FreeModuleHomset
. This should fix the need to skip part of the test suite for those classes. Or did you get the dreaded "layout conflict" error?
but within that method, could we also check transitivity (there's a method which does this already, but I forget what it's called)?
Not done yet. Note however that transitivity is implemented in the method
FreeModuleTensor.common_basis()
.
I've had two thoughts about this. The first is pulling out the functionality into a DiGraph
instance where the vertices are bases and the edges are the various transition maps, thus you could use the shorted_path
method. Also suppose you have a chain A > B > C > D
, along the way you would need to compute A > C
, and I'm thinking we should store that morphism (or leave it as an optional argument). Note that you might have to make the transition matrices immutable.
 In Sage, the
view
function outputs as a pdf. So the name of theview()
method is confusing to me. Do you think we could change the name to something else likeexpand
?We thought quite a lot about the name of this method; here we really need a short name, because it is used very often in interactive mode (notebook). The idea is to display tensors in a nice human readable form, but not to change their state, which something like
expand
might suggest.display
was too long. An alternative wasshow
but we eliminated it because in Sage it is used for graphic outputs. So the short name that we ended with wasview
. We may also wonder whyview
should be reserved for pdf output. Wouldn'tview_pdf
ormake_pdf
be more suggestive ?
IDK why it was chosen, but for better or worse, that's the current standard. Also the name doesn't quite match what it does IMO. So I'd really like a different name. Perhaps display
, print
, equation
, peq
(short for print_equation
), ...?
 Fix all failures from the
TestSuite
or justify why we can skip certain tests.Done. Some fixes consisted in initializing the objects (e.g. by setting some components) prior to the TestSuite. Note that
_test_category
had to be skipped when constructing the object by a direct call to__init__
, since the correct construction must be performed by a call toparent().element_class
. In the latter case,_test_category
passed.
These seem to indicate the need for a parent for these elements. For say the linear forms, you should just need a class
LinFormParent(TensorFreeModule): Element = FreeModuleLinForm
(with perhaps a _repr_
stating its the set of all linear forms) and an if
statement when constructing the TensorFreeModule
. This also means they get the benefits from having a proper category. Do you mind doing this?
Also could you change things like
"blah" + str(obj) + "foo"
to
"blah {} foo".format(obj)
(or if you prefer "blah %s foo"%(obj,)
)? I believe this is more pythonic, and it makes it a lot easier to read. Thanks.
comment:45 in reply to: ↑ 44 ; followup: ↓ 48 Changed 3 years ago by
Replying to tscrim:
Finally, some time to do more review.
Thank you very much for this new piece of work!
I'm thinking we should combine the
*morphismTensor
into the*morphism
methods and the makeFreeModuleTensor
a base class ofFreeModuleHomset
. This should fix the need to skip part of the test suite for those classes. Or did you get the dreaded "layout conflict" error?
Actually, given that endomorphisms are now properly dealt with, as Maps, with some coercion from type(1,1) tensors, I am wondering about the necessity to maintain the class FreeModuleEndomorphismTensor
...
Not done yet. Note however that transitivity is implemented in the method
FreeModuleTensor.common_basis()
.I've had two thoughts about this. The first is pulling out the functionality into a
DiGraph
instance where the vertices are bases and the edges are the various transition maps, thus you could use theshorted_path
method. Also suppose you have a chainA > B > C > D
, along the way you would need to computeA > C
, and I'm thinking we should store that morphism (or leave it as an optional argument). Note that you might have to make the transition matrices immutable.
Using digraphs for this sounds indeed appealing...
 In Sage, the
view
function outputs as a pdf. So the name of theview()
method is confusing to me. Do you think we could change the name to something else likeexpand
?We thought quite a lot about the name of this method; here we really need a short name, because it is used very often in interactive mode (notebook). The idea is to display tensors in a nice human readable form, but not to change their state, which something like
expand
might suggest.display
was too long. An alternative wasshow
but we eliminated it because in Sage it is used for graphic outputs. So the short name that we ended with wasview
. We may also wonder whyview
should be reserved for pdf output. Wouldn'tview_pdf
ormake_pdf
be more suggestive ?IDK why it was chosen, but for better or worse, that's the current standard. Also the name doesn't quite match what it does IMO. So I'd really like a different name. Perhaps
display
,equation
,peq
(short forprint_equation
), ...?
OK, maybe display
is a good replacement of view
. It is longer to write, but one can always use TAB completion.
 Fix all failures from the
TestSuite
or justify why we can skip certain tests.Done. Some fixes consisted in initializing the objects (e.g. by setting some components) prior to the TestSuite. Note that
_test_category
had to be skipped when constructing the object by a direct call to__init__
, since the correct construction must be performed by a call toparent().element_class
. In the latter case,_test_category
passed.These seem to indicate the need for a parent for these elements.
Not always, if I understand correctly. For elements that have a proper parent, like FreeModuleTensor
, the construction has to be performed by parent.element_class
, not by a direct call to FreeModuleTensor.__init__
, for _test_category
to be passed (cf. the doctest of FreeModuleTensor.__init__
). If I am correct, this is a general feature of Sage's category framework.
But for elements that are implemented as a subclass of FreeModuleTensor
, like FreeModuleLinForm
, you are right: a proper parent is missing (see below).
For say the linear forms, you should just need a class
LinFormParent(TensorFreeModule): Element = FreeModuleLinForm(with perhaps a
_repr_
stating its the set of all linear forms) and anif
statement when constructing theTensorFreeModule
. This also means they get the benefits from having a proper category. Do you mind doing this?
Yes I will try. For FreeModuleLinForm
there should be no problem. However for FreeModuleAltForm
, it seems we are facing the element subclass issue discussed in this thread. Indeed, not all type(0,p) tensors are alternating forms. Suppose one wants to compute A+B, with A an alternating pform and B a generic type(0,p) tensor. Then A and B will have different parents, so that A._add_
cannot be called directly. Of course, one could implement coercions from alternating pforms to type(0,p) tensors, but this involves creating a new object from scratch, A' say, by copying the components of A. For objects with a lot of components, in many different bases, this may be not very efficient...
Also could you change things like
"blah" + str(obj) + "foo"to
"blah {} foo".format(obj)(or if you prefer
"blah %s foo"%(obj,)
)? I believe this is more pythonic, and it makes it a lot easier to read. Thanks.
OK, I will. Btw, besides being more pythonic, is there any technical advantage in writing
"blah {} foo".format(obj)
instead of "blah" + str(obj) + "foo"
? IMHO, the latter is more readable than the former.
comment:46 followup: ↓ 51 Changed 3 years ago by
 Milestone changed from sage6.4 to sage6.5
 Status changed from needs_review to needs_work
 Work issues set to documentation
The documentation for FiniteRankFreeModule
should really explain what's the difference between FiniteRankFreeModule(ZZ, 3)
and FreeModule(ZZ, 3)
. This documentation should be understandable by somebody who has never looked at this ticket.
comment:47 followup: ↓ 52 Changed 3 years ago by
You write
The class :class:`FiniteRankFreeModule` does not inherit from :class:`~sage.modules.free_module.FreeModule_generic` since the latter is a derived class of :class:`~sage.modules.module.Module_old`, which does not conform to the new coercion model.
but that's no longer true given #10513.
comment:48 in reply to: ↑ 45 Changed 3 years ago by
Replying to egourgoulhon:
I'm thinking we should combine the
*morphismTensor
into the*morphism
methods and the makeFreeModuleTensor
a base class ofFreeModuleHomset
. This should fix the need to skip part of the test suite for those classes. Or did you get the dreaded "layout conflict" error?Actually, given that endomorphisms are now properly dealt with, as Maps, with some coercion from type(1,1) tensors, I am wondering about the necessity to maintain the class
FreeModuleEndomorphismTensor
...
We're on the same wavelength. Hopefully there won't be a (very annoying) technical issue with python being unable to figure out the MRO.
Using digraphs for this sounds indeed appealing...
Let me know if you have any questions.
OK, maybe
display
is a good replacement ofview
. It is longer to write, but one can always use TAB completion.
The more I think about it, the more I like display
. Thanks for changing it.
These seem to indicate the need for a parent for these elements.
Not always, if I understand correctly. For elements that have a proper parent, like
FreeModuleTensor
, the construction has to be performed byparent.element_class
, not by a direct call toFreeModuleTensor.__init__
, for_test_category
to be passed (cf. the doctest ofFreeModuleTensor.__init__
). If I am correct, this is a general feature of Sage's category framework. But for elements that are implemented as a subclass ofFreeModuleTensor
, likeFreeModuleLinForm
, you are right: a proper parent is missing (see below).For say the linear forms, you should just need a class
LinFormParent(TensorFreeModule): Element = FreeModuleLinForm(with perhaps a
_repr_
stating its the set of all linear forms) and anif
statement when constructing theTensorFreeModule
. This also means they get the benefits from having a proper category. Do you mind doing this?Yes I will try. For
FreeModuleLinForm
there should be no problem. However forFreeModuleAltForm
, it seems we are facing the element subclass issue discussed in this thread. Indeed, not all type(0,p) tensors are alternating forms. Suppose one wants to compute A+B, with A an alternating pform and B a generic type(0,p) tensor. Then A and B will have different parents, so thatA._add_
cannot be called directly. Of course, one could implement coercions from alternating pforms to type(0,p) tensors, but this involves creating a new object from scratch, A' say, by copying the components of A. For objects with a lot of components, in many different bases, this may be not very efficient...
I guess the question is how often do you expect to do this? Plus I suspect a lot of the data can just be referenced, not copied, since the data is (essentially) immutable. It's not much different than other parts of Sage and having to be careful about parents. For instance:
sage: %timeit ZZ(1) + QQ(1) 100000 loops, best of 3: 10.4 µs per loop sage: %timeit ZZ(1) + QQ(1) 100000 loops, best of 3: 9.73 µs per loop sage: %timeit QQ(1) + QQ(1) 100000 loops, best of 3: 6.53 µs per loop sage: %timeit QQ(1) + QQ(1) 100000 loops, best of 3: 7.22 µs per loop
The issue is to minimize the penalty. A benefit from doing the implementation is you can add two alternating forms and get an alternating form (for free I believe).
Also could you change things like
"blah" + str(obj) + "foo"to
"blah {} foo".format(obj)(or if you prefer
"blah %s foo"%(obj,)
)? I believe this is more pythonic, and it makes it a lot easier to read. Thanks.OK, I will. Btw, besides being more pythonic, is there any technical advantage in writing
"blah {} foo".format(obj)
instead of"blah" + str(obj) + "foo"
? IMHO, the latter is more readable than the former.
The "blah %s foo"%obj
is the fastest (not by too much), but if obj
is a tuple, an error gets raised (the wrapping (obj,)
gets rid of this for marginal penalty). The format
allows you to do fancier things like repeating objects, for some time, the %
syntax was deprecated in python3 for format
(again, not anymore), and it's on the same order of speed as using +
's. So really there's no real technical advantage other than readability (all python code I've previously seen used the %
and more recently format
, but to be honest, recent python code has all be Sage).
Somewhat unrelated, I do miss the C++ cout<<"blah"<<obj<<"foo"
...
Last I agree with Jeroen, especially comment:47.
Thanks again for your work on all of this.
comment:49 Changed 3 years ago by
 Commit changed from 6e53a8d3c92d405541e2a21dff5a867d402937e6 to 968468796989dd7415c235a6895dadcab6d9e108
Branch pushed to git repo; I updated commit sha1. New commits:
9684687  Add parents for free module automorphisms and alternating forms; improve documentation.

comment:50 Changed 3 years ago by
Thank you very much Travis and Jeroen for your comments. I've just committed the following changes:
 Introduced the parent class
ExtPowerFreeModule
for alternating forms of a given degree on a free module. All alternating forms are now described by the element classFreeModuleAltForm
. In particular, the classFreeModuleLinForm
has been suppressed.
 Introduced the parent class
FreeModuleLinearGroup
for the general linear group GL(M) of a free module M. The corresponding element class isFreeModuleAutomorphism
, which has been entirely rewritten (it is now both a group element class and a class of type(1,1) tensors).
 Added the following coercions:
 alternating forms of degree p > tensors of type (0,p)
 tensors of type (0,1) > alternating forms of degree 1 (linear form)
 automorphisms > tensors of type (1,1)
 automorphisms > endomorphisms
 Added the following conversions:
 (invertible) endomorphisms > automorphisms
 (invertible) type(1,1) tensors > automorphisms
 Suppressed the classes
FreeModuleAutomorphismTensor
andFreeModuleIdentityTensor
, which were no longer needed thanks to the above coercions.
 Suppressed the class
FreeModuleEndomorphismTensor
, which was no longer needed thanks to the coercions endomorphisms <> tensors of type (1,1).
 Renamed method
view()
todisplay()
;view()
is still there (it is heavily used by the users of SageManifolds) but it is set to deprecated.
 Implemented transitivity in
FiniteRankFreeModule.change_of_basis()
, i.e. if the changes of basis A>B and B>C are known and the change of basis A>C is required, it is computed by automorphism composition.
 Added argument
from_family
inFiniteRankFreeModule.basis()
, thereby allowing to construct a new basis from a family of linearly independent module elements (previously, one had to pass a previous basis and an explicit automorphism relating it to the new basis).
 Systematic use of
"...{}...".format(...)
instead of"..." + str(...) + "..."
(except in LaTeX strings, because of the {})
 Reorganized documentation files in src/doc/en/reference/tensor_free_modules/*.rst
 Added comparisons
 between
FiniteRankFreeModule
andFreeModule
,  between
FiniteRankFreeModule
andVectorSpace
,  between
FiniteRankFreeModule
andCombinatorialFreeModule
 between
in the documentation of FiniteRankFreeModule
. You can see the result here. I've also taken into account #comment:47.
All test suites (both for parents and elements) are passed.
comment:51 in reply to: ↑ 46 Changed 3 years ago by
Replying to jdemeyer:
The documentation for
FiniteRankFreeModule
should really explain what's the difference betweenFiniteRankFreeModule(ZZ, 3)
andFreeModule(ZZ, 3)
. This documentation should be understandable by somebody who has never looked at this ticket.
This is done in the new commit: see here.
comment:52 in reply to: ↑ 47 Changed 3 years ago by
Replying to jdemeyer:
You write
The class :class:`FiniteRankFreeModule` does not inherit from :class:`~sage.modules.free_module.FreeModule_generic` since the latter is a derived class of :class:`~sage.modules.module.Module_old`, which does not conform to the new coercion model.but that's no longer true given #10513.
Thanks for pointing this. I've modified the documentation of FiniteRankFreeModule
accordingly, see here.
comment:53 Changed 3 years ago by
 Status changed from needs_work to needs_review
 Work issues documentation deleted
comment:54 Changed 3 years ago by
 Description modified (diff)
comment:55 Changed 3 years ago by
 Commit changed from 968468796989dd7415c235a6895dadcab6d9e108 to 278c94a66f826eb9dfbf5399441fcfda6f883641
Branch pushed to git repo; I updated commit sha1. New commits:
278c94a  Minor changes to coincide with version 0.7 of SageManifolds

comment:56 Changed 3 years ago by
The new commit makes the ticket coincide with the algebraic part of SageManifolds 0.7, which was released yesterday. The changes are very tiny with respect to commit 9684687; here is the log:
 src/sage/tensor/modules/comp.py:
zero_element()
>zero()
(zero_element()
being deprecated in Sage)  src/sage/tensor/modules/format_utilities.py: improve documentation
 class
FormattedExpansion
: suppress the attribute "tensor" (it was actually useless)
 rename attribute "txt" to "_txt"
 rename attribute "latex" to "_latex"
 class
FiniteRankFreeModule
: methodidentity_map()
: add arguments "name" and "latex_name"  class
FreeModuleHomset
: add methodone()
(for endomorphism sets)  class
FreeModuleTensor
: suppress methods__radd__()
and__rsub__()
: they were introduced to write0 + t
and0  t
, wheret
is a tensor, before the coercion of 0 to a tensor was implemented.  src/doc/en/reference/tensor_free_modules/index.rst:add html target ".. _tensorsonfreemodules"
comment:57 Changed 3 years ago by
 Commit changed from 278c94a66f826eb9dfbf5399441fcfda6f883641 to af361f7b69bff3dc3dc24edbe8e3958ce51255bb
Branch pushed to git repo; I updated commit sha1. New commits:
af361f7  Fix a merge conflict in the reference manual w.r.t. the latest develop version of Sage

comment:58 Changed 3 years ago by
 Commit changed from af361f7b69bff3dc3dc24edbe8e3958ce51255bb to 8f1134352dcd79cf1b3e087dcb30bd023470ec1d
Branch pushed to git repo; I updated commit sha1. New commits:
8f11343  Merge #15916 into Sage 6.6.beta5 to solve a conflict in src/doc/en/reference/index.rst

comment:59 Changed 3 years ago by
 Keywords days64 added
 Status changed from needs_review to positive_review
I'm happy with the current version of this, so positive review. Thanks for your work on this!
comment:61 Changed 3 years ago by
 Commit changed from 8f1134352dcd79cf1b3e087dcb30bd023470ec1d to 99f9ac53bd3e42dd00f5d05b7b126a632161ca55
Branch pushed to git repo; I updated commit sha1. New commits:
99f9ac5  Fix doctest because of new doc category.

comment:62 followup: ↓ 63 Changed 3 years ago by
 Status changed from needs_work to positive_review
This is the only (trivial) failure I can see.
comment:63 in reply to: ↑ 62 ; followup: ↓ 64 Changed 3 years ago by
Replying to tscrim:
This is the only (trivial) failure I can see.
In the patchbot log, there are many doctest failures but they pertain to other parts of Sage: all doctests of files in src/sage/tensor/modules (i.e. the sources of this ticket) pass. So I am puzzled: why the patchbot returns so many failures? Could it be that the ticket branch is based on Sage 6.6.beta5?
comment:64 in reply to: ↑ 63 ; followup: ↓ 65 Changed 3 years ago by
Replying to egourgoulhon:
Replying to tscrim:
This is the only (trivial) failure I can see.
In the patchbot log, there are many doctest failures but they pertain to other parts of Sage: all doctests of files in src/sage/tensor/modules (i.e. the sources of this ticket) pass. So I am puzzled: why the patchbot returns so many failures? Could it be that the ticket branch is based on Sage 6.6.beta5?
It has to do with file permissions on the patchbot account. They are harmless and not really errors as far as Sage is concerned, but comes from python IIRC.
comment:65 in reply to: ↑ 64 Changed 3 years ago by
Replying to tscrim:
Replying to egourgoulhon:
Replying to tscrim:
This is the only (trivial) failure I can see.
In the patchbot log, there are many doctest failures but they pertain to other parts of Sage: all doctests of files in src/sage/tensor/modules (i.e. the sources of this ticket) pass. So I am puzzled: why the patchbot returns so many failures? Could it be that the ticket branch is based on Sage 6.6.beta5?
It has to do with file permissions on the patchbot account. They are harmless and not really errors as far as Sage is concerned, but comes from python IIRC.
OK I see. Thank you Travis!
comment:66 Changed 3 years ago by
 Branch changed from public/tensor_modules15916 to 99f9ac53bd3e42dd00f5d05b7b126a632161ca55
 Resolution set to fixed
 Status changed from positive_review to closed
Branch pushed to git repo; I updated commit sha1. New commits:
Minor modifications in tensor/modules, including the add of method _new_instance() to classes FreeModuleAltForm and FreeModuleLinForm