complex_root_of uses inexact index

[jdemeyer]

When numerically evaluating complex_root_of(a, k), the index k is passed as floating-point number:

sage: complex_root_of(x^8-1, 7).n(2)
---------------------------------------------------------------------------
IndexError                                Traceback (most recent call last)
<ipython-input-1-4eba84b7c14c> in <module>()
----> 1 complex_root_of(x**Integer(8)-Integer(1), Integer(7)).n(Integer(2))

/home/patchbot/sage-patchbot/src/sage/structure/element.pyx in sage.structure.element.Element.n (build/cythonized/sage/structure/element.c:8131)()
    863             0.666666666666667
    864         """
--> 865         return self.numerical_approx(prec, digits, algorithm)
    866 
    867     N = deprecated_function_alias(13055, n)

/home/patchbot/sage-patchbot/src/sage/symbolic/expression.pyx in sage.symbolic.expression.Expression.numerical_approx (build/cythonized/sage/symbolic/expression.cpp:36209)()
   5818         kwds = {'parent': R, 'algorithm': algorithm}
   5819         try:
-> 5820             x = x._convert(kwds)
   5821         except TypeError: # numerical approximation for real number failed
   5822             pass          # try again with complex

/home/patchbot/sage-patchbot/src/sage/symbolic/expression.pyx in sage.symbolic.expression.Expression._convert (build/cythonized/sage/symbolic/expression.cpp:10663)()
   1257         sig_on()
   1258         try:
-> 1259             res = self._gobj.evalf(0, kwds)
   1260         finally:
   1261             sig_off()

/home/patchbot/sage-patchbot/local/lib/python2.7/site-packages/sage/functions/other.pyc in _evalf_(self, poly, index, parent, algorithm)
   2918         except AttributeError:
   2919             prec = 53
-> 2920         sobj = CRootOf(Poly(poly._sympy_()), int(index))
   2921         return sobj.n(ceil(prec*3/10))._sage_()
   2922 

/home/patchbot/sage-patchbot/local/lib/python2.7/site-packages/sympy/polys/rootoftools.pyc in __new__(cls, f, x, index, radicals, expand)
    130         if index < -degree or index >= degree:
    131             raise IndexError("root index out of [%d, %d] range, got %d" %
--> 132                              (-degree, degree - 1, index))
    133         elif index < 0:
    134             index += degree

IndexError: root index out of [-8, 7] range, got 8

When gmpy2 is installed, the index is even passed as complex number:

sage -t --long src/sage/functions/other.py
**********************************************************************
File "src/sage/functions/other.py", line 2862, in sage.functions.other.Function_crootof
Failed example:
    c.n()
Exception raised:
    Traceback (most recent call last):
      File "/home/patchbot/sage-patchbot/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 517, in _run
        self.compile_and_execute(example, compiler, test.globs)
      File "/home/patchbot/sage-patchbot/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 920, in compile_and_execute
        exec(compiled, globs)
      File "<doctest sage.functions.other.Function_crootof[1]>", line 1, in <module>
        c.n()
      File "sage/structure/element.pyx", line 865, in sage.structure.element.Element.n (build/cythonized/sage/structure/element.c:8131)
        return self.numerical_approx(prec, digits, algorithm)
      File "sage/symbolic/expression.pyx", line 5824, in sage.symbolic.expression.Expression.numerical_approx (build/cythonized/sage/symbolic/expression.cpp:36286)
        x = x._convert(kwds)
      File "sage/symbolic/expression.pyx", line 1259, in sage.symbolic.expression.Expression._convert (build/cythonized/sage/symbolic/expression.cpp:10663)
        res = self._gobj.evalf(0, kwds)
      File "/home/patchbot/sage-patchbot/local/lib/python2.7/site-packages/sage/functions/other.py", line 2920, in _evalf_
        sobj = CRootOf(Poly(poly._sympy_()), int(index))
      File "sage/rings/complex_number.pyx", line 1058, in sage.rings.complex_number.ComplexNumber.__int__ (build/cythonized/sage/rings/complex_number.c:10918)
        raise TypeError("can't convert complex to int; use int(abs(z))")
    TypeError: can't convert complex to int; use int(abs(z))
**********************************************************************
File "src/sage/functions/other.py", line 2864, in sage.functions.other.Function_crootof
Failed example:
    c.n(100)
Exception raised:
    Traceback (most recent call last):
      File "/home/patchbot/sage-patchbot/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 517, in _run
        self.compile_and_execute(example, compiler, test.globs)
      File "/home/patchbot/sage-patchbot/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 920, in compile_and_execute
        exec(compiled, globs)
      File "<doctest sage.functions.other.Function_crootof[2]>", line 1, in <module>
        c.n(Integer(100))
      File "sage/structure/element.pyx", line 865, in sage.structure.element.Element.n (build/cythonized/sage/structure/element.c:8131)
        return self.numerical_approx(prec, digits, algorithm)
      File "sage/symbolic/expression.pyx", line 5824, in sage.symbolic.expression.Expression.numerical_approx (build/cythonized/sage/symbolic/expression.cpp:36286)
        x = x._convert(kwds)
      File "sage/symbolic/expression.pyx", line 1259, in sage.symbolic.expression.Expression._convert (build/cythonized/sage/symbolic/expression.cpp:10663)
        res = self._gobj.evalf(0, kwds)
      File "/home/patchbot/sage-patchbot/local/lib/python2.7/site-packages/sage/functions/other.py", line 2920, in _evalf_
        sobj = CRootOf(Poly(poly._sympy_()), int(index))
      File "sage/rings/complex_number.pyx", line 1058, in sage.rings.complex_number.ComplexNumber.__int__ (build/cythonized/sage/rings/complex_number.c:10918)
        raise TypeError("can't convert complex to int; use int(abs(z))")
    TypeError: can't convert complex to int; use int(abs(z))
**********************************************************************
File "src/sage/functions/other.py", line 2866, in sage.functions.other.Function_crootof
Failed example:
    (c^6 + c + 1).n(100) < 1e-25
Exception raised:
    Traceback (most recent call last):
      File "/home/patchbot/sage-patchbot/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 517, in _run
        self.compile_and_execute(example, compiler, test.globs)
      File "/home/patchbot/sage-patchbot/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 920, in compile_and_execute
        exec(compiled, globs)
      File "<doctest sage.functions.other.Function_crootof[3]>", line 1, in <module>
        (c**Integer(6) + c + Integer(1)).n(Integer(100)) < RealNumber('1e-25')
      File "sage/structure/element.pyx", line 865, in sage.structure.element.Element.n (build/cythonized/sage/structure/element.c:8131)
        return self.numerical_approx(prec, digits, algorithm)
      File "sage/symbolic/expression.pyx", line 5824, in sage.symbolic.expression.Expression.numerical_approx (build/cythonized/sage/symbolic/expression.cpp:36286)
        x = x._convert(kwds)
      File "sage/symbolic/expression.pyx", line 1259, in sage.symbolic.expression.Expression._convert (build/cythonized/sage/symbolic/expression.cpp:10663)
        res = self._gobj.evalf(0, kwds)
      File "/home/patchbot/sage-patchbot/local/lib/python2.7/site-packages/sage/functions/other.py", line 2920, in _evalf_
        sobj = CRootOf(Poly(poly._sympy_()), int(index))
      File "sage/rings/complex_number.pyx", line 1058, in sage.rings.complex_number.ComplexNumber.__int__ (build/cythonized/sage/rings/complex_number.c:10918)
        raise TypeError("can't convert complex to int; use int(abs(z))")
    TypeError: can't convert complex to int; use int(abs(z))
**********************************************************************
File "src/sage/functions/other.py", line 2908, in sage.functions.other.Function_crootof._evalf_
Failed example:
    complex_root_of(x^2-2, 1).n()
Exception raised:
    Traceback (most recent call last):
      File "/home/patchbot/sage-patchbot/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 517, in _run
        self.compile_and_execute(example, compiler, test.globs)
      File "/home/patchbot/sage-patchbot/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 920, in compile_and_execute
        exec(compiled, globs)
      File "<doctest sage.functions.other.Function_crootof._evalf_[0]>", line 1, in <module>
        complex_root_of(x**Integer(2)-Integer(2), Integer(1)).n()
      File "sage/structure/element.pyx", line 865, in sage.structure.element.Element.n (build/cythonized/sage/structure/element.c:8131)
        return self.numerical_approx(prec, digits, algorithm)
      File "sage/symbolic/expression.pyx", line 5824, in sage.symbolic.expression.Expression.numerical_approx (build/cythonized/sage/symbolic/expression.cpp:36286)
        x = x._convert(kwds)
      File "sage/symbolic/expression.pyx", line 1259, in sage.symbolic.expression.Expression._convert (build/cythonized/sage/symbolic/expression.cpp:10663)
        res = self._gobj.evalf(0, kwds)
      File "/home/patchbot/sage-patchbot/local/lib/python2.7/site-packages/sage/functions/other.py", line 2920, in _evalf_
        sobj = CRootOf(Poly(poly._sympy_()), int(index))
      File "sage/rings/complex_number.pyx", line 1058, in sage.rings.complex_number.ComplexNumber.__int__ (build/cythonized/sage/rings/complex_number.c:10918)
        raise TypeError("can't convert complex to int; use int(abs(z))")
    TypeError: can't convert complex to int; use int(abs(z))
**********************************************************************
2 items had failures:
   3 of   5 in sage.functions.other.Function_crootof
   1 of   3 in sage.functions.other.Function_crootof._evalf_
    [622 tests, 4 failures, 17.27 s]  
sage -t --long src/sage/interfaces/sympy.py
**********************************************************************
File "src/sage/interfaces/sympy.py", line 617, in sage.interfaces.sympy._sympysage_crootof
Failed example:
    (sols[0]+1)._sage_().n()
Exception raised:
    Traceback (most recent call last):
      File "/home/patchbot/sage-patchbot/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 517, in _run
        self.compile_and_execute(example, compiler, test.globs)
      File "/home/patchbot/sage-patchbot/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 920, in compile_and_execute
        exec(compiled, globs)
      File "<doctest sage.interfaces.sympy._sympysage_crootof[6]>", line 1, in <module>
        (sols[Integer(0)]+Integer(1))._sage_().n()
      File "sage/structure/element.pyx", line 865, in sage.structure.element.Element.n (build/cythonized/sage/structure/element.c:8131)
        return self.numerical_approx(prec, digits, algorithm)
      File "sage/symbolic/expression.pyx", line 5824, in sage.symbolic.expression.Expression.numerical_approx (build/cythonized/sage/symbolic/expression.cpp:36286)
        x = x._convert(kwds)
      File "sage/symbolic/expression.pyx", line 1259, in sage.symbolic.expression.Expression._convert (build/cythonized/sage/symbolic/expression.cpp:10663)
        res = self._gobj.evalf(0, kwds)
      File "/home/patchbot/sage-patchbot/local/lib/python2.7/site-packages/sage/functions/other.py", line 2920, in _evalf_
        sobj = CRootOf(Poly(poly._sympy_()), int(index))
      File "sage/rings/complex_number.pyx", line 1058, in sage.rings.complex_number.ComplexNumber.__int__ (build/cythonized/sage/rings/complex_number.c:10918)
        raise TypeError("can't convert complex to int; use int(abs(z))")
    TypeError: can't convert complex to int; use int(abs(z))
**********************************************************************
1 item had failures:
   1 of   8 in sage.interfaces.sympy._sympysage_crootof
    [218 tests, 1 failure, 3.27 s]

[rws]

That exact arguments are made inexact is a bug in itself. But that they are made complex (EDIT: in some machines) is beyond me.

---

New commits:

​a93662f

24378: Doctest failures related to sympy

[jdemeyer]

Do you have an idea why this depends on the system?

[jdemeyer]

Replying to rws:

That exact arguments are made inexact is a bug in itself. But that they are made complex (EDIT: in some machines) is beyond me.

Do you know where this happens? It seems to me that it is Pynac doing this.

[vdelecroix]

Some sympy behavior depends on the presence/absence of gmpy2. We had this trouble before the upgrade.

[jdemeyer]

Replying to rws:

That exact arguments are made inexact is a bug in itself.

Right, this ticket should just fix that.

[vdelecroix]

Replying to jdemeyer:

Replying to rws:

That exact arguments are made inexact is a bug in itself.

Right, this ticket should just fix that.

Indeed numerical_approximation of symbolic expression is lazily converting all arguments to the same parent. Which is very wrong when a function is defined on R x N.

Perhaps it would be good to add in _evalf_ the check

if not isinstance(index, numbers.Integral):
    raise ValueError

Though I am curious about this sympy misbehavior.

[vdelecroix]

Indeed there is a sympy bug that is now tracked at #24380.

[vdelecroix]

numerical_approx is very wrong when evaluating. It converts x^2 - 2 to x^2 - 2.0 and hence loosing precision for no good reason.

[vdelecroix]

There was indeed a bug in sympy: #24380 needs review!

[rws]

I suggest accepting my quick fix to resolve the blocker. Meanwhile I'm looking into Pynac.

[jdemeyer]

If your only goal is to resolve the blocker doctest failures, then I think it is better to use # known bug. The fact that the index is made floating-point is the real bug, whether it is real or complex is just a small variation.

[rws]

Replying to vdelecroix:

numerical_approx is very wrong when evaluating. It converts x^2 - 2 to x^2 - 2.0 and hence loosing precision for no good reason.

I disagree in this case. The result is exactly what was asked for.

[vdelecroix]

Replying to rws:

Replying to vdelecroix:

numerical_approx is very wrong when evaluating. It converts x^2 - 2 to x^2 - 2.0 and hence loosing precision for no good reason.

I disagree in this case. The result is exactly what was asked for.

The problem under discussion is that you have a number specified by an equation like x^2 - 2 = 0. Now, you want an numerical approximation of it. I claim that there is not much reason to transform the equation into 1.0 * x^2 - 2.0 for doing that. In particular, this is not what the user wants. This kind of conversion is a can of worms. If you want 100 bits of your number, and start by converting the coefficients to floating points with 100 bits of precision you might loose your roots.

Note that in sympy they solve the original equation with appropriate root finding algorithms on rational numbers.

[rws]

Replying to vdelecroix:

Replying to rws:

Replying to vdelecroix:

numerical_approx is very wrong when evaluating. It converts x^2 - 2 to x^2 - 2.0 and hence loosing precision for no good reason.

I disagree in this case. The result is exactly what was asked for.

The problem under discussion is that you have a number specified by an equation like x^2 - 2 = 0. Now, you want an numerical approximation of it.

No, that is solving. N() just converts symbolic expressions---if they happen to contain symbols you won't get a number, you get an error. _convert() helps with that.

Note that in sympy they solve the original equation with appropriate root finding algorithms on rational numbers.

This has nothing to do with this ticket. This ticket is about losing precision in expressions that can be FP evaluated because they contain no symbols but may contain functions. While the result precision from the function at hand complex_root_of is in principle not affected by the exactness of the index there may be existing/future functions where it is. This can be fixed by not FP converting exact function arguments and letting the function itself (i.e. its evalf member) decide what to do with exact arguments. I may well be wrong, and the best solution instead is a type database for function arguments...

[rws]

Replying to vdelecroix:

Note that in sympy they solve the original equation with appropriate root finding algorithms on rational numbers.

Yes but we don't do the solving. We let SymPy do it.

[rws]

New commits:

​ac76814

24378: avoid doctest fails

[rws]

See #24397 for the fix.

[jdemeyer]

You should add the ticket number: # known bug (Trac #24397).

[git]

Branch pushed to git repo; I updated commit sha1. New commits:

​49c6d61

24378: cosmetics

[rws]

Fortunately this can be resolved easily.

---

New commits:

​ae3a1e0

24387: complex_root_of uses inexact index

[rws]

I have added the information to ​https://github.com/pynac/pynac/wiki/%7C-floating-point-evaluation, but Sage Function initialization options need better docs too.

[jdemeyer]

If this fixes the actual underlying issue, could you add

sage: complex_root_of(x^8-1, 7).n(2)

as a doctest?

[rws]

It fixes the issue but there are other problems at super-low precision.

[git]

Branch pushed to git repo; I updated commit sha1. New commits:

​8c3f951

24378: set minimum digits; convert to given prec

[jdemeyer]

The expression prec*3/10 will behave differently on Python 2 and Python 3. You should either use // or from __future__ import division such that it works the same on Python 2 and Python 3.

[jdemeyer]

What is the meaning of the factor 3/10 anyway?

[rws]

Replying to jdemeyer:

What is the meaning of the factor 3/10 anyway?

An approximation to log(2,10) (EDITED) which is good enough until about 1000 bits precision.

[jdemeyer]

Replying to rws:

Replying to jdemeyer:

What is the meaning of the factor 3/10 anyway?

An approximation to log(2,10) (EDITED) which is good enough until about 1000 bits precision.

Then why don't use the actual value 0.3010299956639812? That avoids the problem with division also.

[git]

Branch pushed to git repo; I updated commit sha1. New commits:

​8e65608

24378: don't prematurely optimize

[rws]

Ping?

[vdelecroix]

ceil(prec*log(2,10))) is terrible. Why not using 0.3010299956639812 as suggested in comment:42? A possible alternative would be

from math import log10
int(prec * log10(2))

You definitely do not want to use two symbolic functions here.

[git]

Branch pushed to git repo; I updated commit sha1. New commits:

​14ed7c6

24378: fix previous commit

[jdemeyer]

Or better yet, use the sympy function prec_to_dps which is meant to convert bits to digits:

from sympy.core.evalf import prec_to_dps

[rws]

Thanks both. I'll make a fresh branch.

[rws]

A minor note is that the results from prec_dps(n) are not identical to the previously proposed formulae at small values, even with a suitable shift.

---

New commits:

​277a403

24378: complex_root_of uses inexact index

[jdemeyer]

Good for me if this passes patchbot testing.