Doctest conversion from SymPy of unevaluated integrals

[eviatarbach]

When SymPy can't evaluate an integral, such as integrate((log(x)*log(log(x))), x, algorithm='sympy'), it returns "AttributeError: 'Integral' object has no attribute 'sage'". It should just return an unevaluated integral, the way it does when Maxima is used.

Another example from #15256:

sage: a = integrate(sin(x)*tan(x), x, algorithm='sympy') 
...

 
/usr/local/sage/sage-5.11/local/lib/python2.7/site-packages/sage/symbolic/integration/external.pyc in sympy_integrator(expression, v, a, b)
     37     else:
     38         result = sympy.integrate(ex, (v, a._sympy_(), b._sympy_()))
---> 39     return result._sage_()
     40
     41 def mma_free_integrator(expression, v, a=None, b=None):
 
AttributeError: 'Integral' object has no attribute '_sage_'
sage: 
sage: 
sage: %debug
> /usr/local/sage/sage-5.11/local/lib/python2.7/site-packages/sage/symbolic/integration/external.py(39)sympy_integrator()
     38         result = sympy.integrate(ex, (v, a._sympy_(), b._sympy_()))
---> 39     return result._sage_()
     40 
 
ipdb> print result
Integral(sin(x)*tan(x), x)

Depends on #20204

CC: @kcrisman @asmeurer @williamstein

Component: calculus

Keywords: sympy, integrate

Author: Ralf Stephan

Branch/Commit: 119f46f

Reviewer: Travis Scrimshaw

Issue created by migration from https://trac.sagemath.org/ticket/14723

[eviatarbach]

comment:1

Attachment: 14723.patch.gz

With the patch, integrate(log(x)*log(log(x)), x) is returned.

[asmeurer]

comment:7

Just looking at the patch, that doesn't look like it does the right thing for definite integrals.

[kcrisman]

comment:10

This is a dup of #15256. Not sure which one should be closed.

[rwst]

Work Issues: handle definite integrals too

[rwst]

Changed keywords from none to sympy, integrate

[rwst]

comment:13

The original case is now working in sympy:

sage: integrate(sin(x)*tan(x), x, algorithm='sympy') 
1/2*log(sin(x) + 1) - 1/2*log(sin(x) - 1) - sin(x)

so we need a more complicated one:

sage: integrate(sin(x)*tan(x)/(1-cos(x)), x, algorithm='sympy')
integrate(-sin(x)*tan(x)/(cos(x) - 1), x)

However, comment:7 is right regarding definite integrals:

sage: integrate(sin(x)*tan(x)/(1-cos(x)), x, a, b, algorithm='sympy')
integrate(-sin(x)*tan(x)/(cos(x) - 1), x)

[rwst]

Author: Eviatar Bach

[rwst]

comment:14

It is a matter of either

• adding a _sage_ method to the upstream sympy code in sympy/integrals/integrals.py or
• catching the exception in sage.symbolic.integration.external.sympy_integrator()

EDIT: The problem with the latter is, all sorts of elementary sympy functions have a _sage_ method, so why not add one to sympy.Integral or even do a bulk upgrade of sympy (e.g. gamma._sage_ is missing too)?

Actually, this is sympy/sympy#3444

[rwst]

Changed work issues from handle definite integrals too to fix in sympy

[rwst]

Upstream: Reported upstream. Developers acknowledge bug.

[kcrisman]

comment:16

Eventually we may want to start looking into switching to SymPy for the default integration method, or possibly trying both by default (not sure how long that would take, though). Before that, we'd probably want to work with #7763, though - and what about #2787? I personally don't know that I like deprecating it, but all of this is part of the same package of taking advantage of the capabilities out there by having a more consistent interface.

[rwst]

Changed upstream from Reported upstream. Developers acknowledge bug. to Completely fixed; Fix reported upstream

[rwst]

comment:17

This Sympy patch completely fixes the issue. A pull request containing it was reported there and waits for review.

sympy/sympy#8592

diff --git a/sympy/integrals/integrals.py b/sympy/integrals/integrals.py
index 2a3ccfe..ac434c5 100644
--- a/sympy/integrals/integrals.py
+++ b/sympy/integrals/integrals.py
@@ -1070,6 +1070,27 @@ def as_sum(self, n, method="midpoint"):
             result += self.function.subs(sym, xi)
         return result*dx
 
+    def _sage_(self, ):
+        from sage.symbolic.integration.integral import definite_integral, indefinite_integral
+        f, limits = self.function, list(self.limits)
+        limit = limits.pop(-1)
+        if len(limit) >= 2:
+            if len(limit) == 2:
+                x, b = limit
+                a = None
+            else:
+                x, a, b = limit
+            return definite_integral(f._sage_(),
+                                        x._sage_(),
+                                        a._sage_(),
+                                        b._sage_(),
+                                        hold=True)
+        else:
+            x = limit[0]
+            return indefinite_integral(f._sage_(),
+                                       x._sage_(),
+                                       hold=True)
+
 
 @xthreaded
 def integrate(*args, **kwargs):

[rwst]

Changed dependencies from #20815 to #20185

[rwst]

Changed dependencies from #20185 to #20204

[rwst]

Changed upstream from Completely fixed; Fix reported upstream to None of the above - read trac for reasoning.

[rwst]

comment:85

Summary:

The Sympy patch of this branch contains two hunks, one patches integral.py, the other function.py. The latter one adds a _sage_ method to AppliedUndef but uses a deprecated call method. The added _sage_ method is already there in sympy-1 but gets deleted by a patch in #20185 because it triggers the same error in #20185 as Marc reported here (I was too stupid to rebuild Sympy to see it). So the problem actually exists. However, without the patch this branch does not work.

The first part (the one for integral.py) is no longer necessary because it is in Sympy already.

So what needs to be done is to fix in Sympy and/or Sage the described error. It is investigated in #20185 and its dedicated ticket is #20204, so we depend on that.

[tscrim]

comment:86

While trying to debug #23496, I noticed that assumptions are lost going to/from Sympy and Sage, which was causing different Sympy Symbols to be created. This may cause some subtle problems at some point if we just work around the problem on #23496 instead of fixing it.

[rwst]

Changed upstream from None of the above - read trac for reasoning. to none

[rwst]

Changed author from Eviatar Bach, Ralf Stephan to none

[rwst]

Changed branch from u/rws/i14723 to none

[rwst]

Changed commit from 6c74620 to none

[rwst]

Changed reviewer from Ralf Stephan, Karl-Dieter Crisman to none

[rwst]

comment:87

All conversion problems were resolved in 8.1.rc0. The problematic integrals in this tickets, including the triple one, work now without any additional code. What remains for this ticket is to add doctests.

[rwst]

Branch: u/rws/14723-1

[rwst]

New commits:

119f46f

14723: Doctest conversion from SymPy of unevaluated integrals

[rwst]

Author: Ralf Stephan

[rwst]

Commit: 119f46f

[tscrim]

Reviewer: Travis Scrimshaw

[tscrim]

comment:90

LGTM.

[vbraun]

Changed branch from u/rws/14723-1 to 119f46f