Opened 5 years ago
Closed 5 years ago
#23207 closed enhancement (fixed)
Exterior powers of free modules of finite rank
Reported by: | egourgoulhon | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-8.1 |
Component: | linear algebra | Keywords: | free module exterior power |
Cc: | tscrim, bpym | Merged in: | |
Authors: | Eric Gourgoulhon | Reviewers: | Travis Scrimshaw |
Report Upstream: | N/A | Work issues: | |
Branch: | b797009 (Commits, GitHub, GitLab) | Commit: | b7970097c1aeb7f2960ff30ccec9e70627e50bb2 |
Dependencies: | Stopgaps: |
Description (last modified by )
This ticket implements the exterior power /\p(M) for a free module of finite rank M (p being a positive integer), i.e. the set of alternating contravariant tensors of type (p,0) on M. Previously only the exterior power of the dual of M, i.e. /\p(M*), was implemented, as the parent of alternating forms of degree p. More specifically, the ticket introduces two new classes:
- the parent class
ExtPowerFreeModule
for /\p(M) - the element class
AlternatingContrTensor
for elements of /\p(M)
Note that the pre-existing class for /\p(M*), which was called ExtPowerFreeModule
, has been renamed ExtPowerDualFreeModule
, since it regards the exterior power of the dual of M.
The class for the elements of M, FiniteRankFreeModuleElement
, inherits from the new class AlternatingContrTensor
, reflecting the fact that /\1(M) = M. In particular, this allows one to consider module elements in operations like the exterior product or the interior product. For instance, for a and b in M, a.wedge(b)
returns now the element a/\b of /\2(M).
In addition, the ticket implements the interior products
- /\p(M) x /\q(M*) --> /\q-p(M*)
- /\p(M*) x /\q(M) --> /\q-p(M)
for p<=q via the method interior_product
in classes AlternatingContrTensor
and FreeModuleAltForm
.
Besides, some slight updates in all source files in src/sage/tensor/modules
have been performed:
- for the migration to Python3, a few
range(...)
have been changed tolist(range(...))
in places where a list is expected instead of an iterator - bibliographic references have been reformated to provide links to the master bibliography file
src/doc/en/reference/references/index.rst
.
Change History (20)
comment:1 Changed 5 years ago by
- Branch set to public/manifolds/ext_powers
- Commit set to 8bc9745dc83b564d73b8cab8ceb59950a702f227
comment:2 Changed 5 years ago by
- Cc tscrim bpym added
- Description modified (diff)
- Status changed from new to needs_review
comment:3 follow-up: ↓ 4 Changed 5 years ago by
Quick comment: Rather than antisym=list(range(degree))
(and similar changes), I think it would be better if the Python3 range
could be taken as valid input. It makes the code more robust and makes things like list(range(5))
less random in some points of the code.
I will look at the code itself soon.
comment:4 in reply to: ↑ 3 ; follow-up: ↓ 5 Changed 5 years ago by
Replying to tscrim:
Quick comment: Rather than
antisym=list(range(degree))
(and similar changes), I think it would be better if the Python3range
could be taken as valid input.
This cannot be because the argument antisym
is not intended to be an iterator, but a list of integers or tuples encoding the antisymmetries of the tensor; for instance antisym=[(0,1), (2,3)]
means that the tensor is antisymmetric with respect to the permutation of its first (0) and second (1) argument/index, as well as with respect to the permutation of its third (2) and fourth (3) argument/index. In this context, antisym=[0,1,2,3]
is a short-hand for antisym=[(0,1,2,3)]
. For alternating tensors of degree p
, one therefore should have antisym=[(0,...,p-1)]
, short-handed as antisym=[0,...,p-1]
and generated by antisym=list(range(p))
.
I will look at the code itself soon.
Thanks!
comment:5 in reply to: ↑ 4 ; follow-up: ↓ 6 Changed 5 years ago by
Replying to egourgoulhon:
Replying to tscrim:
Quick comment: Rather than
antisym=list(range(degree))
(and similar changes), I think it would be better if the Python3range
could be taken as valid input.This cannot be because the argument
antisym
is not intended to be an iterator, but a list of integers or tuples encoding the antisymmetries of the tensor; for instanceantisym=[(0,1), (2,3)]
means that the tensor is antisymmetric with respect to the permutation of its first (0) and second (1) argument/index, as well as with respect to the permutation of its third (2) and fourth (3) argument/index. In this context,antisym=[0,1,2,3]
is a short-hand forantisym=[(0,1,2,3)]
. For alternating tensors of degreep
, one therefore should haveantisym=[(0,...,p-1)]
, short-handed asantisym=[0,...,p-1]
and generated byantisym=list(range(p))
.
range
in Python3 has its own type (currently xrange
in Python2) that you can explicitly check against:
sage: from six.moves import range as py3range sage: x = py3range(5) sage: isinstance(x, py3range) True
So you can still support the Python3 range but not support iterators.
comment:6 in reply to: ↑ 5 Changed 5 years ago by
Replying to tscrim:
range
in Python3 has its own type (currentlyxrange
in Python2) that you can explicitly check against:sage: from six.moves import range as py3range sage: x = py3range(5) sage: isinstance(x, py3range) TrueSo you can still support the Python3 range but not support iterators.
Yes, but I am afraid I don't understand what you have in mind. The attribute _antisym
of tensors is a list, which in practice is very short: in most applications, it contains 0 or 1 element, sometimes 2 (e.g. [(0,1), (2,3)] for the fully covariant version of the Riemann tensor). The elements are tuples indicating which indices are involved in the antisymmetry. For the case of alternating tensors, _antisym
is a list with a single element, since there is a single antisymmetry, the one corresponding to all possible permutations of the indices. This unique antisymmetry is encoded by the tuple (0,1,...,p-1)
, p
being the degree of the alternating tensor. So we must have _antisym=[(0,1,...,p-1)]
in this case, with (0,1,...,p-1)
being really a tuple of integers. What would be the advantage of using Python3 range
in this context?
comment:7 follow-up: ↓ 8 Changed 5 years ago by
I understand what the input for antisym
is suppose to be. I guess the fundamental question is do you want to suppose iterators for the input, whether or not they give the tuples or a integers. I'm just worried about the shock when going to Python3. The option I'm suggesting I guess would be to just automatically run antisym = list(antisym)
at the start of the __init__
method. I'm leaning towards the principle of trying to support range
whenever possible. However, it is currently not a very strong opinion, so if you don't want to, then it's not something I will hold up this ticket for.
comment:8 in reply to: ↑ 7 Changed 5 years ago by
Replying to tscrim:
I understand what the input for
antisym
is suppose to be. I guess the fundamental question is do you want to suppose iterators for the input, whether or not they give the tuples or a integers. I'm just worried about the shock when going to Python3. The option I'm suggesting I guess would be to just automatically runantisym = list(antisym)
at the start of the__init__
method.
OK I see.
I'm leaning towards the principle of trying to support
range
whenever possible. However, it is currently not a very strong opinion, so if you don't want to, then it's not something I will hold up this ticket for.
OK.
comment:9 Changed 5 years ago by
- Commit changed from 8bc9745dc83b564d73b8cab8ceb59950a702f227 to 4d24de4a4b36410ae945e4ce5ac4e39b348ad945
Branch pushed to git repo; I updated commit sha1. New commits:
4d24de4 | Replace iteritems and itervalues by .items() and .values() in src/sage/tensor/modules
|
comment:10 Changed 5 years ago by
The commit in comment:9 gets rid of some six.iteritems
and six.itervalues
since the lack of iterator is not crucial in the Python2 version; .items()
and .values()
suffice: no significant loss of performance and this makes the python3_py
plugin of the patchbot happy ;-).
comment:11 follow-ups: ↓ 14 ↓ 16 Changed 5 years ago by
Are you going to support input of range
? I think this would be good and means you don't have to wrap so many of those calls with list
. I know it is a shorthand, but it still seems very nice to have. Also, I think it would be really good to support
resu.set_comp()[:] = range(1, self._rank+1)
Both _init_derived
and _del_derived
are unnecessary since they are just super calls.
The changes to tensor_with_indices.py
are unnecessary as in Python3:
>>> def foo(a1,a2): ... print(a1, a2) ... >>> foo(*range(2)) 0 1
type-(1,0) type-`(1,0)`
Remove the commented code:
# From sage/modules/module.pyx: #----------------------------- ### The Element should also implement _rmul_ (or _lmul_) # # class MyElement(sage.structure.element.ModuleElement): # def _rmul_(self, c): # ...
Break the long line here:
The class :class:`FiniteRankFreeModuleElement` inherits from - :class:`~sage.tensor.modules.alternating_contr_tensor.AlternatingContrTensor` because the elements of a free module `M` of + :class:`~sage.tensor.modules.alternating_contr_tensor.AlternatingContrTensor` + because the elements of a free module `M` of finite rank over a commutative ring `R` are identified with tensors of type `(1,0)` on `M` via the canonical map
I think this looks better (I believe there are two instances of this):
*p-th exterior power of* `p`-*th exterior power of*
Are you concerned about dealing with old pickles? If so, have you checked that you can correctly unpickle a previous object? In particular, I think because you renamed the class, the old pickles will not create the correct class, but instead the dual version you constructed here. Although it might just be much more feasible (on our side) to deal with the backwards incompatibility instead.
comment:12 Changed 5 years ago by
- Commit changed from 4d24de4a4b36410ae945e4ce5ac4e39b348ad945 to 41fcf4efa6ec9fa6a49f7bd67db590d964db8eeb
Branch pushed to git repo; I updated commit sha1. New commits:
41fcf4e | Support of Python3 range in inputs of tensor symmetries
|
comment:13 Changed 5 years ago by
- Commit changed from 41fcf4efa6ec9fa6a49f7bd67db590d964db8eeb to 59b46cd08464e28a660455830f3cf3dbccf91db9
Branch pushed to git repo; I updated commit sha1. New commits:
59b46cd | Remove unnecessary _init_derived and _del_derived from alternating tensors
|
comment:14 in reply to: ↑ 11 Changed 5 years ago by
Replying to tscrim:
Thanks for your comments!
Are you going to support input of
range
? I think this would be good and means you don't have to wrap so many of those calls withlist
. I know it is a shorthand, but it still seems very nice to have.
Done! We have now:
sage: from six.moves import range sage: M = FiniteRankFreeModule(QQ, 3, name='M') sage: e = M.basis('e') sage: a = M.tensor((2,1), sym=range(2)) sage: a.symmetries() symmetry: (0, 1); no antisymmetry sage: b = M.tensor((0,3), antisym=range(3)) sage: b.symmetries() no symmetry; antisymmetry: (0, 1, 2) sage: c = M.tensor((0,3), antisym=range(1,3)) sage: c.symmetries() no symmetry; antisymmetry: (1, 2) sage: t = M.tensor((2,0)) sage: t[:] = [[2,1,-3],[0,-4,5],[-1,4,2]] sage: s = t.symmetrize(*range(2)) sage: s.symmetries() symmetry: (0, 1); no antisymmetry sage: s == t.symmetrize(0,1) True sage: s = t.antisymmetrize(*range(2)) sage: s.symmetries() no symmetry; antisymmetry: (0, 1) sage: s == t.antisymmetrize(0, 1) True
Also, I think it would be really good to support
resu.set_comp()[:] = range(1, self._rank+1)
Done as well; with the same settings as above, we have:
sage: v = M(range(3)) sage: v.display() e_1 + 2 e_2 sage: v.set_comp()[:] = range(1, 4) sage: v.display() e_0 + 2 e_1 + 3 e_2
Both
_init_derived
and_del_derived
are unnecessary since they are just super calls.
Indeed; I've removed them.
The changes to
tensor_with_indices.py
are unnecessary as in Python3:>>> def foo(a1,a2): ... print(a1, a2) ... >>> foo(*range(2)) 0 1
Thanks for pointing this out. I've restored the original version of tensor_with_indices.py
.
Remove the commented code:
# From sage/modules/module.pyx: #----------------------------- ### The Element should also implement _rmul_ (or _lmul_) # # class MyElement(sage.structure.element.ModuleElement): # def _rmul_(self, c): # ...
Done.
Break the long line here:
The class :class:`FiniteRankFreeModuleElement` inherits from - :class:`~sage.tensor.modules.alternating_contr_tensor.AlternatingContrTensor` because the elements of a free module `M` of + :class:`~sage.tensor.modules.alternating_contr_tensor.AlternatingContrTensor` + because the elements of a free module `M` of finite rank over a commutative ring `R` are identified with tensors of type `(1,0)` on `M` via the canonical map
Done.
I think this looks better (I believe there are two instances of this):
*p-th exterior power of* `p`-*th exterior power of*
Done.
Are you concerned about dealing with old pickles?
Not at all. I don't think anybody is unpickling old exterior powers of dual free modules. So we can safely ignore this. But thanks for pointing it. One shall indeed have this in mind when changing the name of a class.
comment:15 Changed 5 years ago by
- Commit changed from 59b46cd08464e28a660455830f3cf3dbccf91db9 to b7970097c1aeb7f2960ff30ccec9e70627e50bb2
Branch pushed to git repo; I updated commit sha1. New commits:
b797009 | Typeset of tensor types in the documentation
|
comment:16 in reply to: ↑ 11 Changed 5 years ago by
Replying to tscrim:
type-(1,0) type-`(1,0)`
Done. I've also changed the phrases like
of type (0,1)
to
of type `(0,1)`
.
comment:17 follow-up: ↓ 18 Changed 5 years ago by
- Milestone changed from sage-8.0 to sage-8.1
- Reviewers set to Travis Scrimshaw
- Status changed from needs_review to positive_review
Great, thank you.
comment:18 in reply to: ↑ 17 Changed 5 years ago by
Thank you very much for the review!
comment:19 Changed 5 years ago by
I don't understand why the latest patchbot reports an issue with the .. SEEALSO::
blocks in alternating_contr_tensor.py
and free_module_alt_form.py
; they seems correctly formed to me and the output html is fine. Moreover, in line 433 of https://github.com/sagemath/sage-patchbot/blob/master/sage_patchbot/plugins.py , it is written
1) correct syntax is .. SEEALSO::
comment:20 Changed 5 years ago by
- Branch changed from public/manifolds/ext_powers to b7970097c1aeb7f2960ff30ccec9e70627e50bb2
- Resolution set to fixed
- Status changed from positive_review to closed
New commits:
Rename class ExtPowerFreeModule to ExtPowerDualFreeModule
First implementation of exterior powers of free modules (classes ExtPowerFreeModule and AlternatingContrTensor)
Class FiniteRankFreeModuleElement inherits from new class AlternatingContrTensor
Add method interior_product in class CompFullyAntiSym
Implement method interior_product in classes AlternatingContrTensor and FreeModuleAltForm
Complete doctests of FreeModuleAltForm.interior_product + update references (links to master file)