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.

Sign up for GitHub

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

Jump to bottom

Use C99 functions in math if available #70309

Closed
serhiy-storchaka opened this issue Jan 15, 2016 · 15 comments
Closed

Use C99 functions in math if available #70309

serhiy-storchaka opened this issue Jan 15, 2016 · 15 comments
Labels
3.7 (EOL) end of life extension-modules C modules in the Modules dir type-feature A feature request or enhancement

Comments

@serhiy-storchaka
Copy link
Member

BPO 26121
Nosy @mdickinson, @tiran, @zware, @serhiy-storchaka
PRs
  • bpo-26121: Use C library implementation for math functions: #515
  • bpo-26121: Use C library implementation for math functions on Windows: #632
  • bpo-26121: Revert to using the own implementations of lgamma and gamm… #637
  • [Do Not Merge] Sample of CPython life with blurb. #703
  • bpo-26121: Fix description in Python 3.7 What's New #3603
    		•
    Files
  • math_libc_funcs.patch
    		•

    Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = None
closed_at = <Date 2017-03-12.12:23:40.722>
created_at = <Date 2016-01-15.11:42:15.479>
labels = ['extension-modules', 'type-feature', '3.7']
title = 'Use C99 functions in math if available'
updated_at = <Date 2017-09-15.16:57:46.090>
user = 'https://github.com/serhiy-storchaka'

    bugs.python.org fields:

    activity = <Date 2017-09-15.16:57:46.090>
actor = 'paulromano'
assignee = 'none'
closed = True
closed_date = <Date 2017-03-12.12:23:40.722>
closer = 'serhiy.storchaka'
components = ['Extension Modules']
creation = <Date 2016-01-15.11:42:15.479>
creator = 'serhiy.storchaka'
dependencies = []
files = ['41623']
hgrepos = []
issue_num = 26121
keywords = ['patch']
message_count = 15.0
messages = ['258288', '258293', '258505', '258508', '289099', '289457', '289478', '289486', '289487', '289491', '289494', '289499', '290199', '290201', '290225']
nosy_count = 4.0
nosy_names = ['mark.dickinson', 'christian.heimes', 'zach.ware', 'serhiy.storchaka']
pr_nums = ['515', '632', '637', '703', '3603']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue26121'
versions = ['Python 3.7']
    @serhiy-storchaka
    Copy link
    Member Author

    Currently the math module uses own implementation of some mathematical functions that are in C99 standard, but not in C89 standard: tgamma, lgamma, erf, erfc. Proposed patch makes it to use functions from standard C library if they are available. They are faster and presumably more accurate.

    Here are microbenchmark results (time in microseconds):

                  0.1     1      3     10     30

    erf unpatched: 0.506 0.655 0.509 0.548 0.239
    erf patched: 0.129 0.252 0.357 0.253 0.253

    erfc unpatched: 0.508 0.646 0.532 0.522 0.251
    erfc patched: 0.129 0.239 0.373 0.371 0.307

                    0.1    1.5     3     10     10.5

    gamma unpatched: 0.369 0.279 0.273 0.274 0.457
    gamma patched: 0.24 0.23 0.412 0.741 0.682

    lgamma unpatched: 0.351 0.338 0.478 0.627 0.52
    lgamma patched: 0.217 0.155 0.37 0.372 0.247

    If some libm implementations are pretty bad, they can be disabled by undefining corresponding HAVE_XXX macros.

    @serhiy-storchaka serhiy-storchaka added extension-modules C modules in the Modules dir type-feature A feature request or enhancement labels Jan 15, 2016
    @tiran
    Copy link
    Member

    tiran commented Jan 15, 2016

    Faster: maybe. More accurate ... Mark can tell you some funny stories. :)

    @mdickinson
    Copy link
    Member

    LGTM. What platform are the benchmarks on?

    Is there some way that this can be submitted to the buildbots *before* applying to the 3.6 branch, so that we can get a sense of how widespread (if at all) libm problems are?

    @serhiy-storchaka
    Copy link
    Member Author

    The benchmarks were ran on 32-bit Linux with AMD processor.

    I'll try to run custom build on buildbots.

    @serhiy-storchaka
    Copy link
    Member Author

    I don't know how to run custom build on buildbots with new workflow.

    @serhiy-storchaka serhiy-storchaka added the 3.7 (EOL) end of life label Mar 6, 2017
    @mdickinson
    Copy link
    Member

    So the GitHub branch fails on my OS X 10.10 machine: there are a number of test failures for gamma and lgamma (but none for erf and erfc).

    Some of the gamma and lgamma test failures are accuracy issues; some are more likely to do with errno handling: OS X probably isn't setting errno appropriate on domain or range errors, so any implementation using the libm lgamma/tgamma would need to add code for error handling.

    Given this, I'd prefer to leave lgamma and gamma as-is, using their current implementations, at least on OS X; I don't regard the accuracy loss in using the libm functions to be within acceptable limits.

    There may still be a case for using the libm versions of erf and erfc.

    @serhiy-storchaka
    Copy link
    Member Author

    Tests are failed only on s390x RHEL 3.x.

    http://buildbot.python.org/all/builders/s390x%20RHEL%203.x/builds/446/steps/test/logs/stdio
    ======================================================================
    FAIL: test_mtestfile (test.test_math.MathTests)
    ----------------------------------------------------------------------

    Traceback (most recent call last):
  File "/home/dje/cpython-buildarea/3.x.edelsohn-rhel-z/build/Lib/test/test_math.py", line 1287, in test_mtestfile
    '\n  '.join(failures))
AssertionError: Failures in test_mtestfile:
  gam0003: gamma(-inf): expected 'ValueError', got nan (not equal)
  gam0045: gamma(1e-160): expected 1e+160, got 1.0000000000000063e+160 (error = 6.24e+145 (40 ulps); permitted error = 0 or 20 ulps)
  gam0046: gamma(1e-308): expected 1e+308, got 1.0000000000000136e+308 (error = 1.36e+294 (68 ulps); permitted error = 0 or 20 ulps)
  gam0047: gamma(5.6e-309): expected 1.7857142857142848e+308, got 1.785714285714199e+308 (error = 8.58e+294 (430 ulps); permitted error = 0 or 20 ulps)
  gam0065: gamma(-1e-160): expected -1e+160, got -1.0000000000000063e+160 (error = 6.24e+145 (40 ulps); permitted error = 0 or 20 ulps)
  gam0066: gamma(-1e-308): expected -1e+308, got -1.0000000000000136e+308 (error = 1.36e+294 (68 ulps); permitted error = 0 or 20 ulps)
  gam0067: gamma(-5.6e-309): expected -1.7857142857142848e+308, got -1.785714285714199e+308 (error = 8.58e+294 (430 ulps); permitted error = 0 or 20 ulps)
  gam0081: gamma(-1.0000000000000002): expected 4503599627370495.5, got 4503599627370484.5 (error = 11 (22 ulps); permitted error = 0 or 20 ulps)
  gam0084: gamma(-100.00000000000001): expected -7.540083334883109e-145, got -7.540083334882688e-145 (error = 4.21e-158 (296 ulps); permitted error = 0 or 20 ulps)
  gam0085: gamma(-99.99999999999999): expected 7.540083334884096e-145, got 7.540083334884402e-145 (error = 3.06e-158 (215 ulps); permitted error = 0 or 20 ulps)
  gam0100: gamma(170.0): expected 4.269068009004705e+304, got 4.269068009005011e+304 (error = 3.06e+291 (628 ulps); permitted error = 0 or 20 ulps)
  gam0101: gamma(171.0): expected 7.257415615307999e+306, got 7.257415615308882e+306 (error = 8.83e+293 (708 ulps); permitted error = 0 or 20 ulps)
  gam0102: gamma(171.624): expected 1.7942117599248104e+308, got 1.794211759924979e+308 (error = 1.69e+295 (845 ulps); permitted error = 0 or 20 ulps)
  gam0120: gamma(-100.5): expected -3.3536908198076787e-159, got -3.353690819807666e-159 (error = 1.26e-173 (25 ulps); permitted error = 0 or 20 ulps)
  gam0121: gamma(-160.5): expected -5.255546447007829e-286, got -5.255546447008121e-286 (error = 2.92e-299 (313 ulps); permitted error = 0 or 20 ulps)
  gam0122: gamma(-170.5): expected -3.3127395215386074e-308, got -3.312739521538679e-308 (error = 7.16e-322 (145 ulps); permitted error = 0 or 20 ulps)
  gam0140: gamma(-63.349078729022985): expected 4.177797167776188e-88, got 4.177797167776136e-88 (error = 5.19e-102 (93 ulps); permitted error = 0 or 20 ulps)
  gam0141: gamma(-127.45117632943295): expected 1.183111089623681e-214, got 1.183111089623727e-214 (error = 4.6e-228 (223 ulps); permitted error = 0 or 20 ulps)

    @serhiy-storchaka
    Copy link
    Member Author

    PR 632 switch on using libc implementation on Windows. Will see how AppVeyor tests passed.

    @serhiy-storchaka
    Copy link
    Member Author

    There are accuracy issues with tgamma() and lgamma() on Windows. But less than on OS X.

    ======================================================================
    FAIL: test_mtestfile (test.test_math.MathTests)
    ----------------------------------------------------------------------

    Traceback (most recent call last):
  File "C:\projects\cpython\lib\test\test_math.py", line 1287, in test_mtestfile
    '\n  '.join(failures))
AssertionError: Failures in test_mtestfile:
  lgam0085: lgamma(-99.99999999999999): expected -331.85460524980596, got -331.86198661322277 (error = 0.00738 (129854318490 ulps); permitted error = 1e-15 or 5 ulps)
  gam0085: gamma(-99.99999999999999): expected 7.540083334884096e-145, got 7.484632144060724e-145 (error = 5.55e-147 (38979707393612 ulps); permitted error = 0 or 20 ulps)
  gam0124: gamma(-176.5): expected -1.196e-321, got 0.0 (error = 1.2e-321 (243 ulps); permitted error = 0 or 20 ulps)

    Does this mean that we should not use libc implementations of tgamma() and lgamma() on Windows?

    @mdickinson
    Copy link
    Member

    Please can we revert to using the existing implementations of lgamma and gamma on all platforms? It's clear that there are many issues with OS-provided implementations.

    @serhiy-storchaka
    Copy link
    Member Author

    PR 637 revert to using the own implementations of lgamma and gamma on all platforms.

    @serhiy-storchaka
    Copy link
    Member Author

    Done.

    @serhiy-storchaka
    Copy link
    Member Author

    New changeset 4125e5c by Serhiy Storchaka in branch 'master':
    bpo-26121: Revert to using the own implementations of lgamma and gamma on all platforms. (#637)
    4125e5c

    @serhiy-storchaka
    Copy link
    Member Author

    New changeset 4dadcd4 by Serhiy Storchaka in branch 'master':
    bpo-26121: Use C library implementation for math functions erf() and erfc() on Windows. (#632)
    4dadcd4

    @mdickinson
    Copy link
    Member

    New changeset 97553fd by Mark Dickinson (Serhiy Storchaka) in branch 'master':
    bpo-26121: Use C library implementation for math functions: (#515)
    97553fd

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    @mdickinson mdickinson mentioned this issue Feb 8, 2023
    Closed
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Assignees
    No one assigned
    Labels
    3.7 (EOL) end of life extension-modules C modules in the Modules dir type-feature A feature request or enhancement
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    3 participants
    @tiran @mdickinson @serhiy-storchaka