gh-101678: refactor the math module to use special functions from c11 #101679
Conversation
|
🤖 New build scheduled with the buildbot fleet by @mdickinson for commit 7d14105 🤖 If you want to schedule another build, you need to add the |
|
What's the benefit of doing this? The downside is that we potentially introduce a quality regression on some platforms. |
This reverts commit 7d14105.
Less C code to maintain, perhaps? BTW, for erf/erfc there are no failures so far.
It's not a for error functions, because platforms claims that they support C11 and libm's versions will play the game anyway ;) There is no such preprocessor flag as |
|
🤖 New build scheduled with the buildbot fleet by @mdickinson for commit 468096b 🤖 If you want to schedule another build, you need to add the |
If the patch satisfies C11 compiler requirement and PEP 11 support, it could be acceptable. |
|
@corona10 Yes, the "this" in my comment could have been clearer. So long as we keep this PR focused on removing what's now effectively dead code (the code that depends on FWIW, speaking as the original author of the |
|
@skirpichev Thanks for the PR! I'll have time to review properly this evening (UTC+0). |
That was essentially the target of the issue. Thank you for review and I'm sorry for a missed issue/pr (adding c11 keyword to search was too restrictive this time...) |
|
Maybe using math_1 wrapper for erf/erfc is more safe? |
I don't think it's needed, since |
Co-authored-by: Mark Dickinson <dickinsm@gmail.com>
Same is true to the tanh, for example, isn't? |
|
I would suggest merging math_1 and math_1_to_whatever (not used anywhere else). The comment before math_1_to_whatever does mention math_1 instead. Ditto for math_1_to_whatever. (One function call less, BTW) @mdickinson, please review last two commits. |
BTW, C11 Annex F says: log1p(±0) returns ±0
Sounds reasonable, but for ease of review let's have a separate PR for that, please! I'd like to get this one in, and that gets harder if the scope keeps changing. |
Very likely, yes. Note that that's not a reason to change |
This reverts commit 47970e2.
Ok, that part (47970e2) was reverted and I've fixed prefixes for gh issues. |
Aside from some tiny performance bonus. |
|
Status check is done, and it's a success ✅. |
|
Aargh. And now I remember (again) why I never use the auto-merge label - there's no opportunity to edit the final commit message. :-( |
|
Thanks for the hint. Next time I'll adjust pr title/body to follow review. |
* main: (82 commits) pythongh-101670: typo fix in PyImport_ExtendInittab() (python#101723) pythonGH-99293: Document that `Py_TPFLAGS_VALID_VERSION_TAG` shouldn't be used. (#pythonGH-101736) no-issue: Add Dong-hee Na as the cjkcodecs codeowner (pythongh-101731) pythongh-101678: Merge math_1_to_whatever() and math_1() (python#101730) pythongh-101678: refactor the math module to use special functions from c11 (pythonGH-101679) pythongh-85984: Remove legacy Lib/pty.py code. (python#92365) pythongh-98831: Use opcode metadata for stack_effect() (python#101704) pythongh-101283: Version was just released, so should be changed in 3.11.3 (pythonGH-101719) pythongh-101283: Fix use of unbound variable (pythonGH-101712) pythongh-101283: Improved fallback logic for subprocess with shell=True on Windows (pythonGH-101286) pythongh-101277: Port more itertools static types to heap types (python#101304) pythongh-98831: Modernize CALL and family (python#101508) pythonGH-101696: invalidate type version tag in `_PyStaticType_Dealloc` (python#101697) pythongh-100221: Fix creating dirs in `make sharedinstall` (pythonGH-100329) pythongh-101670: typo fix in PyImport_AppendInittab() (pythonGH-101672) pythongh-101196: Make isdir/isfile/exists faster on Windows (pythonGH-101324) pythongh-101614: Don't treat python3_d.dll as a Python DLL when checking extension modules for incompatibility (pythonGH-101615) pythongh-100933: Improve `check_element` helper in `test_xml_etree` (python#100934) pythonGH-101578: Normalize the current exception (pythonGH-101607) pythongh-47937: Note that Popen attributes are read-only (python#93070) ...
|
Nice change.
In the past, I tried to document these "build changes", just in case if someone is affected. Like:
https://docs.python.org/dev/whatsnew/3.10.html#build-changes |
|
c11 compiler and stdlib (without optional features) were required since 3.11, so there is nothing new in that sense. |
Shouldn't affect users, hence no news.
Automerge-Triggered-By: GH:mdickinson