Repository: sympy/sympy
Pull request: Prevent RecursionError in nc_simplify for floating point coefficients
<!-- 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 #26556

#### Brief description of what is fixed or changed
Allow simplification of expressions with floating point coefficients for non-commutative symbols 

#### 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. 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 -->
* simplify
   * Prevent RecursionError when simplifying expressions with floating-point coefficients on a non-commutative symbol
<!-- END RELEASE NOTES -->


Problem statement:
RecursionError in simplify of non-commutative expressions with non-integer coefficient
Simplifying expressions with non-commutative symbols and floating point coeefficients generates a recursion error.

```python
from sympy import Symbol, cos, pi
X = Symbol('X', commutative = False)

try:
    display( (2 * cos(pi/3) * X).simplify() )
except RecursionError as e:
    print(f'RecursionError 2*cos(pi/3): {e}')

try:
    display( (2.0 * cos(pi/3) * X).simplify() )
except RecursionError as e:
    print(f'RecursionError 2.0*cos(pi/3): {e}')

try:
    display( (2.0 * cos(pi/6) * X).simplify() )
except RecursionError as e:
    print(f'RecursionError 2.0*cos(pi/6): {e}')
```
outputs:

X
RecursionError 2.0*cos(pi/3): maximum recursion depth exceeded in comparison
RecursionError 2.0*cos(pi/6): maximum recursion depth exceeded in comparison

Hints from issue discussion:
The check here fails if `com_coeff = 1.0`:
https://github.com/sympy/sympy/blob/7c4f580838612edea6dcd6ae7d5b8a75af2d8f10/sympy/simplify/simplify.py#L1761-L1762

Commit message: Add name to mailmap file

Patch to review:
diff --git a/.mailmap b/.mailmap
index 4c0988c98177..0303e3b9fb17 100644
--- a/.mailmap
+++ b/.mailmap
@@ -1616,6 +1616,7 @@ richierichrawr <richierichrawr@users.noreply.github.com>
 rimibis <33387803+rimibis@users.noreply.github.com>
 risubaba <risubhjain1010@gmail.com>
 ritikBhandari <ritikbhandari68@gmail.com>
+rrodenbusch <rrodenbusch@gmail.com>
 rushyam <rushyamsonu@gmail.com>
 sachinSingh16-09 <sachinishu02@gmail.com>
 samithkavishke <55777141+samithkavishke@users.noreply.github.com>
diff --git a/sympy/simplify/simplify.py b/sympy/simplify/simplify.py
index c86eb1500131..af351a3c8478 100644
--- a/sympy/simplify/simplify.py
+++ b/sympy/simplify/simplify.py
@@ -9,7 +9,7 @@
 from sympy.core.parameters import global_parameters
 from sympy.core.function import (expand_log, count_ops, _mexpand,
     nfloat, expand_mul, expand)
-from sympy.core.numbers import Float, I, pi, Rational
+from sympy.core.numbers import Float, I, pi, Rational, equal_valued
 from sympy.core.relational import Relational
 from sympy.core.rules import Transform
 from sympy.core.sorting import ordered
@@ -1758,7 +1758,7 @@ def compare(s, alt_s):
         # get the non-commutative part
         c_args, args = expr.args_cnc()
         com_coeff = Mul(*c_args)
-        if com_coeff != 1:
+        if not equal_valued(com_coeff, 1):
             return com_coeff*nc_simplify(expr/com_coeff, deep=deep)
 
     inv_tot, args = _reduce_inverses(args)
diff --git a/sympy/simplify/tests/test_simplify.py b/sympy/simplify/tests/test_simplify.py
index a26e8e33a2eb..f4392b669375 100644
--- a/sympy/simplify/tests/test_simplify.py
+++ b/sympy/simplify/tests/test_simplify.py
@@ -1080,3 +1080,8 @@ def test_reduce_inverses_nc_pow():
     x, y = symbols("x y", positive=True)
     assert expand((x*y)**Z) == x**Z * y**Z
     assert simplify(x**Z * y**Z) == expand((x*y)**Z)
+
+def test_nc_recursion_coeff():
+    X = symbols("X", commutative = False)
+    assert (2 * cos(pi/3) * X).simplify() == X
+    assert (2.0 * cos(pi/3) * X).simplify() == X


Code review comments:
diff_hunk: @@ -1616,6 +1616,7 @@ richierichrawr <richierichrawr@users.noreply.github.com>
 rimibis <33387803+rimibis@users.noreply.github.com>
 risubaba <risubhjain1010@gmail.com>
 ritikBhandari <ritikbhandari68@gmail.com>
+rrodenbusch <rrodenbusch@gmail.com>
path: .mailmap
text: @user2:
Usually the idea is to add a .mailmap entry like:
```
Richard Rodenbusch <rrodenbusch@user1.com> rrodenbusch <rrodenbusch@user1.com>
```
The first name and email address is what will eventually go in the AUTHORS file. The second is what is recorded in the commit metadata. This line in the .mailmap file associates the author name with the corresponding commits.

There are a number of entries not like that in the .mailmap file that mostly come from people who have misunderstood what these entries are intended to do.

@author:
I will update my entry.  The entry in the documentation for this process mentions the dual entry format after the step to commit the change to .mailmap.   Perhaps we should re-order that?



@user2:
Maybe. I'm not sure which part you are referring to. Is it this:
https://docs.sympy.org/dev/contributing/new-contributors-guide/workflow-process.html#add-your-name-and-email-address-to-the-mailmap-file

@author:
Yes.  That is the document I was following.  I'd tweak the document so it shows the two entry option in the initial section, then  links to the section below with the more detailed description on the entries. I'm attaching changes here so you can look at it before I add it to the PR.

### _first section_

The first time you make a pull request you will need to add your name and email address to the .mailmap file by adding a line like

```
Joe Bloggs <joe@user3.com>  joeb <joe@user3.com>
```

This line in the .mailmap file associates the author name with the corresponding commits. The first name and email address is what will eventually go in the AUTHORS file. The second entry is what is recorded in the commit metadata. (see [Mapping user names to AUTHORS file entry](#mailmap-mapping-names))

The commit metadata name and email should exactly match the name and email that you have configured with git before making the commits (see "Configure git settings" above). The `bin/mailmap_check.py` script can check that this has been done correctly. 


### _then_ _below_

(mailmap-mapping-names)=
### Mapping user names to AUTHORS file entry
Sometimes a commit will be made with an incorrect name or email address or an author will make multiple commits with different names and email addresses or an author wishes to use a proper name that differs from their github name. In this case a line should be added to the .mailmap file where the first name and email address is what should be recorded in the AUTHORS file and the others are the name and email address that was incorrectly used in the other commits.


### _the updated md_
[workflow-process.md](https://github.com/sympy/sympy/files/15168077/workflow-process.md)


@user2:
Yes, maybe that is better.

The premise of the way it is written currently is that you will configure git to have the right name and email address first:
```console
$ git config user.name
Oscar Benjamin
```
It seems that it refers to "configure git settings" that is not present in the document though.

The two entry version is only needed if the name configured in git is not the name that should be in the authors file. It seems to be very common though that the name and/or email in the commits is not the right one for the authors file so it is probably more common to need the two entry version. I agree that
```
Joe Bloggs <joe@user3.com>  joeb <joe@user3.com>
```
is probably the best thing to suggest by default.

@author:
I found 'configure git settings'  in dev-setup.md and added the links.   Do I add these changes be added to this PR or should I generate a new one?

[workflow-process.md](https://github.com/sympy/sympy/files/15168941/workflow-process.md)

[dev-setup.md](https://github.com/sympy/sympy/files/15168959/dev-setup.md)



@user2:
I'd say go with a new PR for the workflow changes.

@user2:
In the new PR you won't have the problems because you are already in the .mailmap file (unless you change your git config...).