Repository: sympy/sympy
Pull request: Fix substituting TensExpr in Mul
<!-- 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 #25051

#### Brief description of what is fixed or changed
Earlier, substituting a TensExpr into a Mul would output a Mul. Now, the Mul is converted to a TensMul.
I've also added some tests for Mul->TensMul and Add->TensAdd.

#### Other comments
The conversion already seemed to work for Add->TensAdd, so I've just added tests for it.

I have put the tests in tensor/tests/test_tensor.py, but is there a better place for them?

#### 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. Formerly, `log(-x)` incorrectly gave `-log(x)`.

* physics.units
  * Corrected a semantical error in the conversion between volt and statvolt which
    reported the volt as being larger than the statvolt.

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 -->
* tensor
  * Substituting a Tensor into a Mul now produces a TensMul as expected.
<!-- END RELEASE NOTES -->


Problem statement:
Substituting Tensor into Mul returns Mul rather than TensMul
When I substitute a tensor into a Mul (using `subs` or `replace`), I would expect the output to be a `TensMul`, but it remains a `Mul`.

(in isympy prompt)
```
In [1]: from sympy import symbols
In [2]: from sympy.tensor.tensor import TensorIndexType, tensor_indices, TensorHead
In [3]: R3 = TensorIndexType('R3', dim=3)
In [4]: i = tensor_indices("i", R3)
In [5]: K = TensorHead("K", [R3])
In [7]: expr = (x*2).replace(x, K(i))
In [9]: type(expr)
Out[9]: sympy.core.mul.Mul
```

This is a problem in more complicated expressions, since the user might not even notice that one of the terms has the wrong type, and then they might wonder why various methods (e.g. expand) don't work as expected.

One possible way to fix this (but only for `subs`) is to check in `Mul._eval_subs` if the argument `new` is a `TensExpr`, and cast the output accordingly.

However, this would not fix the issue for `xreplace` and `replace`. Any ideas on how this can be fixed? Is it acceptable to add a check in `AssocOp.__new__`?

Sympy version: 1.11.1
Python version: 3.10.10
OS: Arch Linux

Hints from issue discussion:
This should be handled with postprocessors, similar to https://github.com/sympy/sympy/blob/master/sympy/matrices/expressions/matexpr.py#L518. 

More generally, it might be a good idea to make tensor expressions and matrix expressions share some code, but that could be a lot of work.
I've made a PR: https://github.com/sympy/sympy/pull/25056

Commit message: tensor: get_postprocessor: remove unnecessary use of list in any()

flake8 does not like it.

Patch to review:
diff --git a/sympy/tensor/tensor.py b/sympy/tensor/tensor.py
index 3d0c5da67b89..07280946d1fb 100644
--- a/sympy/tensor/tensor.py
+++ b/sympy/tensor/tensor.py
@@ -4861,3 +4861,23 @@ def _expand(expr, **kwargs):
         return expr._expand(**kwargs)
     else:
         return expr.expand(**kwargs)
+
+
+def get_postprocessor(cls):
+    def _postprocessor(expr):
+        tens_class = {Mul: TensMul, Add: TensAdd}[cls]
+        if any(isinstance(a, TensExpr) for a in expr.args):
+            return tens_class(*expr.args)
+        else:
+            return expr
+
+    return _postprocessor
+
+"""
+The following makes sure that if a user tries to put a tensor in a Mul, it
+automatically gets converted to a TensMul (see github issue #25051). The
+conversion already seems to work for TensAdd, so we don't do anything for that.
+"""
+Basic._constructor_postprocessor_mapping[TensExpr] = {
+    "Mul": [get_postprocessor(Mul)],
+}
diff --git a/sympy/tensor/tests/test_tensor.py b/sympy/tensor/tests/test_tensor.py
index 043dd9641416..a13675860bc1 100644
--- a/sympy/tensor/tests/test_tensor.py
+++ b/sympy/tensor/tests/test_tensor.py
@@ -2040,3 +2040,22 @@ def test_TensorType():
 def test_dummy_fmt():
     with warns_deprecated_sympy():
         TensorIndexType('Lorentz', dummy_fmt='L')
+
+def test_postprocessor():
+    """
+    Test if substituting a Tensor into a Mul or Add automatically converts it
+    to TensMul or TensAdd respectively. See github issue #25051
+    """
+    R3 = TensorIndexType('R3', dim=3)
+    i = tensor_indices("i", R3)
+    K = TensorHead("K", [R3])
+    x,y,z = symbols("x y z")
+
+    assert isinstance((x*2).xreplace({x: K(i)}), TensMul)
+    assert isinstance((x+2).xreplace({x: K(i)*K(-i)}), TensAdd)
+
+    assert isinstance((x*2).subs({x: K(i)}), TensMul)
+    assert isinstance((x+2).subs({x: K(i)*K(-i)}), TensAdd)
+
+    assert isinstance((x*2).replace(x, K(i)), TensMul)
+    assert isinstance((x+2).replace(x, K(i)*K(-i)), TensAdd)


Code review comments:
diff_hunk: @@ -4861,3 +4861,23 @@ def _expand(expr, **kwargs):
         return expr._expand(**kwargs)
     else:
         return expr.expand(**kwargs)
+
+
+def get_postprocessor(cls):
+    def _postprocessor(expr):
+        tens_class = {Mul: TensMul, Add: TensAdd}[cls]
+        if any(isinstance(a, TensExpr) for a in expr.args):
+            return tens_class(*expr.args)
+        else:
+            return expr
+
+    return _postprocessor
+
+"""
+The following makes sure that if a user tries to put a tensor in a Mul, it
+automatically gets converted to a TensMul (see github issue #25051). The
+conversion already seems to work for TensAdd, so we don't do anything for that.
+"""
path: sympy/tensor/tensor.py
text: @user1:
Use # for comments. You also don't need to mention all these details in a comment. Just the first sentence here is enough, or even nothing at all, since people can just look up what the constructor postprocessor is in basic.py if they don't know what this code does. 

@author:
Okay, I've removed that comment.