Repository: sympy/sympy
Pull request: Fixed issue with piecewise integration
<!-- 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
Fixes #22533


#### Brief description of what is fixed or changed
Currently, `integrate(1/Abs(x))` (where x is real) gives `Piecewise((-log(x), x<= 0), (nan, True))`. The same issue could also occur for piecewise functions whose antiderivatives on some interval are infinite at one of the endpoints. I changed the `_eval_integral` method of sympy/functions/elementary/piecewise.py to detect such a situation and try to use the antiderivative expression instead of the running sum. With the changes I made, `integrate(1/Abs(x))` now returns `Piecewise((-log(x), x<= 0), (log(x), True))`. The change also gives the correct result for other piecewise functions for which this bug exists.

#### Other comments
None

#### 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.

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 -->
* functions
  * fixed a bug with integrating certain piecewise functions
<!-- END RELEASE NOTES -->


Problem statement:
Integral(1/abs(x), x).doit() returns an incomplete answer as output 
This was pointed out on the gitter channel a week back or two but I'm not sure if the user remembered to raise the issue on the github page and hence I am raising it before it gets lost in the chat !

On Master
```
>>> x = Symbol('x', real=True)
>>> Integral(1/abs(x), x).doit()
Piecewise((-log(x), x <= 0), (nan, True))
```
The correct answer for this should `sign(x)*log(abs(x))` for all points except 0 where it should be returning `nan` . I'm not sure where this might be going wrong due to my inexperience with the `Integrals` module but the output seems to capture one half of the number line correctly and returns `nan` for all other values of `x`

Hints from issue discussion:
I would love to work on this issue! However, I'm a newbie to sympy contributing so someone might have to guide me through it. I've been playing around with the code and am trying to locate the cause of the issue.
Cool you should go forward with it , though I'm myself inexperienced with the Integrals module so I guess I might not be of much help but surely you'll find people here who could help you with this ! 
Maybe @oscarbenjamin could get you started with some basic guidance for the same 

I think I have found the source of this problem. When you call integrate on `1/Abs(x)` it internally gets converted into a piecewise function. And by default, when a piecewise function is integrated, it calls the `_eval_integral` method, which tries to return an output so that evaluating the output at two different points and subtracting gives the definite integral between those two points. In this case, the definite integral of `1/Abs(x)` on any interval when both endpoints are negative is defined, so you get the correct answer for negative numbers. However, as soon as you cross over to positive numbers, the integral becomes infinite, causing the `nan` for positive numbers. You can get around this problem by rewriting `1/Abs(x)` as a piecewise function and using `piecewise_integrate`, but I still think something should be done about this, since this does not let you use the result of integrate to evaluate definite integrals where both limits are positive. 
Perhaps there is some way to automatically detect that an infinity has been produced in `_eval_integral` and fall back on `piecewise_integrate`?
It isn't generally possible to construct an antiderivative that can work for the fundamental theorem of calculus for all possible intervals because the theorem only applies if the function is continuous throughout the region which `1/Abs(x)` is not at `x=0`.
Is a fix even necessary for this issue then?
Perhaps, if the function has an infinite discontinuity on a particular interval, `eval_integral` should not try to make the result wok with the fundamental theorem of calculus. That way, the part of the result for positive numbers would be more helpful than `nan` while keeping the part for negative numbers preserved. 

Ideally an indefinite integral would at least give a locally valid antiderivative. In this case the antiderivative is not valid for positive x.
Yeah agreed . I hadn't given much thought about this, though I was initially convinced with what the user had reported in the gitter channel . Closing this as of now . Thank you !
Reopening: `integrate(1/Abs(x), x)` where `x` is a real symbol should give a result that is valid for positive `x`.
I replaced line 519 of functions/elementary/piecewise.py on master, which is 
```
args.append((sum, cond))
```
with 
```
if sum.has(oo, -oo, S.ComplexInfinity, Undefined) and not anti.has(oo, -oo, S.ComplexInfinity, Undefined):
    args.append((anti, cond))
else:
    args.append((sum, cond))
```
This seems to fix the issue. Essentially, what this does is that if `anti` evaluated on the current interval results results in a nan or an infinity (which is what causes `sum` to be `nan`), and anti itself does not contain `nan` or some type of infinity, it uses `anti` instead of `sum` for that interval. 
I ran the tests in bin/test and bin/doctest and all tests seem to be running OK. Is there something obviously wrong with this change or could it potentially cause any problems? Otherwise, would it be alright if I submitted a pull request for this issue? This is my first sympy contribution so I'm not quite sure what to do at this point.

Edit: With this change `integrate(1/Abs(x), x)` where x is real gives `Piecewise((-log(x), x <= 0), (log(x), True))`

 
You should maybe discuss your ideas or ask for suggestions here, but if you have a solution already consider creating a pull request and discussing the issue there ! 

People would also be able to review and acknowledge your changes whereas here people won't possibly be able to check if it is a valid solution or not .

Commit message: Piecewise: Fixed piecewise integration error

Before this change, when attempting to integrate a
piecewise function (or a function that internally
gets converted to a piecewise function
(like 1/Abs(x)), if some part of the
function on some interval evaluates to
some type of infinity at an endpoint of
the interval it could result in the indefinite
integral containing an added infinity or being nan
on that interval. This change fixes this issue.

Example

In [1]: from sympy import *

In [2]: x = Symbol('x', real=True)

In [3]: integrate(1/Abs(x))

Out [3]: Piecewise((-log(x), x <= 0), (log(x), True))

Before this change, integrate(1/Abs(x)) resulted in
Piecewise((-log(x), x <= 0), (nan, True))

Patch to review:
diff --git a/sympy/functions/elementary/piecewise.py b/sympy/functions/elementary/piecewise.py
index ce288fb916a4..e568b1e7c04e 100644
--- a/sympy/functions/elementary/piecewise.py
+++ b/sympy/functions/elementary/piecewise.py
@@ -512,7 +512,10 @@ def handler(ipw):
                 cond = (x < b)
             else:
                 cond = (x <= b)
-            args.append((sum, cond))
+            if sum.has(oo, -oo, S.ComplexInfinity, Undefined):
+                args.append((anti, cond))
+            else:
+                args.append((sum, cond))
         return Piecewise(*args)
 
     def _eval_interval(self, sym, a, b, _first=True):
diff --git a/sympy/functions/elementary/tests/test_piecewise.py b/sympy/functions/elementary/tests/test_piecewise.py
index 40bdeac4009f..f99f57576104 100644
--- a/sympy/functions/elementary/tests/test_piecewise.py
+++ b/sympy/functions/elementary/tests/test_piecewise.py
@@ -1447,3 +1447,9 @@ def test_piecewise_eval():
         ) == (x >= -3) & (x <= oo)
     assert f(Piecewise((x, (Abs(arg(a)) <= 1) | (Abs(arg(a)) < 1)))
         ) == (Abs(arg(a)) <= 1) | (Abs(arg(a)) < 1)
+
+
+def test_issue_22533():
+    x = Symbol('x', real=True)
+    f = Piecewise((-1 / x, x <= 0), (1 / x, True))
+    assert integrate(f, x) == Piecewise((-log(x), x <= 0), (log(x), True))


Code review comments:
diff_hunk: @@ -512,7 +512,10 @@ def handler(ipw):
                 cond = (x < b)
             else:
                 cond = (x <= b)
-            args.append((sum, cond))
+            if sum.has(oo, -oo, S.ComplexInfinity, Undefined):
path: sympy/functions/elementary/piecewise.py
text: @user1:
How about testing infinities on line 504? 

@author:
Testing infinities on line 504 does not catch infinities produced on line 506. I could change it so that it tests infinities right line 506 if that would help somehow.

@user2:
I'd suggest using `S.NegativeInfinity` instead of `-oo` to speed up the code (although both are very fast, an expression is created for the current code). An therefore also `S.Infinity` for consistency. What about `S.NaN`?

@user1:
If `sum` is infinite or undefined, then it should probably be reset to 0.

@author:
> If `sum` is infinite or undefined, then it should probably be reset to 0.

Yes, this seems like a good idea. I'll add it in along with oscargus's suggestion.

@author:
> I'd suggest using `S.NegativeInfinity` instead of `-oo` to speed up the code (although both are very fast, an expression is created for the current code). An therefore also `S.Infinity` for consistency. What about `S.NaN`?


I'll replace `oo` and `oo` with `S.NegativeInfinity` and `S.Infinity` respectively. In this file, `Undefined` is set to `S.NaN`, so `S.NaN` is already covered.