Cleanup of integrate() #1643

Merged
merged 13 commits into from Nov 14, 2012

Projects

None yet

4 participants

@asmeurer
SymPy member

Here I am cleaning up integrate(). My goal is to integrate risch_integrate() with integrate(). I am also fixing some bugs that I come across along the way. I am still working on this, and I'll add a note here when this is finished, but if tests pass, this can be merged early.

@asmeurer asmeurer Expr._eval_interval is more robust against nan
Previously, it would just check if the result after substitution was nan.  But
nan doesn't always absorb into everything, which would lead to nonsensical
results (from integrate()) containing nan.  An example is issue 1227.

Note that we still do have to use subs, because limit() doesn't always do the
same thing as subs when the limit exists.  For example, limit on Piecewise
raises NotImplementedError.
8eab3aa
@asmeurer
SymPy member

Well, after integrating risch_integrate into integrate, test_integrals taks 99 seconds, whereas it used to take 114 seconds (I also added a couple of tests). Actually, it is now 40 seconds, but that's because I marked a particularly slow test as @slow.

asmeurer added some commits Nov 11, 2012
@asmeurer asmeurer Make Piecewise._eval_interval call Expr._eval_interval if one of the …
…limits is None

In this case, evaluation is just substitution.  This was "working" before only
because comparison between None and numbers is allowed in Python 2.
342b265
@asmeurer asmeurer Create and use NonElementaryIntegral in risch.py
Integrals that risch_integrate() has proven to be nonelementary are instances
of this class.  This is for now just an direct subclass of Integral (i.e., no
new functionality is added).  Later, it might contain some additional
information on the integral, such as why it is not elementary.

Also add option separate_integral to risch_integrate(), in preparation for
integrating risch_integrate() with integrate().
3c7db06
@asmeurer asmeurer "Integrate" risch_integrate() into integrate()
Previously, I was going to remove the part of the integrate heuristics that
breaks Integral(g1 + g2 + ...) into Integral(g1) + Integral(g2) + ..., but it
turns out this is a bad idea because many of the heuristics rely on this (for
example, trigintegrate() works on monomials).  Because risch_integrate() works
best on additions that have not been broken up, the strategy I employed was to
call risch_integrate() directly before this loop.  I also call
risch_integrate() again inside the loop, in case the integrand was a sum of an
elementary term and a non-elementary term (like erf(x) + exp(x)).
risch_integrate() is called in the loop after all the heuristics, but before
heurisch(), which should now be considered to be only a last resort.

By default, any NonElementaryIntegral returned from risch_integrate() is
recursively integrated using the other methods, in case it can be written in
terms of special functions.  To use only risch_integrate(), call
integrate(risch=True).  risch_integrate() has been removed from the user
namespace, though it can still be imported manually from
sympy.integrals.risch.  Note that integrate(risch=True) is not equivalent to
risch_integrate() in the sense that the former will return an unevaluated
Integral when it fails and the latter will raise NotImplementedError.
Therefore, you should check isinstance(result, NonElementaryIntegral) if you
want to verify that an unevaluated Integral result is indeed nonelementary.

Many tests had to be rewritten slightly, as risch_integrate() returned answers
in different forms than integrate().  Unfortunately, these are occasionally
worse, notably because risch_integrate() has a habit of returning things like
log(a*b) - log(a) instead of just log(b).

The good news is that for the integrals that risch_integrate() can handle,
integrate() is now orders of magnitude faster.

I also rewrote the grossly outdated and inaccurate docstring of
Integral._eval_integral.
cd42ae8
@asmeurer asmeurer Add some TODOs to the Risch code 092eb78
@asmeurer asmeurer Mark a slow integral test as @slow 242442d
@asmeurer asmeurer Fix a warning in the Risch code 47762bc
@asmeurer
SymPy member

OK, this is all I plan to do for tonight anyway. Chances are I won't get to doing more work on this for a while, so this should be considered as ready to review/merge.

@asmeurer
SymPy member

SymPy Bot Summary: 🔴 Failed after merging asmeurer/integrate-risch_integrate (a164cb7) into master (4055c4b).
@asmeurer: Please fix the test failures.
🔴 Python 2.5.0-final-0: fail
🔴 Python 2.6.6-final-0: fail
🔴 Python 2.7.2-final-0: fail
🔴 Python 2.6.8-final-0: fail
✳️ Python 2.7.3-final-0: pass
✳️ PyPy 1.9.1-dev-0; 2.7.3-final-42: pass
✳️ Python 3.2.2-final-0: pass
✳️ Python 3.3.0-final-0: pass
✳️ Python 3.2.3-final-0: pass
✳️ Python 3.3.0-final-0: pass
✳️ Python 3.3.0-final-0: pass
✳️Sphinx 1.1.3: pass

@Krastanov Krastanov and 1 other commented on an outdated diff Nov 11, 2012
sympy/integrals/integrals.py
+ because a case is not implemented yet), it continues on to the
+ next algorithms below. If the routine proves that the integrals
+ is nonelementary, it still moves on to the algorithms below,
+ because we might be able to find a closed-form solution in terms
+ of special functions. If risch=True, however, it will stop here.
+
+ 4. The Meijer G-Function algorithm:
+
+ - This algorithm works by first rewriting the integrand in terms of
+ very general Meijer G-Function (meijerg in SymPy), integrating
+ it, and then rewriting the result back, if possible. This
+ algorithm is particularly powerful for definite integrals (which
+ is actually part of a different method of Integral), since it can
+ compute closed-form solutions of definite integrals even when no
+ closed-form indefinite integral exists. But it also is capable
+ of computing many definite integrals as well.
@Krastanov
Krastanov Nov 11, 2012

'many definite' -> 'many indefinite' probably?

@Krastanov Krastanov and 1 other commented on an outdated diff Nov 11, 2012
sympy/solvers/ode.py
@@ -2030,13 +2030,13 @@ def ode_1st_homogeneous_coeff_subs_dep_div_indep(eq, func, order, match):
>>> f = Function('f')
>>> pprint(dsolve(2*x*f(x) + (x**2 + f(x)**2)*f(x).diff(x), f(x),
... hint='1st_homogeneous_coeff_subs_dep_div_indep', simplify=False))
- / 2 \
- | f (x)|
- /f(x)\ log|3 + -----|
- log|----| | 2 |
- /x \ \ x / \ x /
- log|--| + --------- + -------------- = 0
- \C1/ 3 3
+ / 3 \
+ |3*f(x) f (x)|
@Krastanov
Krastanov Nov 11, 2012

is there a missing indentation here (between the last two lines)?

@asmeurer
asmeurer Nov 11, 2012

Well, the doctest passes.

@asmeurer
asmeurer Nov 11, 2012

It was missing though. I guess the doctester's whitespace normalization makes this work.

@Krastanov
SymPy member

The errors in 2.6 and 2.5 are related to the new set literals syntax being unsupported in these versions, right?

@rlamy rlamy commented on the diff Nov 11, 2012
sympy/integrals/risch.py
+ elementary solutions, or, in the cases where it can prove the integral to
+ be nonelementary, instances of this class, use integrate(risch=True).
+ In this case, integrate() may raise NotImplementedError if it cannot make
+ such a determination.
+
+ integrate() uses the deterministic Risch algorithm to integrate elementary
+ functions or prove that they have no elementary integral. In some cases,
+ this algorithm can split an integral into an elementary and nonelementary
+ part, so that the result of integrate will be the sum of an elementary
+ expression and a NonElementaryIntegral.
+
+ Example
+ =======
+
+ >>> from sympy import integrate, exp, log, Integral
+ >>> from sympy.abc import x
@rlamy
rlamy Nov 11, 2012

May I complain, once again, about the use of dark magic and global state in docstrings?

@Krastanov
Krastanov Nov 11, 2012

Why is using the abc module bad? It is indeed global (as much as x=Symbol('x') is), however I do not see the black magic (maybe I just do not know the implementation).

But even if it contains magic, isn't the idea of the docstrings to show actual usage?

@asmeurer
asmeurer Nov 11, 2012

I was under the impression that abc was the standard way to get single letter symbols in docstrings. Was it changed?

@rlamy
rlamy Nov 11, 2012

It's bad because it hides the definition of x from the user. When I read this, I don't know what the assumptions on x are. Also, it reinforces the confusion between Python variables and sympy Symbols, which is a major problem for new users.

@asmeurer
asmeurer Nov 11, 2012

I've no major issue with doing it either way, or with changing every docstring to do it either way, but abc is currently standard (unless I missed the change) I don't think it should block this pull. Feel free to open a new PR changing it everywhere and we can discuss it there.

@rlamy
rlamy Nov 12, 2012

I don't think that we have a standard for this. If we have, I'd like to know where and when it was established, because I missed it.

Anyway, I'm not holding up the PR for this, it was more of a general remark.

@asmeurer
asmeurer Nov 12, 2012

It was discussed whenever it happened. So if you dig around, you can probably find it.

@rlamy
SymPy member

Using a tri-state flag in integrate() is annoying, and triples the amount of testing that should be done. If users want to call risch_integrate(), what's wrong with them just calling it?

@asmeurer
SymPy member

There is already a precedent set for meijerg. And anyway, I need some kind of flag to prevent recursion when integrating nonelementary integrals.

@asmeurer
SymPy member

FWIW, I agree that these flags complicate the code. I wasn't brave enough to attempt a major cleanup, mostly because the algorithm is somewhat complex, and any change to it would invariably result in an integral that can no longer be done. But it does need to happen, especially as we add more heuristics.

By the way, I was actually thinking of adding a third flag, heurisch, which would allow to disable the heurisch algorithm, since this is the only cause of hanging in integrate.

@asmeurer
SymPy member

I went ahead and removed the set literals. I also fixed my IPython config issue, so hopefully my SymPy Bot should pass completely now.

@rlamy
SymPy member

Well, using flags is consistent with existing code, so I don't mind it that much. Once this is in, I'll probably try to refactor/remove the flags, though.

@asmeurer
SymPy member

OK. How would you do it?

@rlamy
SymPy member
@asmeurer
SymPy member

I'd have to give you major kudos if you could clean up the meijerg stuff. I didn't have the stomach to even try to understand those heuristics.

@asmeurer
SymPy member

Here's the historical creation of abc: https://code.google.com/p/sympy/issues/detail?id=206. It appears it happend alongside var as a simple but non-hackish shortcut. This predates the change of the docstrings, though. I'm still searching for that.

@asmeurer
SymPy member

All I found was a brief mention on issue 1671 and a773472. I do remember this being discussed, though. I guess one motivation was that it's less boilerplate to just write

>>> from sympy.abc import x, y, z

instead of

>>> from sympy import symbols
>>> x, y, z = symbols("x y z")

I'll keep searching...

@asmeurer
SymPy member

There was this thread: https://groups.google.com/d/topic/sympy/jcAw0SkKgs0/discussion. I guess that was long after then change, though.

@asmeurer
SymPy member

Well, I can't find any more discussion. It turns out there's a big difference between archives existing and being able to effectively search those archives. It's possible the discussion happened on a GitHub diff (this was before pull requests). Or it's possible that I imagined the discussion.

At any rate, feel free to bring it up again. You have a good point about x = Symbol('x') being clearer to new users. As for abc itself, I think it should be kept, because from abc import * is useful for interactive use (and will be more useful if issue 1719 is fully implemented).

@asmeurer
SymPy member

SymPy Bot Summary: ✳️ Passed after merging asmeurer/integrate-risch_integrate (04d6b55) into master (4055c4b).
✳️ Python 2.5.0-final-0: pass
✳️ Python 2.6.6-final-0: pass
✳️ Python 2.7.2-final-0: pass
✳️ Python 2.6.8-final-0: pass
✳️ Python 2.7.3-final-0: pass
✳️ PyPy 1.9.1-dev-0; 2.7.3-final-42: pass
✳️ Python 3.2.2-final-0: pass
✳️ Python 3.3.0-final-0: pass
✳️ Python 3.2.3-final-0: pass
✳️ Python 3.3.0-final-0: pass
✳️ Python 3.3.0-final-0: pass
✳️Sphinx 1.1.3: pass

@jrioux
SymPy member

SymPy Bot Summary: ✳️ Passed after merging asmeurer/integrate-risch_integrate (04d6b55) into master (a655b03).
✳️ PyPy 1.9.1-dev-0; 2.7.3-final-42: pass
✳️ Python 2.7.2-final-0: pass
✳️ Python 3.2.1-final-0: pass
✳️ Sphinx 1.1.3: pass

@Krastanov
SymPy member

+1 from me. If no one complains I will merge this in 24 hours (or someone can merge it before me).

@Krastanov
SymPy member

This was fast... I was just going to merge this but the pull request is no longer mergeable.
@asmeurer, could you merge it to master?

@asmeurer asmeurer Merge branch 'master' into integrate-risch_integrate
Conflicts:
	sympy/integrals/tests/test_integrals.py
	sympy/integrals/tests/test_risch.py
	sympy/solvers/tests/test_ode.py
eac4557
@asmeurer
SymPy member

Merge was slightly nontrivial. Better wait for the tests.

@asmeurer
SymPy member

SymPy Bot Summary: ✳️ Passed after merging asmeurer/integrate-risch_integrate (eac4557) into master (72a7b9b).
✳️ Python 2.5.0-final-0: pass
✳️ Python 2.6.6-final-0: pass
✳️ Python 2.7.2-final-0: pass
✳️ Python 2.6.8-final-0: pass
✳️ Python 2.7.3-final-0: pass
✳️ PyPy 1.9.1-dev-0; 2.7.3-final-42: pass
✳️ Python 3.2.2-final-0: pass
✳️ Python 3.3.0-final-0: pass
✳️ Python 3.2.3-final-0: pass
✳️ Python 3.3.0-final-0: pass
✳️ Python 3.3.0-final-0: pass
✳️Sphinx 1.1.3: pass

@asmeurer asmeurer merged commit 4c5b0d4 into sympy:master Nov 14, 2012

1 check passed

Details default The Travis build passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment