Handling Integrals containing I

[Smit-create]

Handling Integrals containing I

References to other Issues or PRs

Fixes #17841

Brief description of what is fixed or changed

Added additional expand step in simplify method of ExpressionDomain

Other comments

Release Notes

• polys
  • Added expand in simplify method of ExpressionDomain

[sympy-bot]

✅

Hi, I am the SymPy bot (v149). I'm here to help you write a release notes entry. Please read the guide on how to write release notes.

Your release notes are in good order.

Here is what the release notes will look like:

• polys
  • Added expand in simplify method of ExpressionDomain (#18472 by @Smit-create)

This will be added to https://github.com/sympy/sympy/wiki/Release-Notes-for-1.6.

Note: This comment will be updated with the latest check if you edit the pull request. You need to reload the page to see it.

Click here to see the pull request description that was parsed.

<!-- 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 -->

Update

The release notes on the wiki have been updated.

[jksuom]

It seems that integral tests are sensitive to format changes in expressions. Polynomials with domain EX should be avoided if possible. This patch appears to help:

--- a/sympy/integrals/rationaltools.py
+++ b/sympy/integrals/rationaltools.py
@@ -212,7 +212,7 @@ def ratint_logpart(f, g, x, t=None):
     a, b = g, f - g.diff()*Poly(t, x)
 
     res, R = resultant(a, b, includePRS=True)
-    res = Poly(res, t, composite=False)
+    res = Poly(res, t)
 
     assert res, "BUG: resultant(%s, %s) can't be zero" % (a, b)

[jksuom]

It looks like the removal of composite=False makes the slow Wester test fail and has to be reverted. Actually, the code should work with different formats but some condition tests fail in the expression domain. You could try this fix:

--- a/sympy/polys/domains/expressiondomain.py
+++ b/sympy/polys/domains/expressiondomain.py
@@ -203,7 +203,7 @@ def is_positive(self, a):
 
     def is_negative(self, a):
         """Returns True if ``a`` is negative. """
-        return a.ex.as_coeff_mul()[0].is_negative
+        return a.ex.could_extract_minus_sign()
 
     def is_nonpositive(self, a):
         """Returns True if ``a`` is non-positive. """

[codecov]

Codecov Report

Merging #18472 into master will increase coverage by 0.021%.
The diff coverage is 100%.

@@             Coverage Diff              @@
##            master   #18472       +/-   ##
============================================
+ Coverage   75.319%   75.34%   +0.021%     
============================================
  Files          640      640               
  Lines       167117   167154       +37     
  Branches     39419    39428        +9     
============================================
+ Hits        125871   125934       +63     
+ Misses       35708    35680       -28     
- Partials      5538     5540        +2

[jksuom]

This seems to work. For the record, the current implementation is failing because the results of EX.is_negative() differ on non-expanded and expanded arguments.

>>> from sympy import *
>>> x = symbols('x')
>>> k, a0 = symbols('k a0', real=True)
>>> e = -(a0**4 + 2*a0**2*k**2 + k**4)/(2*a0**2 + 2)
>>> EX.is_negative(EX(e))
False
>>> EX.is_negative(EX(e.expand()))
False
>>> EX.is_negative(EX(-e))
True
>>> EX.is_negative(EX(-e.expand()))
False

This should be properly investigated.

[Smit-create]

. For the record, the current implementation is failing because the results of EX.is_negative() differ on non-expanded and expanded arguments.

On the current branch I get:

>>> from sympy import *
>>> x=symbols('x')
>>> k, a0 = symbols('k a0', real=True)
>>> e = -(a0**4 + 2*a0**2*k**2 + k**4)/(2*a0**2 + 2)
>>> EX.is_negative(EX(e))
True
>>> EX.is_negative(EX(e.expand()))
True
>>> EX.is_negative(EX(-e))
False
>>> EX.is_negative(EX(-e.expand()))
False

This seems to be producing expected results now.
@jksuom Am I correct?

[Smit-create]

@jksuom Is this ready to merge?

[jksuom]

Thanks, I think so. The comparison methods of EX domain seem very fragile, but I cannot think of a better fix now. I still believe that composite=False in rationaltools should be removed to avoid the EX domain but the reason for failure of test_W23 should be found first.

[Smit-create]

Yes, I will try to find it out. Thanks :)