Repository: sympy/sympy
Pull request: Fixes incomplete TensorProduct expand if scalar factors present in summands
<!-- Your title above should be a short description of what
was changed. Do not include the issue number in the title. -->

#### References to other Issues or PRs
<!-- If this pull request fixes an issue, write "Fixes #NNNN" in that exact
format, e.g. "Fixes #1234" (see
https://tinyurl.com/auto-closing for more information). Also, please
write a comment on that issue linking back to this pull request once it is
open. -->
Fixes #24142

#### Brief description of what is fixed or changed
(Replaces identical PR 24147 that was inadvertently closed when renaming branches)
See issue #24142: The expansion of a TensorProduct object stops incomplete if summands in the tensor product factors have scalar factors, e.g.
```
from sympy import *
from sympy.physics.quantum import *
U = Operator('U')
V = Operator('V')
P = TensorProduct(2*U - V, U + V)
print(P) 
# (2*U - V)x(U + V)
print(P.expand(tensorproduct=True)) 
#result: 2*Ux(U + V) - Vx(U + V) #expansion has missed 2nd tensor factor and is incomplete
```
This PR also adds test cases to test_tensorproduct.py.
#### Other comments


#### Release Notes

<!-- Write the release notes for this release below between the BEGIN and END
statements. The basic format is a bulleted list with the name of the subpackage
and the release note for this PR. For example:

* solvers
  * Added a new solver for logarithmic equations.

* functions
  * Fixed a bug with log of integers.

or if no release note(s) should be included use:

NO ENTRY

See https://github.com/sympy/sympy/wiki/Writing-Release-Notes for more
information on how to write release notes. The bot will check your release
notes automatically to see if they are formatted correctly. -->

<!-- BEGIN RELEASE NOTES -->
* physics.quantum
  * Fixed incomplete expansion of TensorProduct if summands have scalar factors
<!-- END RELEASE NOTES -->


Problem statement:
Bug in expand of TensorProduct + Workaround + Fix
### Error description
The expansion of a TensorProduct object stops incomplete if summands in the tensor product factors have (scalar) factors, e.g.
```
from sympy import *
from sympy.physics.quantum import *
U = Operator('U')
V = Operator('V')
P = TensorProduct(2*U - V, U + V)
print(P) 
# (2*U - V)x(U + V)
print(P.expand(tensorproduct=True)) 
#result: 2*Ux(U + V) - Vx(U + V) #expansion has missed 2nd tensor factor and is incomplete
```
This is clearly not the expected behaviour. It also effects other functions that rely on .expand(tensorproduct=True), as e.g. qapply() .

### Work around
Repeat .expand(tensorproduct=True) as may times as there are tensor factors, resp. until the expanded term does no longer change. This is however only reasonable in interactive session and not in algorithms.

### Code Fix
.expand relies on the method TensorProduct._eval_expand_tensorproduct(). The issue arises from an inprecise check in TensorProduct._eval_expand_tensorproduct() whether a recursive call is required; it fails when the creation of a TensorProduct object returns commutative (scalar) factors up front: in that case the constructor returns a Mul(c_factors, TensorProduct(..)).
I thus propose the following  code fix in TensorProduct._eval_expand_tensorproduct() in quantum/tensorproduct.py.  I have marked the four lines to be added / modified:
```
    def _eval_expand_tensorproduct(self, **hints):
                ...
                for aa in args[i].args:
                    tp = TensorProduct(*args[:i] + (aa,) + args[i + 1:])
                    c_part, nc_part = tp.args_cnc() #added
                    if len(nc_part)==1 and isinstance(nc_part[0], TensorProduct): #modified
                        nc_part = (nc_part[0]._eval_expand_tensorproduct(), ) #modified
                    add_args.append(Mul(*c_part)*Mul(*nc_part)) #modified
                break
                ...
```
The fix splits of commutative (scalar) factors from the tp returned. The TensorProduct object will be the one nc factor in nc_part (see TensorProduct.__new__ constructor), if any. Note that the constructor will return 0 if a tensor factor is 0, so there is no guarantee that tp contains a TensorProduct object (e.g. TensorProduct(U-U, U+V).

Hints from issue discussion:
Can you make a pull request with this fix?
Will do. I haven't worked with git before, so bear with me.

But as I'm currently digging into some of the quantum package and have more and larger patches in the pipeline, it seems worth the effort to get git set up on my side. So watch out :-)

Commit message: Added to .mailmap

Patch to review:
diff --git a/.mailmap b/.mailmap
index 3e85675b0853..025916850778 100644
--- a/.mailmap
+++ b/.mailmap
@@ -449,6 +449,7 @@ Colin Marquardt <github@marquardt-home.de>
 Colleen Lee <colleenclee@gmail.com> <clee@coursera.org>
 Comer Duncan <comer.duncan@gmail.com>
 Constantin Mateescu <costica1234@me.com>
+Costor <pcs2009@web.de>
 Craig A. Stoudt <craig.stoudt@gmail.com>
 Cristian Di Pietrantonio <cristiandipietrantonio@gmail.com>
 Cristóvão Sousa <crisjss@gmail.com>
diff --git a/sympy/physics/quantum/tensorproduct.py b/sympy/physics/quantum/tensorproduct.py
index 78accaf2955c..ebe7eff3562e 100644
--- a/sympy/physics/quantum/tensorproduct.py
+++ b/sympy/physics/quantum/tensorproduct.py
@@ -246,9 +246,12 @@ def _eval_expand_tensorproduct(self, **hints):
             if isinstance(args[i], Add):
                 for aa in args[i].args:
                     tp = TensorProduct(*args[:i] + (aa,) + args[i + 1:])
-                    if isinstance(tp, TensorProduct):
-                        tp = tp._eval_expand_tensorproduct()
-                    add_args.append(tp)
+                    c_part, nc_part = tp.args_cnc()
+                    #Check for TensorProduct object: is the one object in nc_part, if any:
+                    #(Note: any other object type to be expanded must be added here)
+                    if len(nc_part)==1 and isinstance(nc_part[0], TensorProduct):
+                        nc_part = (nc_part[0]._eval_expand_tensorproduct(), )
+                    add_args.append(Mul(*c_part)*Mul(*nc_part))
                 break
 
         if add_args:
diff --git a/sympy/physics/quantum/tests/test_tensorproduct.py b/sympy/physics/quantum/tests/test_tensorproduct.py
index f1765640ea2a..11d8abb8aa75 100644
--- a/sympy/physics/quantum/tests/test_tensorproduct.py
+++ b/sympy/physics/quantum/tests/test_tensorproduct.py
@@ -44,6 +44,13 @@ def test_tensor_product_abstract():
 def test_tensor_product_expand():
     assert TP(A + B, B + C).expand(tensorproduct=True) == \
         TP(A, B) + TP(A, C) + TP(B, B) + TP(B, C)
+    #Tests for fix of issue #24142
+    assert TP(A-B, B-A).expand(tensorproduct=True) == \
+         TP(A, B) - TP(A, A) - TP(B, B) + TP(B, A)
+    assert TP(2*A + B, A + B).expand(tensorproduct=True) == \
+        2 * TP(A, A) + 2 * TP(A, B) + TP(B, A) + TP(B, B)
+    assert TP(2 * A * B + A, A + B).expand(tensorproduct=True) == \
+        2 * TP(A*B, A) + 2 * TP(A*B, B) + TP(A, A) + TP(A, B)
 
 
 def test_tensor_product_commutator():


Code review comments:
diff_hunk: @@ -246,9 +246,12 @@ def _eval_expand_tensorproduct(self, **hints):
             if isinstance(args[i], Add):
                 for aa in args[i].args:
                     tp = TensorProduct(*args[:i] + (aa,) + args[i + 1:])
-                    if isinstance(tp, TensorProduct):
-                        tp = tp._eval_expand_tensorproduct()
-                    add_args.append(tp)
+                    c_part, nc_part = tp.args_cnc()
+                    #Check for TensorProduct object: is the one object in nc_part, if any:
+                    #(Note: any other object type to be expanded must be added here)
+                    if len(nc_part)==1 and isinstance(nc_part[0], TensorProduct):
path: sympy/physics/quantum/tensorproduct.py
text: @user1:
```suggestion
                    if len(nc_part) == 1 and isinstance(nc_part[0], TensorProduct):
```

@user1:
Add whitespace for operations
https://peps.python.org/pep-0008/#other-recommendations

diff_hunk: @@ -246,9 +246,12 @@ def _eval_expand_tensorproduct(self, **hints):
             if isinstance(args[i], Add):
                 for aa in args[i].args:
                     tp = TensorProduct(*args[:i] + (aa,) + args[i + 1:])
-                    if isinstance(tp, TensorProduct):
-                        tp = tp._eval_expand_tensorproduct()
-                    add_args.append(tp)
+                    c_part, nc_part = tp.args_cnc()
+                    #Check for TensorProduct object: is the one object in nc_part, if any:
+                    #(Note: any other object type to be expanded must be added here)
path: sympy/physics/quantum/tensorproduct.py
text: @user1:
```suggestion
                    # (Note: any other object type to be expanded must be added here)
```

@user1:
There should be one spacing between `#` and comment for inline comments
https://peps.python.org/pep-0008/#inline-comments

diff_hunk: @@ -246,9 +246,12 @@ def _eval_expand_tensorproduct(self, **hints):
             if isinstance(args[i], Add):
                 for aa in args[i].args:
                     tp = TensorProduct(*args[:i] + (aa,) + args[i + 1:])
-                    if isinstance(tp, TensorProduct):
-                        tp = tp._eval_expand_tensorproduct()
-                    add_args.append(tp)
+                    c_part, nc_part = tp.args_cnc()
+                    #Check for TensorProduct object: is the one object in nc_part, if any:
path: sympy/physics/quantum/tensorproduct.py
text: @user1:
```suggestion
                    # Check for TensorProduct object: is the one object in nc_part, if any:
```

diff_hunk: @@ -44,6 +44,13 @@ def test_tensor_product_abstract():
 def test_tensor_product_expand():
     assert TP(A + B, B + C).expand(tensorproduct=True) == \
         TP(A, B) + TP(A, C) + TP(B, B) + TP(B, C)
+    #Tests for fix of issue #24142
+    assert TP(A-B, B-A).expand(tensorproduct=True) == \
+         TP(A, B) - TP(A, A) - TP(B, B) + TP(B, A)
path: sympy/physics/quantum/tests/test_tensorproduct.py
text: @user1:
```suggestion
        TP(A, B) - TP(A, A) - TP(B, B) + TP(B, A)
```