GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — Refactor math module to use C99+ special functions (erf, erfc, etc) · Issue #101678 · python/cpython

Refactor math module to use C99+ special functions (erf, erfc, etc) #101678

New issue

Copy link

New issue

Copy link

Closed

Refactor math module to use C99+ special functions (erf, erfc, etc)#101678

Copy link

Labels

extension-modulesC modules in the Modules dirC modules in the Modules dirtype-featureA feature request or enhancementA feature request or enhancement

Description

skirpichev

opened

on Feb 8, 2023

Issue body actions

The PEP 7 documents, that we must use C11 (without optional features) since CPython 3.11. erf, erfc, lgamma and tgamma (math.gamma) are part of the C99 (and mandatory for C11 too).

Probably, it's not a bad idea to get rid of custom implementations of such functions or document why it's present (e.g. for some exotic platform, broken libc implementation, speed optimization, etc). Keep in mind, that this code isn't tested by CI if we optionally include using standard implementations (as for erf with #ifdef HAVE_ERF).

Linked PRs

Reacted by Oleg Iarygin

Activity

skirpichev
added
type-featureA feature request or enhancementA feature request or enhancement
on Feb 8, 2023
skirpichev
added a commit that references this issue on Feb 8, 2023
pythongh-101678: refactor the math module to use special functions fr…
d202b5c
arhadthedev
added
interpreter-core(Objects, Python, Grammar, and Parser dirs)(Objects, Python, Grammar, and Parser dirs)
on Feb 8, 2023
skirpichev commented on Feb 8, 2023
skirpichev
on Feb 8, 2023
MemberAuthor
More actions
(@arhadthedev, I think it's more related to the stdlib.)
arhadthedev commented on Feb 8, 2023
arhadthedev
on Feb 8, 2023
Member
More actions
Should work because:

starting with 3.11 Building Python now requires [...] Support for IEEE 754 floating point numbers. (Contributed by Victor Stinner in bpo-46917.)

Wikipedia description of the C11-optional Annex F doesn't mention erf or erfc so it should be supported on all platforms.
arhadthedev
added
extension-modulesC modules in the Modules dirC modules in the Modules dir
and removed
interpreter-core(Objects, Python, Grammar, and Parser dirs)(Objects, Python, Grammar, and Parser dirs)
on Feb 8, 2023
arhadthedev commented on Feb 8, 2023
arhadthedev
on Feb 8, 2023
Member
More actions

I think it's more related to the stdlib.

Tags are about affected directories, not user-visible effect. From descriptions of the labels:

extension-modules - C modules in the Modules dir

interpreter-core - Objects, Python, Grammar, and Parser dirs

stdlib Python modules in the Lib dir

Reacted by Alex Waygood
arhadthedev commented on Feb 8, 2023
arhadthedev
on Feb 8, 2023
Member
More actions
cc @vstinner
mdickinson commented on Feb 8, 2023
mdickinson
on Feb 8, 2023
Member
More actions
We've tried this before. The key issue is not availability, but quality. Both when I originally implemented these functions, and when we last tried using the libm versions, the libm versions turned out to be of unacceptably poor quality (cases where the computed results were many millions of ulps out from the correct results).

If you do this, please augment the tests to do very thorough testing of the relevant functions, and then run on all the buildbots. But really, I'd strongly recommend leaving this alone - there's no issue being fixed here. We're only increasing the uncertainty over the quality of the functions.
mdickinson commented on Feb 8, 2023
mdickinson
on Feb 8, 2023
Member
More actions
See #70309 for the previous attempt.
mdickinson commented on Feb 8, 2023
mdickinson
on Feb 8, 2023
Member
More actions

Probably, it's not a bad idea [...]

The rationale for making changes to stable, working code needs to be a lot stronger than this.
skirpichev commented on Feb 8, 2023
skirpichev
on Feb 8, 2023
MemberAuthor
More actions

the libm versions turned out to be of unacceptably poor quality

Isn't that the case where fixes should be submitted to upstream? Or libm implementations will be of poor quality forever...

there's no issue being fixed here

At least, there is no regular testing.

arhadthedev commented on Feb 8, 2023

arhadthedev

on Feb 8, 2023

Member

More actions

For history, here is an example from the failed macOS run:

======================================================================
FAIL: test_mtestfile (test.test_math.MathTests.test_mtestfile)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/runner/work/cpython/cpython/Lib/test/test_math.py", line 2053, in test_mtestfile
    self.fail('Failures in test_mtestfile:\n  ' +
AssertionError: Failures in test_mtestfile:
  lgam0080: lgamma(-0.9999999999999999): expected 36.7368005696771, got 36.25168935623487 (error = 0.485 (6[827](https://github.com/python/cpython/actions/runs/4121854833/jobs/7117957687#step:7:828)3333752865 ulps); permitted error = 1e-15 or 5 ulps)
  lgam0082: lgamma(-1.9999999999999998): expected 35.35050620855721, got 34.9797722396[830](https://github.com/python/cpython/actions/runs/4121854833/jobs/7117957687#step:7:831)3 (error = 0.371 (52176167627355 ulps); permitted error = 1e-15 or 5 ulps)
  lgam0085: lgamma(-99.99999999999999): expected -331.85460524980596, got -331.86198661322277 (error = 0.00738 (129854318490 ulps); permitted error = 1e-15 or 5 ulps)
  lgam0106: lgamma(2.5599[833](https://github.com/python/cpython/actions/runs/4121854833/jobs/7117957687#step:7:834)2785163e+305): expected 1.79769313486231e+308, got inf (not equal)
  lgam0107: lgamma(2.55998332785164e+305): expected 'OverflowError', got inf (not equal)
  lgam0108: lgamma(1.7e+308): expected 'OverflowError', got inf (not equal)
  gam0048: gamma(5.5e-309): expected 'OverflowError', got inf (not equal)
  gam0049: gamma(1e-309): expected 'OverflowError', got inf (not equal)
  gam0050: gamma(1e-323): expected 'OverflowError', got inf (not equal)
  gam0051: gamma(5e-324): expected 'OverflowError', got inf (not equal)
  gam0068: gamma(-5.5e-309): expected 'OverflowError', got -inf (not equal)
  gam0069: gamma(-1e-309): expected 'OverflowError', got -inf (not equal)
  gam0070: gamma(-1e-323): expected 'OverflowError', got -inf (not equal)
  gam0071: gamma(-5e-324): expected 'OverflowError', got -inf (not equal)
  gam0103: gamma(171.625): expected 'OverflowError', got inf (not equal)
  gam0104: gamma(172.0): expected 'OverflowError', got inf (not equal)
  gam0105: gamma(2000.0): expected 'OverflowError', got inf (not equal)
  gam0106: gamma(1.7e+308): expected 'OverflowError', got inf (not equal)
  gam0122: gamma(-170.5): expected -3.3127395215386074e-308, got -3.312739521538751e-308 (error = 1.44e-321 (291 ulps); permitted error = 0 or 20 ulps)

arhadthedev commented on Feb 8, 2023

arhadthedev

on Feb 8, 2023

Member

More actions

For Windows (x64):

======================================================================
FAIL: test_mtestfile (test.test_math.MathTests.test_mtestfile)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "D:\a\cpython\cpython\Lib\test\test_math.py", line 2053, in test_mtestfile
    self.fail('Failures in test_mtestfile:\n  ' +
AssertionError: Failures in test_mtestfile:
  lgam0085: lgamma(-99.99999999999999): expected -331.85460524980596, got -331.8619866132228 (error = 0.00738 (129854318491 ulps); permitted error = 1e-15 or 5 ulps)
  gam0085: gamma(-99.99999999999999): expected 7.540083334884096e-145, got 7.484632144060727e-145 (error = 5.55e-147 (38979707393610 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)

For Windows (x86):

======================================================================
FAIL: test_mtestfile (test.test_math.MathTests.test_mtestfile)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "D:\a\cpython\cpython\Lib\test\test_math.py", line 2053, in test_mtestfile
    self.fail('Failures in test_mtestfile:\n  ' +
AssertionError: Failures in test_mtestfile:
  lgam0085: lgamma(-99.99999999999999): expected -331.85460524980596, got -331.8619866132228 (error = 0.00738 (129854318491 ulps); permitted error = 1e-15 or 5 ulps)
  gam0085: gamma(-99.99999999999999): expected 7.540083334884096e-145, got 7.484632144060727e-145 (error = 5.55e-147 (389797073[936](https://github.com/python/cpython/actions/runs/4121854833/jobs/7117957822#step:5:936)10 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)
----------------------------------------------------------------------

skirpichev commented on Feb 8, 2023
skirpichev
on Feb 8, 2023
MemberAuthor
More actions
Yes, that's from that 5+ old PR, right? But not for erf/erfc.
arhadthedev commented on Feb 8, 2023
arhadthedev
on Feb 8, 2023
Member
More actions
Unfortunately, that's from your PR: https://github.com/python/cpython/actions/runs/4121854833. It seems that C standard library implementations didn't get better.

However, erf really has no flaws here.
skirpichev commented on Feb 8, 2023
skirpichev
on Feb 8, 2023
MemberAuthor
More actions
Indeed. Could we test the first commit only too?
mdickinson commented on Feb 8, 2023
mdickinson
on Feb 8, 2023
Member
More actions

Isn't that the case where fixes should be submitted to upstream?

Good luck with that. Upstream often isn't open source. I've certainly made bug reports on these topics upstream in the past, with typically zero response.
mdickinson commented on Feb 8, 2023
mdickinson
on Feb 8, 2023
Member
More actions

At least, there is no regular testing.

True - for erf and erfc, we're never not going to be using the libm versions on current platforms, and it seems that using the libm versions hasn't caused error reports in recent years. It does seem reasonable to drop the custom implementations there.

For gamma and lgamma, let's add a note to the source (possibly referencing the relevant GitHub issues / PRs) that explains why we're using a custom implementation.
Reacted by Oleg Iarygin and Eric V. Smith
skirpichev commented on Feb 8, 2023
skirpichev
on Feb 8, 2023
MemberAuthor
More actions

True - for erf and erfc

It seems, also for log2.
mdickinson commented on Feb 8, 2023
mdickinson
on Feb 8, 2023
Member
More actions

It seems, also for log2.

Yes, agreed; we should be able to assume that HAVE_LOG2 is always defined at this point.
skirpichev
added a commit that references this issue on Feb 8, 2023
UPD commit msg: pythongh-101678: refactor the math module to use erf/…
538810d
miss-islington
added a commit that references this issue on Feb 9, 2023
gh-101678: refactor the math module to use special functions from c11 (…
5839575
mdickinson
added a commit that references this issue on Feb 9, 2023
gh-101678: Merge math_1_to_whatever() and math_1() (#101730)
45fa12a
mdickinson commented on Feb 9, 2023
mdickinson
on Feb 9, 2023
Member
More actions
Closed by ~~#101678~~ #101679.
mdickinson
closed this as completed on Feb 9, 2023
carljm
added a commit that references this issue on Feb 9, 2023
Merge branch 'main' into inlinecomp2
be3becc
erlend-aasland commented on Feb 10, 2023
erlend-aasland
on Feb 10, 2023 · Hidden as outdated
show commentMore actions
mdickinson commented on Feb 10, 2023
mdickinson
on Feb 10, 2023
Member
More actions
@erlend-aasland Gah. Thanks! I'll edit the message.
Reacted by Erlend E. Aasland

Metadata

Assignees

No one assigned

Labels

extension-modulesC modules in the Modules dirC modules in the Modules dirtype-featureA feature request or enhancementA feature request or enhancement

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions

You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.

GeezerVison

Refactor math module to use C99+ special functions (erf, erfc, etc) #101678

Description

Linked PRs

Activity

skirpichev commented on Feb 8, 2023

arhadthedev commented on Feb 8, 2023

arhadthedev commented on Feb 8, 2023

arhadthedev commented on Feb 8, 2023

mdickinson commented on Feb 8, 2023

mdickinson commented on Feb 8, 2023

mdickinson commented on Feb 8, 2023

skirpichev commented on Feb 8, 2023

arhadthedev commented on Feb 8, 2023

arhadthedev commented on Feb 8, 2023

skirpichev commented on Feb 8, 2023

arhadthedev commented on Feb 8, 2023

skirpichev commented on Feb 8, 2023

mdickinson commented on Feb 8, 2023

mdickinson commented on Feb 8, 2023

skirpichev commented on Feb 8, 2023

mdickinson commented on Feb 8, 2023

mdickinson commented on Feb 9, 2023

erlend-aasland commented on Feb 10, 2023

mdickinson commented on Feb 10, 2023

Metadata

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Issue actions