sagemath-mpmath #35906

mkoeppe · 2023-07-05T19:24:33Z

📚 Description

Instead of relying on mpmath's mechanism to select a backend, which is problematic when other libraries use mpmath too (#25445), we create an isolated copy of mpmath in sage.libs.mpmath._vendor.mpmath. The copy is created at build/sdist time using the pypa vendoring tool; we do not put a copy of the sources into our git source tree.

All uses of mpmath in the Sage library are updated. The Sage library no longer imports from mpmath and no longer sets environment variables that force mpmath (when imported by another library) to use the Sage backend.

The library is intended to be shipped by the pip-installable distribution sagemath-mpmath (added here on the PR), which only has sagemath-categories as a Python dependency.
Downstream projects that wish to use this version of mpmath should declare a dependency (install-requires) on sagemath-mpmath and use import sage.libs.mpmath.all instead of import mpmath.

📝 Checklist

The title is concise, informative, and self-explanatory.
The description explains in detail what this PR is about.
I have linked a relevant issue or discussion.
I have created tests covering the changes.
I have updated the documentation accordingly.

⌛ Dependencies

github-actions · 2023-07-07T00:18:01Z

Documentation preview for this PR (built with commit 1e3218c; changes) is ready! 🎉

dimpase · 2023-07-08T13:16:13Z

what kinds of simplifications could be done on mpmath side here - assuming this is merged?

mkoeppe · 2023-07-08T17:02:11Z

what kinds of simplifications could be done on mpmath side here - assuming this is merged?

I would suggest that they remove the condition 'SAGE_ROOT' in os.environ from https://github.com/mpmath/mpmath/blob/master/mpmath/libmp/backend.py#L41 (in fact, this could be done already now because Sage has not depended on this mechanism since #32799 (because of the needs of downstream, @antonio-rojas @kiwifb)

Other than that, I don't think anything gets simpler for mpmath itself; but for users of mpmath and Sage, the situation becomes clearer and more robust.

dimpase · 2023-07-10T11:11:02Z

I don't like vendoring, and this looks way overblown anyway.
Can the same be achieved by modifying upstream? (I have commit rights in the upstream)

dimpase · 2023-07-10T15:32:44Z

before doing anything here, we should check whether just getting rid of Sage backend in mpmath (which is a very old hack) is the right thing to do. I.e. whether there is a serious speed regression, something I doubt.

mkoeppe · 2023-07-10T16:00:29Z

we should check whether just getting rid of Sage backend in mpmath (which is a very old hack) is the right thing to do.

+1. Do you know how to benchmark it, or should we get help from upstream?

dimpase · 2023-07-10T17:52:47Z

it's a mixed bag - some tests are slower, some are faster. E.g. here are 5 slowest tests from
mpmath/mpmath/tests/test_torture.py

(sage-sh) dima@hilbert:sage-dev$ MPMATH_NOSAGE=yes pytest test_torture.py --durations=5
==================================================================== test session starts =====================================================================
platform linux -- Python 3.11.4, pytest-7.4.0, pluggy-1.0.0
rootdir: /mnt/opt/Sage/sage-dev
collected 90 items                                                                                                                                           

test_torture.py ..........................................................................................                                             [100%]

==================================================================== slowest 5 durations =====================================================================
60.31s call     test_torture.py::test_asymp[<lambda>-150-False23]
12.87s call     test_torture.py::test_asymp[<lambda>-150-False9]
12.16s call     test_torture.py::test_asymp[<lambda>-90-False]
10.97s call     test_torture.py::test_asymp[<lambda>-150-False37]
10.76s call     test_torture.py::test_asymp[f_wrapped-90-False]
=============================================================== 90 passed in 286.13s (0:04:46) ===============================================================
(sage-sh) dima@hilbert:sage-dev$ pytest test_torture.py --durations=5
==================================================================== test session starts =====================================================================
platform linux -- Python 3.11.4, pytest-7.4.0, pluggy-1.0.0
rootdir: /mnt/opt/Sage/sage-dev
collected 90 items                                                                                                                                           

test_torture.py ..........................................................................................                                             [100%]

==================================================================== slowest 5 durations =====================================================================
59.20s call     test_torture.py::test_asymp[<lambda>-150-False23]
12.18s call     test_torture.py::test_asymp[<lambda>-150-False9]
10.12s call     test_torture.py::test_asymp[<lambda>-90-False]
9.85s call     test_torture.py::test_asymp[<lambda>-150-False37]
9.53s call     test_torture.py::test_asymp[barnesg-90-False]
=============================================================== 90 passed in 239.50s (0:03:59) ===============================================================

So overall with Sage backend it's about 20% faster. OTOH one test there is must faster without Sage backend:

(sage-sh) dima@hilbert:sage-dev$ MPMATH_NOSAGE=yes pytest test_torture.py -k test_bernoulli_huge
==================================================================== test session starts =====================================================================
platform linux -- Python 3.11.4, pytest-7.4.0, pluggy-1.0.0
rootdir: /mnt/opt/Sage/sage-dev
collected 90 items / 89 deselected / 1 selected                                                                                                              

test_torture.py .                                                                                                                                      [100%]

============================================================== 1 passed, 89 deselected in 0.32s ==============================================================
(sage-sh) dima@hilbert:sage-dev$ pytest test_torture.py -k test_bernoulli_huge
==================================================================== test session starts =====================================================================
platform linux -- Python 3.11.4, pytest-7.4.0, pluggy-1.0.0
rootdir: /mnt/opt/Sage/sage-dev
collected 90 items / 89 deselected / 1 selected                                                                                                              

test_torture.py .                                                                                                                                      [100%]

============================================================== 1 passed, 89 deselected in 4.08s ==============================================================

jhpalmieri · 2023-10-05T05:00:08Z

build/pkgs/vendoring/dependencies

@@ -0,0 +1,4 @@
+$(PYTHON) rich click toml | $(PYTHON_TOOLCHAIN)


There is no toml package in Sage. Should this be tomli instead?

the toml package was removed from Sage in the meantime; we'll have to re-add it for sagemath-mpmath

jhpalmieri · 2023-10-05T05:21:47Z

build/pkgs/rich/dependencies

@@ -0,0 +1,4 @@
+$(PYTHON) markdown_it_py | $(PYTHON_TOOLCHAIN)


This seems to require poetry-core?

Not any more, I've now made it a wheel package, so there's no more build dependency.

…sagemath-mpmath

…o run vendoring script # Conflicts: # build/pkgs/sagelib/dependencies

…SymPy) does not use Sage types

…pmath

pkgs/sagemath-mpmath/pyproject.toml.m4: Protect the nodoctest file

…install, do not erase sage in site-packages

…py files in doctest

mkoeppe · 2023-10-09T00:13:42Z

partially squashed

skirpichev · 2023-10-18T03:33:35Z

@dimpase, I assume you did tests without gmpy2. Is there advantages of using sage backend c.f. gmpy?

we should check whether just getting rid of Sage backend in mpmath

Which will be a simplest solution for mpmath/mpmath#705...

mkoeppe · 2024-05-29T20:12:52Z

Closing this PR as this change appears to be slated for inclusion in mpmath 1.4:

Remove sage backend mpmath/mpmath#732

mkoeppe force-pushed the sagemath_mpmath branch 2 times, most recently from f1f81fb to 53ec31a Compare July 5, 2023 21:45

mkoeppe added c: packages: standard c: distribution t: refactoring labels Jul 5, 2023

jhpalmieri reviewed Oct 5, 2023

View reviewed changes

mkoeppe self-assigned this Oct 8, 2023

mkoeppe force-pushed the sagemath_mpmath branch 2 times, most recently from 4f66aaa to 984cf52 Compare October 9, 2023 00:03

build/pkgs/vendoring: New, plus its dependencies

bf975b2

mkoeppe force-pushed the sagemath_mpmath branch from 984cf52 to efaffe6 Compare October 9, 2023 00:04

Matthias Koeppe added 9 commits October 8, 2023 17:05

build/pkgs/sagemath_mpmath, pkgs/sagemath-mpmath: New

8a1c726

pkgs/sagemath-standard/setup.py: Accept sage.doctest: distribution = …

7c21631

…sagemath-mpmath

build/pkgs/sagelib/dependencies: Add dependency on sagemath_mpmath, t…

268612b

…o run vendoring script # Conflicts: # build/pkgs/sagelib/dependencies

sage.env: Make sure that a non-vendored copy of mpmath (e.g. used by …

b9bc5d5

…SymPy) does not use Sage types

src/sage/features/standard.py: Switch 'mpmath' feature to sage.libs.m…

b074bf7

…pmath

src/tox.ini (relint): Exclude vendored mpmath

fbb0337

Do not doctest vendored mpmath

4c01b98

pkgs/sagemath-mpmath/pyproject.toml.m4: Protect the nodoctest file

src/sage/libs/mpmath: Eliminate _vendor/__init__.py; in sagelib/spkg-…

2445784

…install, do not erase sage in site-packages

src/sage/doctest/sources.py (_test_enough_doctests): Honor nodoctest.…

0527c26

…py files in doctest

mkoeppe force-pushed the sagemath_mpmath branch from efaffe6 to 0527c26 Compare October 9, 2023 00:05

dimpase mentioned this pull request Oct 18, 2023

Test sage integration or remove sage backend mpmath/mpmath#705

Closed

mkoeppe added the s: needs info label Oct 24, 2023

vbraun force-pushed the develop branch from 883e05f to e349b00 Compare November 12, 2023 16:25

mkoeppe closed this May 29, 2024

github-actions bot removed the s: needs info label May 29, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

sagemath-mpmath #35906

sagemath-mpmath #35906

mkoeppe commented Jul 5, 2023 •

edited

Loading

github-actions bot commented Jul 7, 2023

dimpase commented Jul 8, 2023

mkoeppe commented Jul 8, 2023 •

edited

Loading

dimpase commented Jul 10, 2023

dimpase commented Jul 10, 2023

mkoeppe commented Jul 10, 2023

dimpase commented Jul 10, 2023 •

edited

Loading

jhpalmieri Oct 5, 2023

mkoeppe Oct 5, 2023

jhpalmieri Oct 5, 2023

mkoeppe Oct 8, 2023

mkoeppe commented Oct 9, 2023

skirpichev commented Oct 18, 2023

mkoeppe commented May 29, 2024

		@@ -0,0 +1,4 @@
		$(PYTHON) rich click toml \| $(PYTHON_TOOLCHAIN)

		@@ -0,0 +1,4 @@
		$(PYTHON) markdown_it_py \| $(PYTHON_TOOLCHAIN)

sagemath-mpmath #35906

sagemath-mpmath #35906

Conversation

mkoeppe commented Jul 5, 2023 • edited Loading

📚 Description

📝 Checklist

⌛ Dependencies

github-actions bot commented Jul 7, 2023

dimpase commented Jul 8, 2023

mkoeppe commented Jul 8, 2023 • edited Loading

dimpase commented Jul 10, 2023

dimpase commented Jul 10, 2023

mkoeppe commented Jul 10, 2023

dimpase commented Jul 10, 2023 • edited Loading

jhpalmieri Oct 5, 2023

Choose a reason for hiding this comment

mkoeppe Oct 5, 2023

Choose a reason for hiding this comment

jhpalmieri Oct 5, 2023

Choose a reason for hiding this comment

mkoeppe Oct 8, 2023

Choose a reason for hiding this comment

mkoeppe commented Oct 9, 2023

skirpichev commented Oct 18, 2023

mkoeppe commented May 29, 2024

mkoeppe commented Jul 5, 2023 •

edited

Loading

mkoeppe commented Jul 8, 2023 •

edited

Loading

dimpase commented Jul 10, 2023 •

edited

Loading