Join GitHub today
GitHub is home to over 40 million developers working together to host and review code, manage projects, and build software together.
Sign upbpo-39310: Add math.ulp(x) #17965
bpo-39310: Add math.ulp(x) #17965
Conversation
Add math.ulp(): return the unit in the last place of x.
This comment has been minimized.
This comment has been minimized.
The documentation should be enhanced |
/*[clinic end generated code: output=f5207867a9384dd4 input=f1004db2d2436ab5]*/ | ||
{ | ||
if (Py_IS_INFINITY(x) || Py_IS_NAN(x)) { | ||
return x; |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
vstinner
Jan 12, 2020
Author
Member
Currently, ulp(-INF) returns -INF. Do you prefer ulp() result to always be positive or NaN? So return +INF for ulp(-INF)?
This comment has been minimized.
This comment has been minimized.
serhiy-storchaka
Jan 12, 2020
Member
It is the only case when ulp()
returns a negative value. It looks surprising. And does not confirm the current documentation.
ulp(nan)
also does not confirm the current documentation (because nan <= nan
is false). Would not be better to raise an exception?
What different implementations of ulp()
does? Is there some specification?
This comment has been minimized.
This comment has been minimized.
vstinner
Jan 12, 2020
Author
Member
It is the only case when ulp() returns a negative value.
Ok, I changed this behavior.
ulp(nan) also does not confirm the current documentation (because nan <= nan is false). Would not be better to raise an exception?
Hum, should I just document that ulp(x) returns x if x is not a number?
I dislike the exception idea. Other math functions also return NaN if one argument is NaN. For example, nextafter() returns NaN.
This comment has been minimized.
This comment has been minimized.
mdickinson
Jan 13, 2020
Member
Java returns +inf for an infinity input, so we'd be consistent with Java. I agree with @serhiy-storchaka that we should return a positive number (or NaN) in all cases.
For NaN, I think it's fine to propagate as usual, but the documentation should specify that that's what happens.
self.assertEqual(math.ulp(0.0), | ||
sys.float_info.min * sys.float_info.epsilon) | ||
self.assertEqual(math.ulp(1.0), sys.float_info.epsilon) | ||
self.assertEqual(math.ulp(2.0 ** 52), 1.0) |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
vstinner
Jan 12, 2020
Author
Member
Tests are hardcoded for binary64. Is it really worth it to make tests generic for other implementations of foat? I'm quite sure that many tests are hardcoded for binary64, I mean, not only test_float.
This comment has been minimized.
This comment has been minimized.
mdickinson
Jan 13, 2020
Member
I'd stick a @requires_IEEE754 on this and just make the tests specific to IEEE 754.
This comment has been minimized.
This comment has been minimized.
mdickinson
Jan 13, 2020
•
Member
2.0 ** 52
is unsafe here, since it relies on the platform libm having an accurate pow, and in general that's not a safe assumption. You could use float(2**52)
or ldexp(1.0, 52)
instead as a safer alternative.
This comment has been minimized.
This comment has been minimized.
Done. I copied the test_math.ulp() docstring. |
This comment has been minimized.
This comment has been minimized.
I think Java does as well as can be done here: https://www.geeksforgeeks.org/java-math-ulp-method-examples/ Short course:
|
.. function:: ulp(x) | ||
|
||
Return the value of the least significant bit of a float *x*, such that the | ||
first float bigger than *x* is ``x + ulp(x)``. Then, given an expected |
This comment has been minimized.
This comment has been minimized.
mdickinson
Jan 13, 2020
Member
This description doesn't work for negative x
: e.g., if x = -1.0
. Maybe the documentation should describe what happens only for positive x
, and then comment that for negative x
, the result is just ulp(-x)
(or ulp(abs(x))
)?
This comment has been minimized.
This comment has been minimized.
mdickinson
Jan 13, 2020
Member
We should also explicitly state the behaviour for zeros, infinities and nans. ulp
is not a standard function, and even the experts don't agree on how it should be defined. See for example this article.
Return the value of the least significant bit of a float *x*, such that the | ||
first float bigger than *x* is ``x + ulp(x)``. Then, given an expected | ||
result *x* and a tolerance of ``n`` ulps, the result ``y`` should be such | ||
that ``abs(y - x) <= n * ulp(x)``. |
This comment has been minimized.
This comment has been minimized.
mdickinson
Jan 13, 2020
Member
This gets messy across power of two boundaries; in that case, "a tolerance of n ulps" isn't a well-defined thing without further clarification or specification. It may be better simply to drop this sentence.
return x - x2; | ||
} | ||
return x2 - x; | ||
} |
vstinner commentedJan 12, 2020
•
edited by bedevere-bot
Add math.ulp(): return the unit in the last place of x.
https://bugs.python.org/issue39310