Opened 2 years ago
Last modified 21 months ago
#26311 new defect
py3: strange behavior of sleep() on Sage types
Reported by: | embray | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-pending |
Component: | python3 | Keywords: | |
Cc: | Merged in: | ||
Authors: | Reviewers: | ||
Report Upstream: | Reported upstream. Developers acknowledge bug. | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description (last modified by )
For some reason, passing Python's time.sleep()
a Sage RealLiteral
less than 1 returns more-or-less immediately:
sage: x = 0.5 sage: type(x) <class 'sage.rings.real_mpfr.RealLiteral'> sage: x2 = float(0.5) sage: type(x2) <class 'float'> sage: timeit('sleep(x)') 625 loops, best of 3: 1 µs per loop sage: timeit('sleep(x2)') 5 loops, best of 3: 500 ms per loop sage: # One more time for the folks in back... sage: timeit('sleep(x)') 625 loops, best of 3: 1.01 µs per loop
It's not just in the context of timeit
either. 0.5 is long enough that if you do sleep(x2)
directly you can feel the delay, whereas with sleep(x)
there is much less or even no perceptible delay.
However,
sage: timeit('sleep(1.0)') 5 loops, best of 3: 1 s per loop
as expected.
The problem appears to stem from the (relatively) new PyTime API, and in particular this function:
static int _PyTime_FromObject(_PyTime_t *t, PyObject *obj, _PyTime_round_t round, long unit_to_ns) { if (PyFloat_Check(obj)) { double d; d = PyFloat_AsDouble(obj); if (Py_IS_NAN(d)) { PyErr_SetString(PyExc_ValueError, "Invalid value NaN (not a number)"); return -1; } return _PyTime_FromFloatObject(t, d, round, unit_to_ns); } else { long long sec; Py_BUILD_ASSERT(sizeof(long long) <= sizeof(_PyTime_t)); sec = PyLong_AsLongLong(obj); if (sec == -1 && PyErr_Occurred()) { if (PyErr_ExceptionMatches(PyExc_OverflowError)) _PyTime_overflow(); return -1; } if (_PyTime_check_mul_overflow(sec, unit_to_ns)) { _PyTime_overflow(); return -1; } *t = sec * unit_to_ns; return 0; } }
So it does not properly handle custom types that implement __float__
.
Reported upstream:
In the meantime I don't see that there's much to be done except to always wrap Sage types in float()
before passing them to time functions (or, more extreme, provide our own wrappers for common time functions...)
Change History (11)
comment:1 Changed 2 years ago by
- Description modified (diff)
comment:2 Changed 2 years ago by
comment:3 Changed 2 years ago by
- Milestone changed from sage-8.4 to sage-8.5
comment:4 Changed 2 years ago by
- Milestone changed from sage-8.5 to sage-8.7
Retargeting some of my tickets (somewhat optimistically for now).
comment:5 Changed 23 months ago by
- Description modified (diff)
- Report Upstream changed from N/A to Reported upstream. No feedback yet.
comment:6 Changed 23 months ago by
It feels like there's been a bad habit in CPython lately of needlessly breaking support for custom numerical types.
comment:7 Changed 23 months ago by
- Report Upstream changed from Reported upstream. No feedback yet. to Reported upstream. Developers acknowledge bug.
comment:8 follow-up: ↓ 9 Changed 23 months ago by
Yeesh, that thread on bpo is a mess... :(
comment:9 in reply to: ↑ 8 ; follow-up: ↓ 10 Changed 23 months ago by
- Description modified (diff)
Replying to embray:
Yeesh, that thread on bpo is a mess... :(
Wait until you see the code. DRY clearly does not apply to the CPython code base: I had to make the same fix 3 times.
comment:10 in reply to: ↑ 9 Changed 23 months ago by
comment:11 Changed 21 months ago by
- Milestone changed from sage-8.7 to sage-pending
Removing most of the rest of my open tickets out of the 8.7 milestone, which should be closed.
Thanks for the investigation.