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

Remove outdated mpmath Sage backend #38113

Open
wants to merge 11 commits into
base: develop
Choose a base branch
from

Conversation

mkoeppe
Copy link
Contributor

@mkoeppe mkoeppe commented May 29, 2024

This is the first step of preparation for mpmath 1.4

📝 Checklist

  • The title is concise and informative.
  • 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 and checked the documentation preview.

⌛ Dependencies

@mkoeppe mkoeppe added this to the sage-10.5 milestone May 30, 2024
Copy link

github-actions bot commented May 30, 2024

Documentation preview for this PR (built with commit da04709; changes) is ready! 🎉
This preview will update shortly after each push to this PR.

@kwankyu
Copy link
Collaborator

kwankyu commented Aug 21, 2024

@asmeurer

@kiwifb
Copy link
Member

kiwifb commented Aug 23, 2024

I get plenty of doctest errors of the kind

mpmath_create() expects 'mpz','int'[,'int','str'] arguments

like in

sage -t --long --random-seed=116790357055389508556504894381735541723 /usr/lib/python3.12/site-packages/sage/symbolic/function.pyx
**********************************************************************
File "/usr/lib/python3.12/site-packages/sage/symbolic/function.pyx", line 960, in sage.symbolic.function.BuiltinFunction.?
Failed example:
    cos(mpmath.mpc(1,1))
Exception raised:
    Traceback (most recent call last):
      File "/usr/lib/python3.12/site-packages/sage/doctest/forker.py", line 716, in _run
        self.compile_and_execute(example, compiler, test.globs)
      File "/usr/lib/python3.12/site-packages/sage/doctest/forker.py", line 1146, in compile_and_execute
        exec(compiled, globs)
      File "<doctest sage.symbolic.function.BuiltinFunction.?[10]>", line 1, in <module>
        cos(mpmath.mpc(Integer(1),Integer(1)))
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
      File "/usr/lib/python3.12/site-packages/mpmath/ctx_mp_python.py", line 562, in __new__
        r_real, r_imag = map(cls.context.mpf, [r_real, r_imag])
        ^^^^^^^^^^^^^^
      File "/usr/lib/python3.12/site-packages/mpmath/ctx_mp_python.py", line 69, in __new__
        val = cls.mpf_convert_arg(val, prec, rounding, base)
              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
      File "/usr/lib/python3.12/site-packages/mpmath/ctx_mp_python.py", line 79, in mpf_convert_arg
        if isinstance(x, numbers.Rational): return from_rational(x.numerator,
                                                   ^^^^^^^^^^^^^^^^^^^^^^^^^^
      File "/usr/lib/python3.12/site-packages/mpmath/libmp/libmpf.py", line 406, in from_rational
        return mpf_div(from_int(p), from_int(q), prec, rnd)
                       ^^^^^^^^^^^
      File "/usr/lib/python3.12/site-packages/mpmath/libmp/libmpf.py", line 249, in from_int
        return from_man_exp(n, 0, prec, rnd)
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    TypeError: mpmath_create() expects 'mpz','int'[,'int','str'] arguments
**********************************************************************

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Aug 23, 2024

If you use your own version of src/sage/env.py or set MPMATH_* variables elsewhere, check if you have made the changes made here

@kiwifb
Copy link
Member

kiwifb commented Aug 25, 2024

I do not have any changes to env.py anymore. I have nothing about mpmath in sage_conf. From a sage session

fbissey@tarazed ~ $ sage
┌────────────────────────────────────────────────────────────────────┐
│ SageMath version 10.5.beta2, Release Date: 2024-08-10              │
│ Using Python 3.12.5. Type "help()" for help.                       │
└────────────────────────────────────────────────────────────────────┘
┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┓
┃ Warning: this is a prerelease version, and it may be unstable.     ┃
┗━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┛
sage: import sage.env
sage: sage.env.MPMATH_SAGE
---------------------------------------------------------------------------
AttributeError                            Traceback (most recent call last)
Cell In[2], line 1
----> 1 sage.env.MPMATH_SAGE

AttributeError: module 'sage.env' has no attribute 'MPMATH_SAGE'
sage: sage.env.MPMATH_NOSAGE
---------------------------------------------------------------------------
AttributeError                            Traceback (most recent call last)
Cell In[3], line 1
----> 1 sage.env.MPMATH_NOSAGE

AttributeError: module 'sage.env' has no attribute 'MPMATH_NOSAGE'
sage: import os
sage: os.getenv("MPMATH_SAGE")
sage: os.getenv("MPMATH_NOSAGE")
'1'

I will try some test runs with those variables defined that way in the running shel.

@kiwifb
Copy link
Member

kiwifb commented Aug 25, 2024

No luck and it happens outside of the testing framework too

fbissey@tarazed ~ $ sage
┌────────────────────────────────────────────────────────────────────┐
│ SageMath version 10.5.beta2, Release Date: 2024-08-10              │
│ Using Python 3.12.5. Type "help()" for help.                       │
└────────────────────────────────────────────────────────────────────┘
┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┓
┃ Warning: this is a prerelease version, and it may be unstable.     ┃
┗━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┛
sage: import os
sage: os.getenv("MPMATH_SAGE")
sage: os.getenv("MPMATH_NOSAGE")
'1'
sage: import mpmath
sage: os.getenv("MPMATH_SAGE")
sage: os.getenv("MPMATH_NOSAGE")
'1'
sage: cos(mpmath.mpf('1.321412'))
mpf('0.24680737898640387')
sage: cos(mpmath.mpc(1,1))
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
Cell In[8], line 1
----> 1 cos(mpmath.mpc(Integer(1),Integer(1)))

File /usr/lib/python3.12/site-packages/mpmath/ctx_mp_python.py:562, in _mpc.__new__(cls, real, imag)
    560 else:
    561     i_real, i_imag = imag, 0
--> 562 r_real, r_imag = map(cls.context.mpf, [r_real, r_imag])
    563 i_real, i_imag = map(cls.context.mpf, [i_real, i_imag])
    564 real = r_real - i_imag

File /usr/lib/python3.12/site-packages/mpmath/ctx_mp_python.py:69, in _mpf.__new__(cls, val, **kwargs)
     67         raise ValueError
     68 else:
---> 69     val = cls.mpf_convert_arg(val, prec, rounding, base)
     70 v._mpf_ = mpf_pos(val, prec, rounding)
     71 return v

File /usr/lib/python3.12/site-packages/mpmath/ctx_mp_python.py:79, in _mpf.mpf_convert_arg(cls, x, prec, rounding, base)
     77 if isinstance(x, str): return from_str(x, prec, rounding, base)
     78 if isinstance(x, cls.context.constant): return x.func(prec, rounding)
---> 79 if isinstance(x, numbers.Rational): return from_rational(x.numerator,
     80                                                          x.denominator,
     81                                                          prec, rounding)
     82 if hasattr(x, '_mpf_'): return x._mpf_
     83 if hasattr(x, '_mpmath_'):

File /usr/lib/python3.12/site-packages/mpmath/libmp/libmpf.py:406, in from_rational(p, q, prec, rnd)
    403 def from_rational(p, q, prec, rnd=round_fast):
    404     """Create a raw mpf from a rational number p/q, round if
    405     necessary."""
--> 406     return mpf_div(from_int(p), from_int(q), prec, rnd)

File /usr/lib/python3.12/site-packages/mpmath/libmp/libmpf.py:249, in from_int(n, prec, rnd)
    247     if n in int_cache:
    248         return int_cache[n]
--> 249 return from_man_exp(n, 0, prec, rnd)

TypeError: mpmath_create() expects 'mpz','int'[,'int','str'] arguments

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Aug 25, 2024

Is this with the same mpmath version as in the Sage distribution?

@kiwifb
Copy link
Member

kiwifb commented Aug 25, 2024

I was going to comment about that. I use mpmath 1.4.0a1 to get python 3.13 support. So no, but it is indicative of future behavior.
Additionally, I need the stuff here to build the documentation with that version of mpmath.

@kiwifb
Copy link
Member

kiwifb commented Aug 25, 2024

In this version of mpmath, the only thing tested is MPMATH_NOGMPY - I am now wondering if it is a mpmath bug and not something we can correct in sage.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Aug 25, 2024

I have so far not tested with anything in the 1.4 development series.
For this PR, I would set an upper bound mpmath <1.4

@kiwifb
Copy link
Member

kiwifb commented Aug 25, 2024

OK, I will try to get to the bottom of this separately. It looks like coercion from sage integer to something mpmath likes is at fault.

@kiwifb
Copy link
Member

kiwifb commented Aug 26, 2024

For the record, I dug a bit deeper and the message is not quite what I thought it was about. It ultimately comes from gmpy2 (and I thought for a moment that it could be a change in gmpy 2.2.1). A closer examination of the code shows that this particular complaint is displayed when less than two arguments are passed to mpmath_create. On the other hand doing the same operation (mpmath.mpc(1, 1)) in python rather than sage does works.

It literally looks like sage integers (real numbers are fine) are reduced to "None" in mpmath 1.4a.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Aug 26, 2024

Thanks for the update. I've opened #38565 to reproduce this, and as a possible target PR for an continuous integration workflow for mpmath

@skirpichev
Copy link

Maybe you could include tests? // #36447

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Aug 27, 2024

Maybe you could include tests? // #36447

Good idea, I've cherry-picked one of your commits there

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment