Repository: sympy/sympy
Pull request: Implemented Sum((-1)**n/sqrt(n), (n, 1, oo)).is_absolutely_convergent()
Fixes #14112 .
Fixes #14152 .

This PR also introduces a new convergence test -- the **Ratio Test**. Actually, when I followed normalhuman's comment on the issue, I solved the particular issue, but I had another problem -

```
>>> from sympy import *
>>> n = symbols('n')
>>> a = Sum(factorial(n)/5**n, (n, 1, oo))
>>> a.is_convergent()
NotImplementedError : The algorithm to find the Sum convergence of 5**(-_n)*factorial(_n))**(1/_n) is not yet implemented.
```
While looking through the file, I then came to realize that the algorithm of **Ratio Test** hadn't been implemented, which can solve this problem easily. So, the code snippet of the Ratio_test algorithm has been implemented, and the issue fixed.

Test case has also been added.

Problem statement:
Sum((-1)**n/sqrt(n), (n, 1, oo)).is_absolutely_convergent() not implemented
```
>>> Sum((-1)**n/sqrt(n), (n, 1, oo)).is_absolutely_convergent()`
Traceback (most recent call last):

  File "<ipython-input-23-75eab3fe4b66>", line 1, in <module>
    Sum((-1)**n/sqrt(n), (n, 1, oo)).is_absolutely_convergent()

  File ".\sympy\concrete\summations.py", line 557, in is_absolutely_convergent
    return Sum(abs(self.function), self.limits).is_convergent()

  File ".\sympy\concrete\summations.py", line 428, in is_convergent
    order = O(sequence_term, (sym, S.Infinity))

  File ".\sympy\series\order.py", line 217, in __new__
    expr = expr.as_leading_term(*args)

  File ".\sympy\core\expr.py", line 2864, in as_leading_term
    obj = self._eval_as_leading_term(x)

  File ".\sympy\core\mul.py", line 1625, in _eval_as_leading_term
    return self.func(*[t.as_leading_term(x) for t in self.args])

  File ".\sympy\core\mul.py", line 1625, in <listcomp>
    return self.func(*[t.as_leading_term(x) for t in self.args])

  File ".\sympy\core\expr.py", line 2864, in as_leading_term
    obj = self._eval_as_leading_term(x)

  File ".\sympy\core\function.py", line 741, in _eval_as_leading_term
    '%s has no _eval_as_leading_term routine' % self.func)

NotImplementedError: Abs has no _eval_as_leading_term routine
```

ValueError in Sum((-1)**(2*n)/n, (n, 1, oo)).is_convergent()
```
var('n')
Sum((-1)**(2*n)/n, (n, 1, oo)).is_convergent()
```
results in
```
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/lvk/sympy/sympy/concrete/summations.py", line 489, in is_convergent
    maxima = solveset(sequence_term.diff(sym), sym, interval)
  File "/home/lvk/sympy/sympy/solvers/solveset.py", line 947, in solveset
    return _solveset(f, symbol, domain, _check=True)
  File "/home/lvk/sympy/sympy/solvers/solveset.py", line 763, in _solveset
    result += _solve_as_rational(equation, symbol, domain)
  File "/home/lvk/sympy/sympy/solvers/solveset.py", line 409, in _solve_as_rational
    valid_solns = _solveset(g, symbol, domain)
  File "/home/lvk/sympy/sympy/solvers/solveset.py", line 717, in _solveset
    result = Union(*[solver(m, symbol) for m in f.args])
  File "/home/lvk/sympy/sympy/solvers/solveset.py", line 717, in <listcomp>
    result = Union(*[solver(m, symbol) for m in f.args])
  File "/home/lvk/sympy/sympy/solvers/solveset.py", line 696, in <lambda>
    solver = lambda f, x, domain=domain: _solveset(f, x, domain)
  File "/home/lvk/sympy/sympy/solvers/solveset.py", line 736, in _solveset
    lhs, rhs_s = inverter(f, 0, symbol)
  File "/home/lvk/sympy/sympy/solvers/solveset.py", line 701, in <lambda>
    inverter = lambda f, rhs, symbol: inverter_func(f, rhs, symbol, domain)
  File "/home/lvk/sympy/sympy/solvers/solveset.py", line 122, in invert_real
    return _invert(f_x, y, x, domain)
  File "/home/lvk/sympy/sympy/solvers/solveset.py", line 104, in _invert
    x1, s = _invert_real(f_x, FiniteSet(y), x)
  File "/home/lvk/sympy/sympy/solvers/solveset.py", line 201, in _invert_real
    s, b = integer_log(rhs, base)
  File "/home/lvk/sympy/sympy/core/power.py", line 130, in integer_log
    raise ValueError('y cannot take value as 0')
ValueError: y cannot take value as 0
```

Hints from issue discussion:
@asmeurer @normalhuman I would like to fix this issue, can you guide me how to proceed! 
The method `is_absolutely_convergent` returns `Sum(abs(self.function), self.limits).is_convergent()` (incidentally, that should be Abs). The problem is this:
```
>>> a = (-1)**n / sqrt(n)
>>> Abs(a)
exp(-pi*im(n))*Abs(1/sqrt(n))
```
Even though we are talking about n being an index of a series, it is not treated as an integer (hence real) number. The logical place to enforce assumptions on the index is [here](https://github.com/sympy/sympy/blob/master/sympy/concrete/summations.py#L403), where the series is already reduced to positive-index form. Something like this:
```
sym_ = Dummy(sym.name, integer=True, positive=True)
sequence_term = sequence_term.xreplace({sym: sym_})
sym = sym_
```
should be enough. Then in the rest of computation goes as before. 

In the example given here, after that xreplace the terms will become `1/sqrt(n)` which is known to be divergent by the p-series test.
> (incidentally, that should be Abs)

Documentation of `Abs` says:
```
If you pass a SymPy expression to the built-in abs(), it will
pass it automatically to Abs().
```
So I don't think it is necessary to do so. Since `self.function` is going to be a SymPy object.
It's not necessary. But using SymPy functions in SymPy expressions is a natural thing to do, and it saves the round-trip to `abs` and back. 
Yes, that can be done. If you do so, then do it in other places in `is_convergent` method as well.
Just one more thing, I want to comment. With your suggested code-snippet, I guess the output error (in case it occurs), gives sequence term with dummy expression, like this:
```
In [4]: Sum(sin(n)/n, (n, 1, oo)).is_convergent()
---------------------------------------------------------------------------
NotImplementedError                       Traceback (most recent call last)
<ipython-input-4-a000bd8a6a22> in <module>()
----> 1 Sum(sin(n)/n, (n, 1, oo)).is_convergent()
NotImplementedError: The algorithm to find the Sum convergence of sin(_n)/_n is not yet implemented
```
Instead of simply `sin(n) / n` should be printed. I know it wouldn't be hard to do that, but I wanted to leave the comment here, in case I don't get to the PR when someone submits :)
I'm confident I could find a large number of cases where is_convergent() breaks down

is_convergent() seems to always break, I'm not sure what the problem is

Commit message: Added ratio test

Patch to review:
diff --git a/sympy/concrete/summations.py b/sympy/concrete/summations.py
index 2d6361e0a7c5..52f8825ec1d0 100644
--- a/sympy/concrete/summations.py
+++ b/sympy/concrete/summations.py
@@ -401,6 +401,10 @@ def is_convergent(self):
             lower_limit = -upper_limit
             upper_limit = S.Infinity
 
+        sym_ = Dummy(sym.name, integer=True, positive=True)
+        sequence_term = sequence_term.xreplace({sym: sym_})
+        sym = sym_
+
         interval = Interval(lower_limit, upper_limit)
 
         # Piecewise function handle
@@ -429,6 +433,15 @@ def is_convergent(self):
 
         order = O(sequence_term, (sym, S.Infinity))
 
+        ### ----------- ratio test ---------------- ###
+        next_sequence_term = sequence_term.xreplace({sym: sym + 1})
+        ratio = simplify(next_sequence_term/sequence_term)
+        lim_ratio = limit(ratio, sym, upper_limit)
+        if (lim_ratio) > 1:
+            return S.false
+        if (lim_ratio) < 1:
+            return S.true
+
         ### --------- p-series test (1/n**p) ---------- ###
         p1_series_test = order.expr.match(sym**p)
         if p1_series_test is not None:
@@ -526,6 +539,8 @@ def _dirichlet_test(g_n):
                 if dirich2 is not None:
                     return dirich2
 
+        _sym = self.limits[0][0]
+        sequence_term = sequence_term.xreplace({sym: _sym})
         raise NotImplementedError("The algorithm to find the Sum convergence of %s "
                                   "is not yet implemented" % (sequence_term))
 
diff --git a/sympy/concrete/tests/test_sums_products.py b/sympy/concrete/tests/test_sums_products.py
index 21534f0f1c93..4f663f4d7862 100644
--- a/sympy/concrete/tests/test_sums_products.py
+++ b/sympy/concrete/tests/test_sums_products.py
@@ -936,7 +936,6 @@ def test_is_convergent():
 
     # root test --
     assert Sum((-12)**n/n, (n, 1, oo)).is_convergent() is S.false
-    assert Sum(2**n/factorial(n), (n, 1, oo)).is_convergent() is S.true
 
     # integral test --
 
@@ -1010,3 +1009,7 @@ def test_issue_10156():
     e = 2*y*Sum(2*cx*x**2, (x, 1, 9))
     assert e.factor() == \
         8*y**3*Sum(x, (x, 1, 3))*Sum(x**2, (x, 1, 9))
+
+
+def test_issue_14112():
+    assert Sum((-1)**n/sqrt(n), (n, 1, oo)).is_absolutely_convergent() is S.false


Code review comments:
diff_hunk: @@ -429,6 +433,15 @@ def is_convergent(self):
 
         order = O(sequence_term, (sym, S.Infinity))
 
+        ### ----------- ratio test ---------------- ###
+        next_sequence_term = sequence_term.xreplace({sym: sym + 1})
+        ratio = simplify(next_sequence_term/sequence_term)
path: sympy/concrete/summations.py
text: @user1:
simplify should be avoided in favor of more specific operations, as [its own docstring](http://docs.sympy.org/latest/modules/simplify/simplify.html#sympy.simplify.simplify.simplify) says. Here you want to cancel powers of n (that's done by `powsimp`) and the factorials (with `combsimp`). So, use `combsimp(powsimp(next_sequence_term/sequence_term))`

diff_hunk: @@ -429,6 +433,15 @@ def is_convergent(self):
 
         order = O(sequence_term, (sym, S.Infinity))
 
+        ### ----------- ratio test ---------------- ###
+        next_sequence_term = sequence_term.xreplace({sym: sym + 1})
+        ratio = simplify(next_sequence_term/sequence_term)
+        lim_ratio = limit(ratio, sym, upper_limit)
+        if (lim_ratio) > 1:
+            return S.false
+        if (lim_ratio) < 1:
path: sympy/concrete/summations.py
text: @user1:
Is this test for series with positive terms only? Consider something like `sequence_term = (-1)**n`.

@author:
`sequence_term = (-1)**n` will be caught by the 'Divergence test` code snippet.
And yes, this is intended for positive terms only.

@user1:
There should probably be a comment explaining the assumptions for this test for future maintenance. 

@author:
I''ll add a comment about the test.
Are there any problems in the other changes?

@user2:
If the ratio is -2, the series diverges but the test will declare it convergent. The limit should be taken of `Abs(next_sequence_term/sequence_term)`

@user2:
For a concrete example, here is a wrong result we get because of incorrect ratio test implementation.
```
>>> Sum((-2)**n + (-3)**n, (n, 1, oo)).is_convergent()
True
```