Opened 4 years ago

Closed 4 years ago

#26839 closed defect (fixed)

RealLazyField: converting constants in an expression to float

Reported by: Pat Hooper Owned by:
Priority: major Milestone: sage-8.7
Component: basic arithmetic Keywords: RealLazyField
Cc: Vincent Delecroix Merged in:
Authors: Vincent Delecroix Reviewers: Sébastien Labbé
Report Upstream: N/A Work issues:
Branch: 4c44eae (Commits, GitHub, GitLab) Commit: 4c44eaefe5d22776f58b8114af52d471181e982f
Dependencies: Stopgaps:

Status badges

Description

RealLazyField exhibits the following bug on 8.4 stable and on development branch:

sage: float(sin(RLF.pi()))

---------------------------------------------------------------------------
AttributeError                            Traceback (most recent call last)
<ipython-input-1-750e6734ef1c> in <module>()
----> 1 float(sin(RLF.pi()))

/home/pat/Software/Sage/sage-git/sage/local/lib/python2.7/site-packages/sage/rings/real_lazy.pyx in sage.rings.real_lazy.LazyNamedUnop.__float__ (build/cythonized/sage/rings/real_lazy.c:16203)()
   1404             0.8414709848078965
   1405         """
-> 1406         return self.eval(float)
   1407 
   1408     def __call__(self, *args):

/home/pat/Software/Sage/sage-git/sage/local/lib/python2.7/site-packages/sage/rings/real_lazy.pyx in sage.rings.real_lazy.LazyNamedUnop.eval (build/cythonized/sage/rings/real_lazy.c:15520)()
   1348             2.0
   1349         """
-> 1350         arg = self._arg.eval(R)
   1351         cdef bint has_extra_args = self._extra_args is not None and len(self._extra_args) > 0
   1352         if type(R) is type:

/home/pat/Software/Sage/sage-git/sage/local/lib/python2.7/site-packages/sage/rings/real_lazy.pyx in sage.rings.real_lazy.LazyConstant.eval (build/cythonized/sage/rings/real_lazy.c:16924)()
   1490                 else:
   1491                     raise TypeError("The complex constant I is not in this real field.")
-> 1492         f = getattr(R, self._name)
   1493         if self._extra_args is None:
   1494             return f()

AttributeError: type object 'float' has no attribute 'pi'

The same issue appears with RDF replacing float.

Vincent noted that the following works fine:

sage: (sin(RLF.pi())).eval(RDF)
 1.2246467991473515e-16

On the other hand I get:

sage: (sin(RLF.pi())).eval(float)

---------------------------------------------------------------------------
AttributeError...

Change History (18)

comment:1 Changed 4 years ago by Vincent Delecroix

This is already visible without the sin

sage: RLF.pi().eval(float)
AttributeError                            Traceback (most recent call last)
.../sage/rings/real_lazy.pyx
in sage.rings.real_lazy.LazyConstant.eval
(build/cythonized/sage/rings/real_lazy.c:16446)()
   1467         self._is_special = name in ['e', 'I']
   1468 
-> 1469     cpdef eval(self, R):
   1470         """
   1471         Convert ``self`` into an element of ``R``.

...sage/rings/real_lazy.pyx
in sage.rings.real_lazy.LazyConstant.eval
(build/cythonized/sage/rings/real_lazy.c:16329)()
   1490                 else:
   1491                     raise TypeError("The complex constant I is not in this real field.")
-> 1492         f = getattr(R, self._name)
   1493         if self._extra_args is None:
   1494             return f()

AttributeError: type object 'float' has no attribute 'pi'

comment:2 Changed 4 years ago by Vincent Delecroix

Authors: Vincent Delecroix
Branch: u/vdelecroix/26839
Commit: 9faa2567bfd7333dcc29b492dc36c122e98092e0
Status: newneeds_review

New commits:

9faa25626839: fix evaluation of lazy constants

comment:3 Changed 4 years ago by git

Commit: 9faa2567bfd7333dcc29b492dc36c122e98092e0c4cde9e12208df044a02da5d52b7d34ad895f241

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

c4cde9e26839: get rid of _is_special

comment:4 Changed 4 years ago by Vincent Delecroix

Pat, can you make the review?

comment:5 Changed 4 years ago by Sébastien Labbé

Reviewers: Sébastien Labbé
Status: needs_reviewpositive_review

comment:6 Changed 4 years ago by Volker Braun

Status: positive_reviewneeds_work

There are some numerical noise issues, see patchbot.

comment:7 Changed 4 years ago by Sébastien Labbé

Indeed:

----------------------------------------------------------------------
sage -t --long src/sage/rings/real_lazy.pyx  # 1 doctest failed
----------------------------------------------------------------------

For reference, I am copying the failure below:

Only doctesting files that failed last test.
Doctesting 2 files using 8 threads.
sage -t --long src/sage/rings/real_lazy.pxd
    [0 tests, 0.00 s]
sage -t --long src/sage/rings/real_lazy.pyx
**********************************************************************
File "src/sage/rings/real_lazy.pyx", line 1495, in sage.rings.real_lazy.LazyConstant.eval
Failed example:
    RLF.pi().eval(RealBallField(128))
Expected:
    [3.1415926535897932384626433832795028842 +/- 1.65e-38]
Got:
    [3.1415926535897932384626433832795028842 +/- 1.06e-38]
**********************************************************************
1 item had failures:
   1 of  12 in sage.rings.real_lazy.LazyConstant.eval
    [285 tests, 1 failure, 0.56 s]
----------------------------------------------------------------------
sage -t --long src/sage/rings/real_lazy.pyx  # 1 doctest failed
----------------------------------------------------------------------

comment:8 Changed 4 years ago by Sébastien Labbé

On my laptop running 8.5.beta4, I get radius 1.65e-38 with 128 and 1.06e-38 with 129.

┌────────────────────────────────────────────────────────────────────┐
│ SageMath version 8.5.beta4, Release Date: 2018-11-18               │
│ Using Python 2.7.15. Type "help()" for help.                       │
└────────────────────────────────────────────────────────────────────┘
sage: RLF.pi().eval(RealBallField(127))
[3.1415926535897932384626433832795028842 +/- 4.00e-38]
sage: RLF.pi().eval(RealBallField(128))
[3.1415926535897932384626433832795028842 +/- 1.65e-38]
sage: RLF.pi().eval(RealBallField(129))
[3.1415926535897932384626433832795028842 +/- 1.06e-38]
sage: RLF.pi().eval(RealBallField(130))
[3.14159265358979323846264338327950288420 +/- 7.66e-39]

On another machine running more recent 8.5.rc0, I get the failure:

┌────────────────────────────────────────────────────────────────────┐
│ SageMath version 8.5.rc0, Release Date: 2018-12-10                 │
│ Using Python 2.7.15. Type "help()" for help.                       │
└────────────────────────────────────────────────────────────────────┘
sage: RLF.pi().eval(RealBallField(127))
[3.1415926535897932384626433832795028842 +/- 1.88e-38]
sage: RLF.pi().eval(RealBallField(128))
[3.1415926535897932384626433832795028842 +/- 1.06e-38]
sage: RLF.pi().eval(RealBallField(129))
[3.14159265358979323846264338327950288420 +/- 7.66e-39]
sage: RLF.pi().eval(RealBallField(130))
[3.14159265358979323846264338327950288420 +/- 3.25e-39]

Finally, running Sage with Python 3, I get the failure too:

┌────────────────────────────────────────────────────────────────────┐
│ SageMath version 8.5, Release Date: 2018-12-22                     │
│ Using Python 3.6.6. Type "help()" for help.                        │
└────────────────────────────────────────────────────────────────────┘
AA.options.display_format = 'radical'
sage: RLF.pi().eval(RealBallField(127))
[3.1415926535897932384626433832795028842 +/- 1.88e-38]
sage: RLF.pi().eval(RealBallField(128))
[3.1415926535897932384626433832795028842 +/- 1.06e-38]
sage: RLF.pi().eval(RealBallField(129))
[3.14159265358979323846264338327950288420 +/- 7.66e-39]
sage: RLF.pi().eval(RealBallField(130))
[3.14159265358979323846264338327950288420 +/- 3.25e-39]

Did the version of RealBallField changed between 8.5.beta4 and 8.5.rc0?

Last edited 4 years ago by Sébastien Labbé (previous) (diff)

comment:9 Changed 4 years ago by Vincent Delecroix

#24369 ?

comment:10 Changed 4 years ago by Vincent Delecroix

#26360: Upgrade arb to 2.15.1 ?

comment:11 Changed 4 years ago by Sébastien Labbé

When I reviewed this ticket (no failures), I was using 8.5.beta6.

comment:12 in reply to:  10 Changed 4 years ago by Sébastien Labbé

Replying to vdelecroix:

#26360: Upgrade arb to 2.15.1 ?

Looks like it, merged in 8.5.rc0 which is just between 8.5.beta6 and 8.5.

comment:13 Changed 4 years ago by git

Commit: c4cde9e12208df044a02da5d52b7d34ad895f2414c44eaefe5d22776f58b8114af52d471181e982f

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

f52cef426839: fix evaluation of lazy constants
4c44eae26839: get rid of _is_special

comment:14 Changed 4 years ago by Vincent Delecroix

Milestone: sage-8.5sage-8.7
Status: needs_workneeds_review

comment:15 Changed 4 years ago by Sébastien Labbé

Note that all doctests were fixed as follows in #26360:

-            [1.333333333333333 +/- 5.37e-16]
+            [1.333333333333333 +/- ...e-16]
Version 1, edited 4 years ago by Sébastien Labbé (previous) (next) (diff)

comment:16 Changed 4 years ago by Vincent Delecroix

I don't think it was a good move. Tracking arb enclosure regression/improvement is worth the trouble (see eg https://github.com/fredrik-johansson/arb/issues/227).

comment:17 Changed 4 years ago by Sébastien Labbé

Status: needs_reviewpositive_review

ok

comment:18 Changed 4 years ago by Volker Braun

Branch: u/vdelecroix/268394c44eaefe5d22776f58b8114af52d471181e982f
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.