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

Merged
merged 1 commit into from Jan 13, 2020
Merged

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

merged 1 commit into from Jan 13, 2020

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

@vstinner

This comment has been minimized.

Copy link
Member Author

vstinner commented Jan 12, 2020

The documentation should be enhanced 😊

Modules/mathmodule.c Show resolved Hide resolved
Lib/test/test_math.py Outdated Show resolved Hide resolved
@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.
Doc/library/math.rst Outdated Show resolved Hide resolved
Doc/library/math.rst Outdated Show resolved Hide resolved
@vstinner vstinner force-pushed the vstinner:ulp branch from d74c6c1 to 86ad841 Jan 13, 2020
@vstinner

This comment has been minimized.

Copy link
Member Author

vstinner commented Jan 13, 2020

@mdickinson, @tim-one, @serhiy-storchaka: I updated the PR. Would you mind to review it again?

I rewrote the documentation to clarify the behavior on corner cases: zero, min/max, inf, NaN.

Doc/library/math.rst Outdated Show resolved Hide resolved
Lib/test/test_math.py Outdated Show resolved Hide resolved
Copy link
Member

mdickinson left a comment

One request for an explicit test for ulp(-0.0). Apart from that, this LGTM.

Copy link
Member

serhiy-storchaka left a comment

Agree with @mdickinson. The rest LGTM.

@vstinner

This comment has been minimized.

Copy link
Member Author

vstinner commented Jan 13, 2020

Interesting case, math.ulp(x) == x is for one value, the smallest denormalized positive float:

>>> import math, sys
>>> x = math.ulp(0)
>>> math.ulp(x) == x
True
>>> x
5e-324
@vstinner

This comment has been minimized.

Copy link
Member Author

vstinner commented Jan 13, 2020

I'm not sure that math.ulp(sys.float_info.max) is correct: it looks 2x larger than what we should get, since (max - math.ulp(max)/2) != max and (max + math.ulp(max)/2) != max.

vstinner@apu$ ./python
Python 3.9.0a2+ (heads/nextafter:6bbcab5fa2, Jan 11 2020, 16:24:52) 
>>> import math, sys
>>> max = sys.float_info.max
>>> inf = math.inf

# what do we have between max and inf?

>>> math.nextafter(max, inf)
inf
>>> math.nextafter(inf, -inf) == max
True
>>> inf - max
inf

# from max to first float before max (prev)

>>> prev=math.nextafter(max, -inf)
>>> (max - math.ulp(max)) == prev
True
>>> (max - math.ulp(max)/2) == prev  # oh!!!
True
>>> (max - math.ulp(max)/4) == prev
False
>>> (max - math.ulp(max)/4) == max
True

# from max to inf

>>> max + math.ulp(max)
inf
>>> max + math.ulp(max)/2  # oh!!!
inf
>>> max + math.ulp(max)/4
1.7976931348623157e+308
>>> max + math.ulp(max)/4 == max
True

# max ULP value
>>> math.ulp(max)
1.99584030953472e+292
>>> math.ulp(max)/2
9.9792015476736e+291
@mdickinson

This comment has been minimized.

Copy link
Member

mdickinson commented Jan 13, 2020

>>> (max - math.ulp(max)/2) == prev  # oh!!!
True

This is fine; it's just round-ties-to-even at work. max - math.ulp(max) / 2 is exactly halfway between two representable floats: max and prev. Of those two, prev has its last bit clear, so the tie is rounded to prev rather than max.

@vstinner

This comment has been minimized.

Copy link
Member Author

vstinner commented Jan 13, 2020

One request for an explicit test for ulp(-0.0). Apart from that, this LGTM.

I wrote tests differently to better highlight that we expect ulp(-x) = ulp(x), and not copy/paste results from tests on positive numbers to tests on negative numbers:

        # negative number: ulp(-x) == ulp(x)
        for x in (0.0, 1.0, 2 ** 52, 2 ** 64, INF):
            with self.subTest(x=x):
                self.assertEqual(math.ulp(-x), math.ulp(x))
@vstinner

This comment has been minimized.

Copy link
Member Author

vstinner commented Jan 13, 2020

Oh, nntplib test fail on the Documentation job of Travis CI:

Warning, treated as error:

**********************************************************************

File "library/nntplib.rst", line ?, in default

Failed example:

    s = NNTP('news.gmane.io')

Exception raised:

    Traceback (most recent call last):

      File "/home/travis/build/python/cpython/Lib/doctest.py", line 1329, in __run

        exec(compile(example.source, filename, "single",

      File "<doctest default[0]>", line 1, in <module>

        s = NNTP('news.gmane.io')

      File "/home/travis/build/python/cpython/Lib/nntplib.py", line 1049, in __init__

        super().__init__(file, host, readermode, timeout)

      File "/home/travis/build/python/cpython/Lib/nntplib.py", line 331, in __init__

        self.welcome = self._getresp()

      File "/home/travis/build/python/cpython/Lib/nntplib.py", line 456, in _getresp

        raise NNTPTemporaryError(resp)

    nntplib.NNTPTemporaryError: 400 load at 18.62, try later

I retry the job.

@vstinner

This comment has been minimized.

Copy link
Member Author

vstinner commented Jan 13, 2020

This is fine; it's just round-ties-to-even at work. max - math.ulp(max) / 2 is exactly halfway between two representable floats: max and prev. Of those two, prev has its last bit clear, so the tie is rounded to prev rather than max.

Floating points are so complex :-(

@vstinner vstinner force-pushed the vstinner:ulp branch from 95062b4 to 3afeffc Jan 13, 2020
@vstinner

This comment has been minimized.

Copy link
Member Author

vstinner commented Jan 13, 2020

Oops, I forgot to update the documentation in the docstring and commit message: updated.

@vstinner vstinner force-pushed the vstinner:ulp branch from 3afeffc to d381835 Jan 13, 2020
Add math.ulp(): return the value of the least significant bit
of a float.
@vstinner vstinner force-pushed the vstinner:ulp branch from d381835 to 7efb4fe Jan 13, 2020
@vstinner vstinner merged commit 0b2ab21 into python:master Jan 13, 2020
9 checks passed
9 checks passed
Docs
Details
Windows (x86)
Details
Windows (x64)
Details
macOS
Details
Ubuntu
Details
Azure Pipelines PR #20200113.16 succeeded
Details
bedevere/issue-number Issue number 39310 found
Details
bedevere/news News entry found in Misc/NEWS.d
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@vstinner vstinner deleted the vstinner:ulp branch Jan 13, 2020
@vstinner

This comment has been minimized.

Copy link
Member Author

vstinner commented Jan 13, 2020

Thanks a lot @tim-one, @serhiy-storchaka and @mdickinson: the review was required and really helpful here ;-)

sthagen added a commit to sthagen/cpython that referenced this pull request Jan 13, 2020
bpo-39310: Add math.ulp(x) (pythonGH-17965)
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.