Repository: sympy/sympy
Pull request: Handling Integrals containing I
<!-- Your title above should be a short description of what
was changed. Do not include the issue number in the title. -->
Handling Integrals containing I
#### 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 #17841

#### Brief description of what is fixed or changed
Added additional `expand` step in `simplify` method of `ExpressionDomain`

#### Other comments


#### Release Notes

<!-- Write the release notes for this release below. 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 -->
* polys
  * Added  `expand` in `simplify` method of `ExpressionDomain`
<!-- END RELEASE NOTES -->

Problem statement:
integrate throws error for rational functions involving I
Relatively simple integrals such as
```python
x = symbols('x')
f = diff(1/(x**2+x+I),x)
integrate(f,x)
```
fail with the error
```
sympy.polys.polyerrors.PolynomialDivisionFailed: couldn't reduce degree in a polynomial division algorithm when dividing [EX(-2), EX(-1)] by [EX(-(-15 + 8*I)*(15 + 8*I)/289)]. This can happen when it's not possible to detect zero in the coefficient domain. The domain of computation is EX. You may want to use a different simplification algorithm. Note that in general it's not possible to guarantee to detect zero in this domain.
```
The best workaround I have found so far is
```python
c = symbols('c',complex=True)
integrate(f.subs(I,c),x).subs(c,I)
```
but ``integrate(f,x,manual=True)`` also works.

Hints from issue discussion:
The presence of `I` makes the expression domain `EX` the coefficient domain of the denominator polynomial. This is problematic because `EX` has no reliable zero detection algorithm. The relevant polynomial division actually works correctly but the (unsimplified) coefficient expressions that are expected to be zero are not recognized as such, whence the error.

The expression domain has its own light-weight `simplify` method (the full `simplify` would be too expensive) but it does not cover this case. However, it seems that an addfitional `expand` step would suffice.
```
--- a/sympy/polys/domains/expressiondomain.py
+++ b/sympy/polys/domains/expressiondomain.py
@@ -45,7 +45,7 @@ def denom(f):
             return f.__class__(f.ex.as_numer_denom()[1])
 
         def simplify(f, ex):
-            return f.__class__(ex.cancel())
+            return f.__class__(ex.cancel().expand())
 
         def __abs__(f):
             return f.__class__(abs(f.ex))
```
In fact, not all default methods of `expand` are needed, `mul` alone would suffice in this case. This can be implemented by suitable keywords as follows, for example.
```
--- a/sympy/polys/domains/expressiondomain.py
+++ b/sympy/polys/domains/expressiondomain.py
@@ -9,6 +9,10 @@
 from sympy.polys.polyutils import PicklableWithSlots
 from sympy.utilities import public
 
+eflags = dict(mul=True, power_exp=False, power_base=False,
+              basic=False, multinomial=False, log=False)
+
+
 @public
 class ExpressionDomain(Field, CharacteristicZero, SimpleDomain):
     """A class for arbitrary expressions. """
@@ -45,7 +49,7 @@ def denom(f):
             return f.__class__(f.ex.as_numer_denom()[1])
 
         def simplify(f, ex):
-            return f.__class__(ex.cancel())
+            return f.__class__(ex.cancel().expand(**eflags))
 
         def __abs__(f):
             return f.__class__(abs(f.ex))
```
This method allows for more fine-grained control of simplification.
Better to just use expand_mul. If deep=False works too it should also use that. 
I'd prefer expand with explicit flags, including deep=False. expand_mul will do the same but it adds an extra function call. Besides, it may turn out that other expand methods will also be needed.
expand_mul already hard-codes those flags. And if somehow additional flags are added in the future it will stay correct. I wish we had made it so that mul=None is the default and mul=True disables the other flags.

What is the actual expression in question that needs to be expanded here? Calling expand_mul after cancel will expand the numerator over the denominator, and will generally result in a much larger expression. So we almost certainly don't want to do that.

Commit message: issue 17841

Patch to review:
diff --git a/sympy/integrals/tests/test_integrals.py b/sympy/integrals/tests/test_integrals.py
index 0179054d6ed6..2970b2cb7d70 100644
--- a/sympy/integrals/tests/test_integrals.py
+++ b/sympy/integrals/tests/test_integrals.py
@@ -1648,3 +1648,7 @@ def test_issue_2975():
     C = Symbol('C')
     y = Symbol('y')
     assert integrate(1/(y**2+C)**(S(3)/2), (y, -w/2, w/2)) == w/(C**(S(3)/2)*sqrt(1 + w**2/(4*C)))
+
+def test_issue_17841():
+    f = diff(1/(x**2+x+I), x)
+    assert integrate(f, x) == 1/(x**2 + x + I)
diff --git a/sympy/polys/domains/expressiondomain.py b/sympy/polys/domains/expressiondomain.py
index d770ac25dc6b..723a0b968711 100644
--- a/sympy/polys/domains/expressiondomain.py
+++ b/sympy/polys/domains/expressiondomain.py
@@ -9,6 +9,9 @@
 from sympy.polys.polyutils import PicklableWithSlots
 from sympy.utilities import public
 
+eflags = dict(mul=True, power_exp=False, power_base=False,
+              basic=False, multinomial=False, log=False)
+
 @public
 class ExpressionDomain(Field, CharacteristicZero, SimpleDomain):
     """A class for arbitrary expressions. """
@@ -45,7 +48,7 @@ def denom(f):
             return f.__class__(f.ex.as_numer_denom()[1])
 
         def simplify(f, ex):
-            return f.__class__(ex.cancel())
+            return f.__class__(ex.cancel().expand(**eflags))
 
         def __abs__(f):
             return f.__class__(abs(f.ex))


Code review comments:
diff_hunk: @@ -9,6 +9,9 @@
 from sympy.polys.polyutils import PicklableWithSlots
 from sympy.utilities import public
 
+eflags = dict(mul=True, power_exp=False, power_base=False,
path: sympy/polys/domains/expressiondomain.py
text: @user1:
The first attribute could be `deep=False` to save time. It is not necessary to inspect anything but the topmost node as this `simplify` method will be called repeatedly on each step.