Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bpo-39310: Add math.ulp(x) #17965

Open
wants to merge 4 commits into
base: master
from
Open

bpo-39310: Add math.ulp(x) #17965

wants to merge 4 commits into from

Conversation

@vstinner
Copy link
Member

vstinner commented Jan 12, 2020

Add math.ulp(): return the unit in the last place of x.

https://bugs.python.org/issue39310

Add math.ulp(): return the unit in the last place of x.
@vstinner

This comment has been minimized.

Copy link
Member Author

vstinner commented Jan 12, 2020

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.

Copy link
@serhiy-storchaka

serhiy-storchaka Jan 12, 2020

Member

It can return negative value.

This comment has been minimized.

Copy link
@vstinner

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.

Copy link
@serhiy-storchaka

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.

Copy link
@vstinner

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.

Copy link
@mdickinson

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.

Copy link
@serhiy-storchaka

serhiy-storchaka Jan 12, 2020

Member

Maybe use sys.float_info.radix ** (sys.float_info.mant_dig-1)?

This comment has been minimized.

Copy link
@vstinner

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.

Copy link
@mdickinson

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.

Copy link
@mdickinson

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.

@vstinner

This comment has been minimized.

Copy link
Member Author

vstinner commented Jan 12, 2020

The documentation should be enhanced blush

Done. I copied the test_math.ulp() docstring.

@tim-one

This comment has been minimized.

Copy link
Member

tim-one commented Jan 12, 2020

I think Java does as well as can be done here:

https://www.geeksforgeeks.org/java-math-ulp-method-examples/

Short course:

  • Return a NaN for a NaN input.
  • Return +inf for an infinite input (regardless of sign).
  • Return the smallest positive representable double for +0 or -0 input.
.. 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.

Copy link
@mdickinson

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.

Copy link
@mdickinson

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.

Copy link
@mdickinson

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;
}

This comment has been minimized.

Copy link
@mdickinson

mdickinson Jan 13, 2020

Member

This set of behaviour choices looks good to me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.