✅

Hi, I am the SymPy bot. 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:

• integrals
  • Fixed a bug with splitting of Piecewise functions. (#27396 by @rahulrangers)

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

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

#### Brief description of what is fixed or changed
Previously, the code incorrectly handled periodic functions like abs(cos(x)) when integration limits extended outside the interval
[0, T] (where T is the period). The issue was caused by splitting piecewise functions only within the domain [0, T]. As a result,
for limits outside this range, the integral evaluation returned incorrect results.

This commit fixes the issue by transforming the integration limits to (0, b) using the transform function.
The integration is then computed correctly using the formula:
∫(0 to b+nT) f(x) dx = ∫(0 to b) f(x) dx + n ∫(0 to T) f(x) dx.

**Key changes:**
- Added logic to detect periodicity using the `periodicity()` function.
- Transformed the integration limits for periodic functions to the range (0, b) to align with the formula.
- Properly handled integration direction and combined results for periodic segments and the remaining interval.

#### Other comments


#### 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. Formerly, `log(-x)` incorrectly gave `-log(x)`.

* physics.units
  * Corrected a semantical error in the conversion between volt and statvolt which
    reported the volt as being larger than the statvolt.

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 -->
* integrals
  * Fixed a bug with splitting of Piecewise functions. 

<!-- END RELEASE NOTES -->

I believe that this may fix issue #27185.

I don't think that this is the right fix because this is adding more code on top but there is no clear sign of where is the code that actually results in buggy output.

Somewhere the wrong function is being called that produces the wrong result. A fix here should identify where that is and should then make sure that the wrong function is either fixed or is not being called. I am not interesting in trying to fix this by adding more code on top without identifying where the actual problem is.

I don't think that this is the right fix because this is adding more code on top but there is no clear sign of where is the code that actually results in buggy output.

the bug is here which is a part of _eval_interval() :

the output of self.interval(x) gives back the function which is split in (0,period) intervel only, but the real problem comes when the limits are outside this function which is not handled in _eval_interval() function.

In [2]: integrate(Abs(cos(x)),(x,0,5*pi))
Out[2]: 4

The correct ans is 10 but we got incorrect answer 4 because the output of ok, abei = self._intervals(x) is
[(-oo, pi/2, sin(_Dummy_34), 0), (3*pi/2, 2*pi, sin(_Dummy_34), 0), (-oo, oo, -sin(_Dummy_34), 1)] so the function calculates correctly for (0, 2pi) and then for (2pi,5pi) it won't split the function and keeps it as -sinx in _clip() Line(407-422) part of eval_interval() function as seen above, which gives ans to be 4 for the limits (0,2pi) and 0 for the limits (2pi,5pi) which is supposed to be 6 in this interval so the result 4 is produced instead of 10

Somewhere the wrong function is being called that produces the wrong result. A fix here should identify where that is and should then make sure that the wrong function is either fixed or is not being called. I am not interesting in trying to fix this by adding more code on top without identifying where the actual problem is.

so I thought to tranform the function and use this formula ∫(0 to b+nT) f(x) dx = ∫(0 to b) f(x) dx + n ∫(0 to T) f(x) dx so that by just splitting the function correctly in (0,period) we get the correct result, so that we don't need to change anything in eval_interval() function.

@oscarbenjamin If the above solution is not ideal, could you please suggest alternative approach to address this issue?

The first thing needed is to prevent the _intervals method from returning the wrong result. Why does it return the wrong result?

Somewhere it is using some function that does not handle periodicity properly and either _intervals should not use that function or the function should be improved to handle periodicity properly.

The first thing needed is to prevent the _intervals method from returning the wrong result. Why does it return the wrong result?

the _intervels() function is somehow calling solve_univariate_inequality() function and in that function it is given that for trignometric equations the inequality are restricted in its periodic interval as mentioned below. this causes issue when the limits are outside the periodic interval.

Somewhere it is using some function that does not handle periodicity properly and either _intervals should not use that function or the function should be improved to handle periodicity properly.

So if we want to change _intervals function we need to somehow make a fix such that solve_univariate_inequality() function works properly for the interval consisting of limits instead of periodic interval but this degrades the performance for larger limits like (0,50pi) which can be easily computed using 50*(ans in the limits(0,pi)) if pi is the period using the formula.

Exactly so this is a bug:

In [1]: solve_univariate_inequality(cos(x) < 0, x)
Out[1]: 
π           3⋅π
─ < x ∧ x < ───
2            2 

Either that needs to be fixed or ._intervals must not use the solve_univariate_inequality function since this function produces mathematically incorrect results that cannot be depended on.

Exactly so this is a bug:

In [1]: solve_univariate_inequality(cos(x) < 0, x)
Out[1]: 
π           3⋅π
─ < x ∧ x < ───
2            2 

Either that needs to be fixed or ._intervals must not use the solve_univariate_inequality function since this function produces mathematically incorrect results that cannot be depended on.

1. is there any function which solves trignometric inequalties as replacement of solve_univariate_inequality ?
2. Why can't we use this formula ∫(0 to b+nT) f(x) dx = ∫(0 to b) f(x) dx + n ∫(0 to T) f(x) for now as using this formula we can solve these kind of integrals in periodic interval ? I understand it is mandatory to solve a bug which produces mathematically incorrect results but eventhough solving this bug still degrades the performance for larger limits like (0,50pi) so why can't we use the above formula which performs fast and give accurate results .

1. is there any function which solves trignometric inequalties as replacement of solve_univariate_inequality ?

I don't think that there is. Something could be added though or solve_univariate_inequality could be improved.

2. Why can't we use this formula ∫(0 to b+nT) f(x) dx = ∫(0 to b) f(x) dx + n ∫(0 to T) f(x) for now as using this formula we can solve these kind of inequalities in periodic interval ?

This formula is what ._intervals should be using but first it must not call solve_univariate_inequality. The problem with the code that you have added here is that it is in the wrong place. It should be in the ._intervals function and I don't think it should use the periodicity function.

This formula is what ._intervals should be using but first it must not call solve_univariate_inequality. The problem with the code that you have added here is that it is in the wrong place. It should be in the ._intervals function and I don't think it should use the periodicity function.

I think it is better if i change the code in _eval_interval instead _intervals as we are not even passing limits to _intervals so we won't be able to use the above formula and for now shall I let ._interval function use solve_univariate_inequality (it is not using this function directly though it is through _solve_inequality -> solve_univariate_inequality) as it is giving correct result for periodic interval and periodicity function is needed to find period which we will be using in the formula i think i can use that function in _eval_interval
if you agree for the above, i will change the code in _eval_interval and push the changes.

I think it is better if i change the code in _eval_interval instead _intervals as we are not even passing limits to _intervals so we won't be able to use the above formula

That seems reasonable.

for now shall I let ._interval function use solve_univariate_inequality

If that means that it will continue to return incorrect results then no: either integrate should not use ._interval or the bug in ._interval should be fixed.

It is important here not to just put more code on top of bad code. The bad code should be fixed or removed or not used any more. The bug needs to be fixed first and then we can add non-buggy code that makes it possible to handle things that were previously handled by the buggy code.

I am not interested in adding code in one place that tries to cancel a bug in another place while still leaving the bug there. The buggy code needs to be fixed so that each function only returns mathematically correct results. Putting new code on top of bad code without fixing the bugs makes everything more and more of a mess over time.

It is not possible to predict what intervals will be handled correctly by solve_univariate_inequality so any "fix" here that works by passing different inputs to solve_univariate_inequality is not fixing the actual bug.

If that means that it will continue to return incorrect results then no: either integrate should not use ._interval or the bug in ._interval should be fixed.

i mean ._interval would continue to split only for periodic interval but it would give correct ans for the definite integrations whose limits are outside the periodic intervals as the formula above just needs a correct split in periodic intervals.

It is important here not to just put more code on top of bad code. The bad code should be fixed or removed or not used any more. The bug needs to be fixed first and then we can add non-buggy code that makes it possible to handle things that were previously handled by the buggy code.

I agree that the buggy code should be fixed here first but here the solve_univariate_inequality() function is not entirely incorrect as it is giving accurate result for periodic interval. Are you expecting solve_univariate_inequality() to give Union(Interval.open(pi/2, 3*pi/2) + 2*n*pi, n in Integers) for cosx<0 ?

It is not possible to predict what intervals will be handled correctly by solve_univariate_inequality

No, solve_univariate_inequality() function correctly gives the result in periodic interval which solves the actual issue using the above formula

@oscarbenjamin can you tell me what were you expecting as the output of self._intervals is it to split the function correctly in the entire interval of limits correctly ? but to do this we need to change the arguments of the function to include the limits for intervals is it okay to change the arguments of ._interval function.

I agree that the buggy code should be fixed here first but here the solve_univariate_inequality() function is not entirely incorrect as it is giving accurate result for periodic interval.

Unless it is fixed so that it never returns incorrect results then it should not be used.

Are you expecting solve_univariate_inequality() to give Union(Interval.open(pi/2, 3*pi/2) + 2*n*pi, n in Integers) for cosx<0 ?

There isn't a way to represent that. Since solve_univariate_inequality returns a Boolean though it can just return cos(x) < 0.

is it okay to change the arguments of ._interval function.

Yes. I would probably remove the ._interval function altogether and replace it with a new function that does not use solve_univariate_inequality unless solve_univariate_inequality is going to be fixed.

I don't have confidence that either ._interval or solve_univariate_inequality is correct and not buggy. The new function can handle periodic inequalities explicitly rather than trying to depend on solve_univariate_inequality to do it since the output format of solve_univariate_inequality cannot really represent periodic inequalities.

Since solve_univariate_inequality returns a Boolean though it can just return cos(x) < 0.

It could also raise an exception like NotImplementedError. It is documented that solve_univariate_inequality may do this.

Yes. I would probably remove the ._interval function altogether and replace it with a new function that does not use solve_univariate_inequality unless solve_univariate_inequality is going to be fixed.

I don't have confidence that either ._interval or solve_univariate_inequality is correct and not buggy. The new function can handle periodic inequalities explicitly rather than trying to depend on solve_univariate_inequality to do it since the output format of solve_univariate_inequality cannot really represent periodic inequalities.

Then i will start creating a function in inequalities.py which handles periodic inequalities explicitly and evaluates answer through the above formula.
shall i make this function to only handle periodic inequalities and let other cases be handled by ._intervals function?

I think that the ._intervals function should be replaced by a different function that handles both periodic and non-periodic conditions. There should not be any code like:

if is_periodic:
   other_function()
else:
   _intervals()

The _intervals function either needs to be fixed to be always correct or should never be used.