search

sagemath / sage

Main repository of SageMath
https://www.sagemath.org
Other
1.47k stars 485 forks source link

Parent methods tensor vs. tensor_product #30373

Open mkoeppe opened 4 years ago

mkoeppe commented 4 years ago

We unify the use of the methods tensor vs. tensor_product in parent classes.

Current situation:

Category ModulesWithBasis provides a parent method tensor to construct a tensor product of modules.

sage: C = CombinatorialFreeModule(QQ, ['x', 'y'])                                                                                                                
sage: C.tensor(C)                                                                                                                                                
Free module generated by {'x', 'y'} over Rational Field # Free module generated by {'x', 'y'} over Rational Field

It is not implemented completely for FreeModule

sage: V = FreeModule(QQ, 2)                                                                                                                                      
sage: V.tensor(V)                                                                                                                                                
AttributeError: type object 'FreeModule_ambient_field_with_category' has no attribute 'Tensor'
sage:

In contrast, FiniteRankFreeModule (which is not in ModulesWithBasis) uses a parent method tensor to construct elements.

sage: F = FiniteRankFreeModule(QQ, 2)
sage: F.tensor((2, 2))                                                                                                                                           
Type-(2,2) tensor on the 2-dimensional vector space over the Rational Field

FilteredVectorSpace has both tensor (from ModulesWithBasis) and tensor_product, but only the latter works.

sage: FV = FilteredVectorSpace(2)                                                                                                                                
sage: FV.tensor_product(FV)                                                                                                                                      
QQ^4
sage: FV.tensor(FV)                                                                                                                                              
AttributeError: type object 'FilteredVectorSpace_class_with_category' has no attribute 'Tensor'

The same is true for FreeQuadraticModule_integer_symmetric:

sage: L = IntegralLattice(Matrix(ZZ, 2, [2,1,1,-2]))                                                                                                             
sage: L.tensor_product(L)                                                                                                                                        
Lattice of degree 4 and rank 4 over Integer Ring
Standard basis 
Inner product matrix:
[ 4  2  2  1]
[ 2 -4  1 -2]
[ 2  1 -4 -2]
[ 1 -2 -2  4]
sage: L.tensor(L)                                                                                                                                                
AttributeError: type object 'FreeQuadraticModule_integer_symmetric_with_categor' has no attribute 'Tensor'

The proposed solution in this ticket is to standardize on the method tensor_product, making tensor a deprecated alias.

Modules already has a tensor_square method, which tensor_product complements well.

We also add tensor_power.

No changes are made to the global tensor (unique instance of TensorProductFunctor).

Tickets:

See also: #18349

CC: @tscrim @egourgoulhon @mjungmath @jhpalmieri @fchapoton

Component: linear algebra

Author: Matthias Koeppe

Branch/Commit: u/mkoeppe/parent_methods_tensor_vs__tensor_product @ ed6a13f

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

mkoeppe commented 4 years ago

Description changed:

--- 
+++ 
@@ -50,3 +50,11 @@
 AttributeError: type object 'FreeQuadraticModule_integer_symmetric_with_categor' has no attribute 'Tensor'

+ +The proposed solution in this ticket is to standardize on tensor_product, making tensor a deprecated alias.
+ +Modules already has a tensor_square method, which tensor_product complements well. + +We also add tensor_power. + +

mkoeppe commented 4 years ago

Description changed:

--- 
+++ 
@@ -1,4 +1,4 @@
-We unify the use of `tensor` vs. `tensor_product` in parent classes.
+We unify the use of the methods `tensor` vs. `tensor_product` in parent classes.

 Current situation:

@@ -51,10 +51,10 @@

-The proposed solution in this ticket is to standardize on tensor_product, making tensor a deprecated alias.
+The proposed solution in this ticket is to standardize on the method tensor_product, making tensor a deprecated alias.

Modules already has a tensor_square method, which tensor_product complements well.

We also add tensor_power.

- +No changes are made to the global tensor (unique instance of TensorProductFunctor).

mkoeppe commented 3 years ago

Description changed:

--- 
+++ 
@@ -58,3 +58,5 @@
 We also add `tensor_power`.

 No changes are made to the global `tensor` (unique instance of `TensorProductFunctor`).
+
+See also: #18349
jhpalmieri commented 3 years ago
comment:6

A few other things to think about:

mjungmath commented 3 years ago
comment:7

Replying to @jhpalmieri:

A few other things to think about:

  • In the first example, C.tensor(C, C) works as expected. In contrast, the tensor function doesn't take multiple inputs, but instead an iterable: tensor([C,C,C]) instead of tensor(C, C, C). I would prefer a consistent syntax.

+1. I don't have a strong opinion which one we choose. Multiple inputs feels more flexible (and doesn't need an additional bracket).

  • What about tensoring elements together? Right now the tensor function works on elements, although the documentation doesn't make this clear. I am happy for this to continue, but again, the syntax is different for tensor([a,b,c]) vs. a.tensor(b,c).

I like that behavior, too. However, it feels to me like this behavior was not intended and just works because the element provides a tensor method (that's probably why it's not mentioned in the documentation). From a mathematical-categorial viewpoint this behavior is anyway unexpected (functors usually do not act on objects of objects).

If we want to promote this, which I would agree with, the syntax should indeed be the same.

This might also be a good opportunity to introduce the @ notation you have proposed in here which I really like (so +1 from my side, too).

As for FiniteRankFreeModule, introducing methods like tensor_product and tensor_power is readily done (I already finished the former).

So, should we split the task into several pieces, and I provide the FiniteRankFreeModule add-on? You just have to tell me which syntax you prefer.

mkoeppe commented 3 years ago
comment:8

Replying to @mjungmath:

This might also be a good opportunity to introduce the @ notation you have proposed in here which I really like (so +1 from my side, too).

-1 on this - as per discussion in #30244, @ should be for tensor contraction, not tensor product

mjungmath commented 3 years ago
comment:9

Replying to @mkoeppe:

Replying to @mjungmath:

This might also be a good opportunity to introduce the @ notation you have proposed in here which I really like (so +1 from my side, too).

-1 on this - as per discussion in #30244, @ should be for tensor contraction, not tensor product

But the idea in principle remains, doesn't it? What about # instead? That's already the symbol for tensor products as in categories/tensor.py.

Arrrg, that's commenting...

mkoeppe commented 3 years ago
comment:10

Python has only a fixed set of binary operators available. One can resort to overloading something that's rarely used otherwise. In #30244 I suggested & as a possibility, but I am not sure if it's a good idea.

mjungmath commented 3 years ago
comment:11

Replying to @mkoeppe:

Python has only a fixed set of binary operators available. One can resort to overloading something that's rarely used otherwise. In #30244 I suggested & as a possibility, but I am not sure if it's a good idea.

It feels rather uncanonical. But tensor products seem to be used quite frequently, so if there's no better bet...

What's your opinion on the syntax?

Should I open another ticket to provide tensor_product, tensor_power for FiniteRankFreeModule or just push it into here as open branch?

jhpalmieri commented 3 years ago
comment:12

We should make people use the unicode ⊗ for tensor product.

More seriously, if we can switch the syntax to tensor(a,b,c,...) with no extra brackets, that would be nice. I don't know if any of the tensor constructors take optional arguments, so I don't know how hard that would be to implement.

mjungmath commented 3 years ago
comment:13

I've opened a ticket for the FiniteRankFreeModule case: #31276.

tscrim commented 3 years ago
comment:14

Multiple inputs can also be really annoying when you want to create a list dynamically as you then have to add a * and possibly extra parentheses. The easiest thing to do is to support both behaviors. This is simple enough to implement.

The tensor unicode would be good, except it would depend on how the Python interpreter takes that. Most likely, we would need to implement another find-replace case in the preparser to go to some overloaded infix operator such as &.

mjungmath commented 3 years ago
comment:15

Speaking of shortcuts for operations: what about using ^ for the wedge product? There is nothing more appropriate than that!

If that meets your approval, I'll open a ticket for this.

jhpalmieri commented 3 years ago
comment:16

I find it too easily confused with the exponentiation operator, so I would not like to see it featured prominently in doctests.

mjungmath commented 3 years ago
comment:17

So far, the ^ is not supported for exterior algebras:

sage: M = FiniteRankFreeModule(QQ, 3)
sage: AM = M.dual_exterior_power(2)
sage: a = AM.an_element()
sage: a^2
Traceback (most recent call last)
...
TypeError: unsupported operand parent(s) for ^: '2nd exterior power of the dual of the 3-dimensional vector space over the Rational Field' and 'Integer Ring'

But I agree. One would rather expect a^3 == a.wedge(a).wedge(a) if supported.

Mh, one minute ago, I thought it was an ingenious idea. :D

tscrim commented 3 years ago
comment:18

Replying to @jhpalmieri:

I find it too easily confused with the exponentiation operator, so I would not like to see it featured prominently in doctests.

However, I think it would be a nice syntactic sugar since a ^ b, for a,b in an exterior algebra, as exponentiation does not make sense.

mjungmath commented 3 years ago
comment:19

Replying to @tscrim:

Replying to @jhpalmieri:

I find it too easily confused with the exponentiation operator, so I would not like to see it featured prominently in doctests.

However, I think it would be a nice syntactic sugar since a ^ b, for a,b in an exterior algebra, as exponentiation does not make sense.

How do we treat ^ with integers then? As a wedge product, this is the simple multiplication. And I still agree, that this case can easily be confused with the exponential since exponentiation is the standard behavior of Sage anywhere else.

egourgoulhon commented 3 years ago
comment:20

Replying to @jhpalmieri:

We should make people use the unicode ⊗ for tensor product.

A relevant ticket here: #30473

egourgoulhon commented 3 years ago
comment:21

Replying to @mjungmath:

Replying to @tscrim:

However, I think it would be a nice syntactic sugar since a ^ b, for a,b in an exterior algebra, as exponentiation does not make sense.

How do we treat ^ with integers then? As a wedge product, this is the simple multiplication. And I still agree, that this case can easily be confused with the exponential since exponentiation is the standard behavior of Sage anywhere else.

+1 (and IMHO a^b looks too far from a wedge product).

mkoeppe commented 3 years ago
comment:22

Setting new milestone based on a cursory review of ticket status, priority, and last modification date.

mkoeppe commented 2 years ago

Description changed:

--- 
+++ 
@@ -51,7 +51,7 @@

-The proposed solution in this ticket is to standardize on the method tensor_product, making tensor a deprecated alias.
+The proposed solution in this ticket is to standardize on the method tensor_product, making tensor a deprecated alias.

Modules already has a tensor_square method, which tensor_product complements well.

@@ -59,4 +59,7 @@

No changes are made to the global tensor (unique instance of TensorProductFunctor).

+Tickets: +- #31276 Tensor Product Method for FiniteRankFreeModule + See also: #18349

mkoeppe commented 2 years ago

Branch: u/mkoeppe/parent_methods_tensor_vs__tensor_product

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 2 years ago

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

ae72687sage.categories: tensor -> tensor_product
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 2 years ago

Commit: ae72687

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 2 years ago

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

8026732sage.categories: tensor -> tensor_product
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 2 years ago

Changed commit from ae72687 to 8026732

mkoeppe commented 2 years ago
comment:31

Looks like the element classes also need to get tensor_product methods.

I'd welcome comments in this early stage whether this is going in a good direction

egourgoulhon commented 2 years ago
comment:32

Replying to @mkoeppe:

Looks like the element classes also need to get tensor_product methods.

I'd welcome comments in this early stage whether this is going in a good direction

FWIW, in FiniteRankFreeModule's, the element tensor product is implemented via the operator *, i.e. via the method __mul__.

egourgoulhon commented 2 years ago

Description changed:

--- 
+++ 
@@ -21,6 +21,7 @@
 In contrast, `FiniteRankFreeModule` (which is not in `ModulesWithBasis`) uses a parent method `tensor` to construct elements.

+sage: F = FiniteRankFreeModule(QQ, 2) sage: F.tensor((2, 2))
Type-(2,2) tensor on the 2-dimensional vector space over the Rational Field

mkoeppe commented 2 years ago

Description changed:

--- 
+++ 
@@ -63,4 +63,7 @@
 Tickets:
 - #31276 Tensor Product Method for `FiniteRankFreeModule`

+Follow-up:
+- #34448 Put tensor modules of `FiniteRankFreeModule` in `Modules().TensorProducts()`
+
 See also: #18349
mkoeppe commented 2 years ago
comment:36

The branch of #34448 shows the necessary workarounds in sage.tensors if we wanted to keep tensor (not tensor_product) as the name of the method that implements the action of TensorProductFunctor: The method FiniteRankFreeModule.tensor would have to double both as that and as the method that construct an element.

Any thoughts which of the 2 ways to go?

tscrim commented 2 years ago
comment:37

The tensor() method is something used by the tensor functor as the hook within each class to build the tensor product IIRC. So it is a little more special of a method.

mkoeppe commented 2 years ago
comment:38

Yes, exactly. The branch here on the ticket changes it:

diff --git a/src/sage/categories/tensor.py b/src/sage/categories/tensor.py
index 67c4e64..18109d0 100644
--- a/src/sage/categories/tensor.py
+++ b/src/sage/categories/tensor.py
@@ -48,7 +48,7 @@ class TensorProductFunctor(CovariantFunctorialConstruction):

         sage: TestSuite(tensor).run()
     """
-    _functor_name = "tensor"
+    _functor_name = "tensor_product"
     _functor_category = "TensorProducts"
     symbol = " # "
     unicode_symbol = f" {unicode_otimes} "
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 2 years ago

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

9de134fModulesWithBasis.ParentMethods: Rename tensor to tensor_product, with deprecation
785c197sage.categories: tensor -> tensor_product
77cd522Also rename element methods from tensor to tensor_product
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 2 years ago

Changed commit from 8026732 to 77cd522

tscrim commented 2 years ago
comment:40

I see. It wasn’t entirely clear that was the proposal.

I am maybe marginally in favor of tensor since we say “A tensor B” and it would create slightly tension with the functor being called tensor In the global namespace. For the middle example, another way out of this would be to check the types of the input.

IMO, the last two examples in the ticket description perhaps should fail because the result doesn’t know about its tensor product construction. Yes, QQ^2 (x) QQ^2 = QQ^2 (+) QQ^2 = QQ^4, where = means isomorphic as QQ-vector spaces here, but there are two separate canonical projections onto QQ^2 in the middle case and a functoral way to lift up QQ^2 endomorphisms in the first case. There are also different structures if we have additional structure, such as a grading. TL;DR they are losing information that the result of tensor (Perhaps implicitly) is assuming is preserved.

mkoeppe commented 2 years ago
comment:41

The problem is that we have the clash with FiniteRankFreeModule.tensor.

mkoeppe commented 2 years ago
comment:42

Replying to Travis Scrimshaw:

For the middle example, another way out of this would be to check the types of the input.

Ah, I see that is what you referred to already.

mkoeppe commented 2 years ago
comment:43

So yes, in the branch of #34448 I fix the clash by checking types. But I'm not sure if this a clean enough solution.

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 2 years ago

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

bb0f26csrc/sage/homology/chain_complex.py: Add missing import
719de1egit grep -l '[.]tensor(' | xargs sed -i.bak 's/[.]tensor(/.tensor_product(/'
aebe837src/sage/categories/modules_with_basis.py: Restore 'tensor' in documentation of deprecated tensor methods
3d6f44eFix up tensor -> tensor_product changes
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 2 years ago

Changed commit from 77cd522 to 3d6f44e

mkoeppe commented 2 years ago
comment:45

Replying to Travis Scrimshaw:

IMO, the last two examples in the ticket description perhaps should fail because the result doesn’t know about its tensor product construction.

I haven't looked at these examples in detail, but your explanation makes sense. Nothing for this ticket though.

tscrim commented 2 years ago
comment:46

Replying to Matthias Köppe:

So yes, in the branch of #34448 I fix the clash by checking types. But I'm not sure if this a clean enough solution.

Indeed, it is not the most clean solution, but it is a side effect of English (and Python) that we cannot easily distinguish between “a tensor in” and “the tensor with” in such a concise way.

mkoeppe commented 2 years ago
comment:47

Replying to Travis Scrimshaw:

I am maybe marginally in favor of tensor since we say “A tensor B” and it would create slightly tension with the functor being called tensor In the global namespace.

We could of course make tensor a non-deprecated alias and document that classes are allowed to overload it for additional idiomatic uses.

tscrim commented 2 years ago
comment:48

Replying to Matthias Köppe:

Replying to Travis Scrimshaw:

IMO, the last two examples in the ticket description perhaps should fail because the result doesn’t know about its tensor product construction.

I haven't looked at these examples in detail, but your explanation makes sense. Nothing for this ticket though.

If we rename it to tensor_product, then it starts working in a way that is incompatible with the tensor functor. It’s not a really big problem, but it could run into subtleties with tensor([C, FV]) versus tensor([FV, C]) with one of them working (and returning a wrong answer depending on the implementation that I haven’t checked) and the other not.

mkoeppe commented 2 years ago
comment:49

Hm? No, the tensor functor calls the method given in its _functor_name. That's what the change in comment:38 is for.

mkoeppe commented 2 years ago
comment:50

Oh, I think I get what you are saying: These two examples are undisciplined tensor products that don't comply with the guarantees of the functor?

tscrim commented 2 years ago
comment:51

Replying to Matthias Köppe:

Hm? No, the tensor functor calls the method given in its _functor_name. That's what the change in comment:38 is for.

Right, and FV and L from the description have a tensor_product method already. This could potentially take a CFM C and return, say, a filtered vector space (with the right methods), but not the other way around. Even if both worked (which is also possible), they would result in very different implementations. Although since tensor products in general are not “commutative,” we can wave it away, but it would mean more brittle code.

tscrim commented 2 years ago
comment:52

Replying to Matthias Köppe:

Oh, I think I get what you are saying: These two examples are undisciplined tensor products that don't comply with the guarantees of the functor?

Yes, that’s right. Sorry if I wasn’t explaining clearly enough.

mkoeppe commented 2 years ago
comment:53

By the way, FilteredVectorSpace appears to be the only consumer of sage.modules.tensor_operations, which can probably be eliminated