Simplify before testing convergence

[AnimeshSinha1309]

Added simplification of function, before testing for convergence and added tests.

References to other Issues or PRs

Fixes #16699

Brief description of what is fixed or changed

Fixing issues with Sum of the factorial series, just added simplification of the expression before the tests for convergence start.

Release Notes

• concrete
  • simplify sums before testing for convergence

[sympy-bot]

✅

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

• concrete
  • simplify sums before testing for convergence (#16739 by @AnimeshSinha1309)

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

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.

Added simplification of function, before testing for convergence and added tests.

#### References to other Issues or PRs
Fixes #16699

#### Brief description of what is fixed or changed
Fixing issues with Sum of the factorial series, just added simplification of the expression before the tests for convergence start.

#### Release Notes

<!-- BEGIN RELEASE NOTES -->
* concrete
  * simplify sums before testing for convergence
<!-- END RELEASE NOTES -->

[smichr]

Is there a way to just return None if you know that simplification? Or if this is a high-level routine then maybe using simplify makes sense.

[maurogaravello]

I do not think this resolves the issue. This patch works in the specific example and probably is better than nothing.
If you consider

expr = 1 - cos(1/n)
s = Sum(expr, (n, 1, oo))

the result given by SymPy is False, even with the proposed patch, but the correct result is True.
This is due, as in the original example, to a non sharp result of order.

A possible alternative is to evaluate limit(expr * n, n, oo). In the case the result is a positive number (not 0), then the convergence is False. This is in substitution of the lines (445-446)

if p1_series_test[p] >= -1:
                return S.false 

[AnimeshSinha1309]

@smichr I did not fully understand what you said. Do you mean that if the call to simplify fails? When exactly should I return None.
I see that this patch is slowing things down and not solving the issue.
Also the following calls to the order function still don't give answer that make sense:

>>> print(O(factorial(n), (n, oo)))
O(factorial(n), (n, oo))
>>> print(O(factorial(n)/factorial(n-2), (n, oo)))
O(1, (n, oo))
>>> print(O(factorial(n)/factorial(n+2), (n, oo)))
O(1, (n, oo))

We would want that the factorial() calls all mapped to a result O(exp(n*log(n)), (n, oo)), because the factorial is bounded by (n/2)**n and (n/3)**n, right?

[AnimeshSinha1309]

So again, this function call is the problem here, right?

>>> n = Symbol('n')
>>> O(1-cos(n), (n, 0))
O(1)

The result should have been O(n^2), but that would require the Order function to have some understanding of the Taylor series expansions, won't it?

Also calling limit(expr * n) might just work, but even then all the calls to order don't really make much sense. 😢

[maurogaravello]

So again, this function call is the problem here, right?

>>> n = Symbol('n')
>>> O(1-cos(n), (n, 0))
O(1)

Exactly. This is the main problem.
O(1) here is correct, but not sharp!
O(n**-2) is also correct. This would produce the correct result.

[smichr]

I see that I had an incomplete thought in my suggestion. What it was intended to suggest is that maybe obtaining a meaningless O expression of something like that would allow you to know that you should return none of else use a limit or simplification.

[oscarbenjamin]

What's the status of this? @AnimeshSinha1309 @smichr

[AnimeshSinha1309]

Sorry, I did not follow up on this. My Attempts did not work out, I would appriciate any help I can get. I am available to work again, trying now.

[AnimeshSinha1309]

Honestly I am out of ideas, simple fixes to the is_convergent() method, or calling Simplify, fail on examples like 1-cos(1/n), and fixing the Order constructor is hard and I always end up breaking a bunch of tests. I am wondering if it would be okay to just fix a few cases, and leave fixing the Order function for another issue.

For now, this is my plan for fixing the limit comparison test in summations.py

### ------ limit-comparison with p-series ------ ###
if limit(sequence_term * sym, sym, oo) > 0:
    return False
if limit(sequence_term * (sym ** 1.0001), sym, oo) < oo:
    return True

[AnimeshSinha1309]

sympy/sympy/concrete/products.py

Lines 359 to 360 in 12d8f53

	>>> Product(n/(n + 1), (n, 1, oo)).is_convergent()
	False

Is this example not incorrect, since Product(n/(n + 1), (n, 1, oo)) = (1/2) * (2/3) * ... * (n-1/n) = 1/n = 1/oo = 0. So the answer should be True, it's convergent, should it not?

[jksuom]

An infinite product with limit 0 is considered divergent.

[codecov]

Codecov Report

Merging #16739 into master will increase coverage by 0.559%.
The diff coverage is 70.833%.

@@              Coverage Diff              @@
##            master    #16739       +/-   ##
=============================================
+ Coverage   73.922%   74.482%   +0.559%     
=============================================
  Files          620       623        +3     
  Lines       160243    161391     +1148     
  Branches     37600     37878      +278     
=============================================
+ Hits        118456    120208     +1752     
+ Misses       36305     35858      -447     
+ Partials      5482      5325      -157

[AnimeshSinha1309]

Thanks for that, I fixed my mistake, undid the example that I had changed. I am just confused about one more thing, I changed a few lines in Summation, and added 3 tests for it, why is my Codecov score still falling, should I just keep adding mode examples?

[oscarbenjamin]

It looks like this and everything below is untested. Are these tests (alternating series etc) not already implemented somewhere else?

[AnimeshSinha1309]

I don't fully understand, there are already tests for Alternating Series and for everything else that follows. I believe that the issue might be that my p-series limit comparison is just returning a solution, blocking access to some of the functions later in the code. But now the codecov score looks pretty decent, right? And if not, what is it that I fix, I have no clue about codecov?

[czgdp1807]

Closing as the associated issue is outdated.