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

build/pkgs/mathics: Reduce to a pip package #37395

Merged
merged 16 commits into from
Jul 24, 2024

Conversation

mkoeppe
Copy link
Contributor

@mkoeppe mkoeppe commented Feb 19, 2024

Our Mathics package is outdated (and also the interface shows test failures in CI - e.g. in https://github.com/sagemath/sage/actions/runs/7894201915/job/21638423818#step:14:3279)

Trying to update Mathics reveals more dependencies, include scikit-image and llvm-lite.

Here we reduce it to a "pip" package to simplify maintenance of the package on our side.

The latest released version 6.0.4 fails to install in Sage because of rigid version constraints.

We are using the current HEAD version (7.0.0dev), which includes

📝 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

Copy link

github-actions bot commented Apr 5, 2024

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

@mkoeppe mkoeppe force-pushed the mathics_reduce_to_pip branch 2 times, most recently from e2dc845 to 96a8d8f Compare May 27, 2024 07:19
@mkoeppe mkoeppe marked this pull request as ready for review May 27, 2024 07:22
@mkoeppe mkoeppe self-assigned this May 27, 2024
@mkoeppe
Copy link
Contributor Author

mkoeppe commented May 27, 2024

With the current branch,

sage: import mathics
sage: mathics(pi/2)
File ~/s/sage/sage-rebasing/worktree-algebraic-2018-spring/local/var/lib/sage/venv-python3.12/lib/python3.12/site-packages/mathics/core/load_builtin.py:119, in definition_contribute(definitions)
    117 def definition_contribute(definitions):
    118     # let MakeBoxes contribute first
--> 119     _builtins["System`MakeBoxes"].contribute(definitions)
    120     for name, item in _builtins.items():
    121         if name != "System`MakeBoxes":

KeyError: 'System`MakeBoxes'

@mkoeppe
Copy link
Contributor Author

mkoeppe commented May 29, 2024

@rocky @mmatera @soehms Any idea what might be going wrong here?

@rocky
Copy link

rocky commented May 29, 2024

I looked at https://github.com/sagemath/sage/actions/runs/9291389646/job/25570382749?pr=37395 and do not see how this:

 File "sage/structure/formal_sum.py", line 25, in sage.structure.formal_sum
[sagemath_categories-10.4.beta7] [spkg-check] Failed example:
[sagemath_categories-10.4.beta7] [spkg-check]     A = FormalSum([(1, 2/3)]); A
[sagemath_categories-10.4.beta7] [spkg-check] Exception raised:
[sagemath_categories-10.4.beta7] [spkg-check]     Traceback (most recent call last):
[sagemath_categories-10.4.beta7] [spkg-check]       File "/sage/pkgs/sagemath-categories/.tox/sagepython-sagewheels-nopypi-norequirements/lib/python3.10/site-packages/sage/doctest/forker.py", line 714, in _run
...
[sagemath_categories-10.4.beta7] [spkg-check]         self.compile_and_execute(example, compiler, test.globs)
[sagemath_categories-10.4.beta7] [spkg-check]       File "/sage/pkgs/sagemath-categories/.tox/sagepython-sagewheels-.
...
[sagemath_categories-10.4.beta7] [spkg-check]     ModuleNotFoundError: No module named 'sage.modules'

Is it?

@mkoeppe
Copy link
Contributor Author

mkoeppe commented May 29, 2024

The failure in that GH Actions workflow is unrelated, I think.

Here's a backtrace from local testing:

sage: mathics(pi/2)
/Users/mkoeppe/s/sage/sage-rebasing/worktree-algebraic-2018-spring/local/var/lib/sage/venv-python3.10/lib/python3.10/site-packages/mathics_scanner/characters.py:11: DeprecationWarning: pkg_resources is deprecated as an API. See https://setuptools.pypa.io/en/latest/pkg_resources.html
  import pkg_resources
---------------------------------------------------------------------------
KeyError                                  Traceback (most recent call last)
Cell In[1], line 1
----> 1 mathics(pi/Integer(2))

File ~/s/sage/sage-rebasing/worktree-algebraic-2018-spring/src/sage/misc/lazy_import.pyx:409, in sage.misc.lazy_import.LazyImport.__call__()
    407         True
    408     """
--> 409     return self.get_object()(*args, **kwds)
    410 
    411 def __repr__(self):

File ~/s/sage/sage-rebasing/worktree-algebraic-2018-spring/src/sage/interfaces/interface.py:306, in Interface.__call__(self, x, name)
    299     return cls(self, x, name=name)
    300 try:
    301     # Special methods do not and should not have an option to
    302     # set the name directly, as the identifier assigned by the
    303     # interface should stay consistent. An identifier with a
    304     # user-assigned name might change its value, so we return a
    305     # new element.
--> 306     result = self._coerce_from_special_method(x)
    307     return result if name is None else result.name(new_name=name)
    308 except TypeError:

File ~/s/sage/sage-rebasing/worktree-algebraic-2018-spring/src/sage/interfaces/interface.py:334, in Interface._coerce_from_special_method(self, x)
    332     s = '_gp_'
    333 try:
--> 334     return (x.__getattribute__(s))(self)
    335 except AttributeError:
    336     return self(x._interface_init_())

File ~/s/sage/sage-rebasing/worktree-algebraic-2018-spring/src/sage/structure/sage_object.pyx:917, in sage.structure.sage_object.SageObject._mathics_()
    915         import sage.interfaces.mathics
    916         G = sage.interfaces.mathics.mathics
--> 917     return self._interface_(G)
    918 
    919 _mathics_init_ = _mathematica_init_

File ~/s/sage/sage-rebasing/worktree-algebraic-2018-spring/src/sage/symbolic/expression.pyx:1202, in sage.symbolic.expression.Expression._interface_()
   1200     if is_a_constant(self._gobj):
   1201         return self.pyobject()._interface_(I)
-> 1202     return super()._interface_(I)
   1203 
   1204 def _maxima_(self, session=None):

File ~/s/sage/sage-rebasing/worktree-algebraic-2018-spring/src/sage/structure/sage_object.pyx:724, in sage.structure.sage_object.SageObject._interface_()
    722     except Exception:
    723         raise NotImplementedError("coercion of object %s to %s not implemented:\n%s\n%s" % (repr(self), I))
--> 724 X = I(s)
    725 if c:
    726     try:

File ~/s/sage/sage-rebasing/worktree-algebraic-2018-spring/src/sage/interfaces/interface.py:299, in Interface.__call__(self, x, name)
    296         pass
    298 if isinstance(x, str):
--> 299     return cls(self, x, name=name)
    300 try:
    301     # Special methods do not and should not have an option to
    302     # set the name directly, as the identifier assigned by the
    303     # interface should stay consistent. An identifier with a
    304     # user-assigned name might change its value, so we return a
    305     # new element.
    306     result = self._coerce_from_special_method(x)

File ~/s/sage/sage-rebasing/worktree-algebraic-2018-spring/src/sage/interfaces/interface.py:749, in InterfaceElement.__init__(self, parent, value, is_name, name)
    747 else:
    748     try:
--> 749         self._name = parent._create(value, name=name)
    750     except (TypeError, RuntimeError, ValueError) as x:
    751         raise TypeError(x)

File ~/s/sage/sage-rebasing/worktree-algebraic-2018-spring/src/sage/interfaces/interface.py:517, in Interface._create(self, value, name)
    515 def _create(self, value, name=None):
    516     name = self._next_var_name() if name is None else name
--> 517     self.set(name, value)
    518     return name

File ~/s/sage/sage-rebasing/worktree-algebraic-2018-spring/src/sage/interfaces/mathics.py:597, in Mathics.set(self, var, value)
    587 """
    588 Set the variable var to the given value.
    589 
   (...)
    594     True
    595 """
    596 cmd = f'{var}={value};'
--> 597 _ = self.eval(cmd)

File ~/s/sage/sage-rebasing/worktree-algebraic-2018-spring/src/sage/interfaces/mathics.py:580, in Mathics.eval(self, code, *args, **kwds)
    570 def eval(self, code, *args, **kwds):
    571     """
    572     Evaluates a command inside the Mathics interpreter and returns the output
    573     in printable form.
   (...)
    578         '2'
    579     """
--> 580     res = self._eval(code)
    581     if res.result == 'Null':
    582         if len(res.out) == 1:

File ~/s/sage/sage-rebasing/worktree-algebraic-2018-spring/src/sage/interfaces/mathics.py:563, in Mathics._eval(self, code)
    553 def _eval(self, code):
    554     """
    555     Evaluates a command inside the Mathics interpreter and returns the output
    556     as a Mathics result.
   (...)
    561         <Integer: 2>
    562     """
--> 563     self._lazy_init()
    564     S = self._session
    565     expr = S.evaluate(code)

File ~/s/sage/sage-rebasing/worktree-algebraic-2018-spring/src/sage/interfaces/mathics.py:502, in Mathics._lazy_init(self)
    500 if not self._initialized:
    501     self._initialized = True
--> 502     self._start()

File ~/s/sage/sage-rebasing/worktree-algebraic-2018-spring/src/sage/interfaces/mathics.py:518, in Mathics._start(self)
    516 if not self._session:
    517     from mathics.session import MathicsSession
--> 518     self._session = MathicsSession()
    519     from sage.interfaces.sympy import sympy_init
    520     sympy_init()

File ~/s/sage/sage-rebasing/worktree-algebraic-2018-spring/local/var/lib/sage/venv-python3.10/lib/python3.10/site-packages/mathics/session.py:76, in MathicsSession.__init__(self, add_builtin, catch_interrupt, form, character_encoding)
     74     mathics.settings.SYSTEM_CHARACTER_ENCODING = character_encoding
     75 self.form = form
---> 76 self.reset(add_builtin, catch_interrupt)

File ~/s/sage/sage-rebasing/worktree-algebraic-2018-spring/local/var/lib/sage/venv-python3.10/lib/python3.10/site-packages/mathics/session.py:82, in MathicsSession.reset(self, add_builtin, catch_interrupt)
     78 def reset(self, add_builtin=True, catch_interrupt=False):
     79     """
     80     reset the definitions and the evaluation objects.
     81     """
---> 82     self.definitions = Definitions(add_builtin)
     83     self.evaluation = Evaluation(
     84         definitions=self.definitions, catch_interrupt=catch_interrupt
     85     )
     86     self.last_result = None

File ~/s/sage/sage-rebasing/worktree-algebraic-2018-spring/local/var/lib/sage/venv-python3.10/lib/python3.10/site-packages/mathics/core/definitions.py:157, in Definitions.__init__(self, add_builtin, builtin_filename, extension_modules)
    155         loaded = True
    156 if not loaded:
--> 157     definition_contribute(self)
    158     for module in extension_modules:
    159         try:

File ~/s/sage/sage-rebasing/worktree-algebraic-2018-spring/local/var/lib/sage/venv-python3.10/lib/python3.10/site-packages/mathics/core/load_builtin.py:119, in definition_contribute(definitions)
    117 def definition_contribute(definitions):
    118     # let MakeBoxes contribute first
--> 119     _builtins["System`MakeBoxes"].contribute(definitions)
    120     for name, item in _builtins.items():
    121         if name != "System`MakeBoxes":

KeyError: 'System`MakeBoxes'
sage: 

@mmatera
Copy link
Contributor

mmatera commented May 29, 2024

It looks like mathics.core.load_builtin.import_and_load_builtins() was not called before building the interpreter.

@mmatera
Copy link
Contributor

mmatera commented May 29, 2024

My gues is, just after this line:

from mathics.session import MathicsSession

we need to add

from mathics.core.load_builtin import import_and_load_builtins
import_and_load_builtins()

in a way that when the session is created, the builtins be already loaded.

@rocky
Copy link

rocky commented May 29, 2024

It looks like mathics.core.load_builtin.import_and_load_builtins() was not called before building the interpreter.

Yes, I was going mention that too.

Let me explain. The interface has changed, and while it is not ideal it is what it is for now.

WMA has a ton of builtin functions and a longer-term project is to be able to control how collections of builtins get loaded. Previously, Mathics3 builtin functions got loaded as a batch process as a result of importing the the mathics.builtins module. (The specific order of what gets loaded when or at what point in the loading this kicked in was undetermined and probably varied as the code changed).

So that we can control things better, now you need that explicit call, import_and_load_builtins(), which makes it very deterministic when this loading happens.

We've talked about adding that call somewhere reasonable. Things haven't shaken out though yet in my opinion because all of this is a bit up in the air until we get farther down the line in terms of being able to load sets of builtins on lazily at a finer level. (I think of this analogous to GNU emacs "autoload"

@mkoeppe
Copy link
Contributor Author

mkoeppe commented May 29, 2024

Thanks a lot!

This gets me at least one step further:

sage: mathics(pi/2)
/Users/mkoeppe/s/sage/sage-rebasing/worktree-algebraic-2018-spring/local/var/lib/sage/venv-python3.10/lib/python3.10/site-packages/mathics_scanner/characters.py:11: DeprecationWarning: pkg_resources is deprecated as an API. See https://setuptools.pypa.io/en/latest/pkg_resources.html
  import pkg_resources
type sage.libs.mpmath.ext_main.mpf doesn't define __round__ method
    Not able to load mathics.builtin.testing_expressions.equality_inequality. Check your installation.
    mathics.builtin loads from /Users/mkoeppe/s/sage/sage-rebasing/worktree-algebraic-2018-spring/local/var/lib/sage/venv-python3.10/lib/python3.10/site-packages/mathics/core/load
type sage.libs.mpmath.ext_main.mpf doesn't define __round__ method
    Not able to load mathics.builtin.numbers.constants. Check your installation.
    mathics.builtin loads from /Users/mkoeppe/s/sage/sage-rebasing/worktree-algebraic-2018-spring/local/var/lib/sage/venv-python3.10/lib/python3.10/site-packages/mathics/core/load
/Users/mkoeppe/s/sage/sage-rebasing/worktree-algebraic-2018-spring/local/var/lib/sage/venv-python3.10/lib/python3.10/site-packages/mathics/core/parser/convert.py:41: DeprecationWarning: invalid escape sequence '\!'
  return s.encode("raw_unicode_escape").decode("unicode_escape")
Pi / 2

@mkoeppe
Copy link
Contributor Author

mkoeppe commented May 29, 2024

I think we're running into an incompatibility with the (outdated...) Sage backend of mpmath

@mmatera
Copy link
Contributor

mmatera commented May 29, 2024

What about if you evaluate mathics(Cos(pi/2)) instead?

@mkoeppe
Copy link
Contributor Author

mkoeppe commented May 29, 2024

sage: mathics(cos(pi/2))
/Users/mkoeppe/s/sage/sage-rebasing/worktree-algebraic-2018-spring/local/var/lib/sage/venv-python3.10/lib/python3.10/site-packages/mathics_scanner/characters.py:11: DeprecationWarning: pkg_resources is deprecated as an API. See https://setuptools.pypa.io/en/latest/pkg_resources.html
  import pkg_resources
type sage.libs.mpmath.ext_main.mpf doesn't define __round__ method
    Not able to load mathics.builtin.testing_expressions.equality_inequality. Check your installation.
    mathics.builtin loads from /Users/mkoeppe/s/sage/sage-rebasing/worktree-algebraic-2018-spring/local/var/lib/sage/venv-python3.10/lib/python3.10/site-packages/mathics/core/load
type sage.libs.mpmath.ext_main.mpf doesn't define __round__ method
    Not able to load mathics.builtin.numbers.constants. Check your installation.
    mathics.builtin loads from /Users/mkoeppe/s/sage/sage-rebasing/worktree-algebraic-2018-spring/local/var/lib/sage/venv-python3.10/lib/python3.10/site-packages/mathics/core/load
/Users/mkoeppe/s/sage/sage-rebasing/worktree-algebraic-2018-spring/local/var/lib/sage/venv-python3.10/lib/python3.10/site-packages/mathics/core/parser/convert.py:41: DeprecationWarning: invalid escape sequence '\!'
  return s.encode("raw_unicode_escape").decode("unicode_escape")
0

@mkoeppe
Copy link
Contributor Author

mkoeppe commented May 29, 2024

sage: mathics.Cos(pi/2)
/Users/mkoeppe/s/sage/sage-rebasing/worktree-algebraic-2018-spring/local/var/lib/sage/venv-python3.10/lib/python3.10/site-packages/mathics_scanner/characters.py:11: DeprecationWarning: pkg_resources is deprecated as an API. See https://setuptools.pypa.io/en/latest/pkg_resources.html
  import pkg_resources
type sage.libs.mpmath.ext_main.mpf doesn't define __round__ method
    Not able to load mathics.builtin.testing_expressions.equality_inequality. Check your installation.
    mathics.builtin loads from /Users/mkoeppe/s/sage/sage-rebasing/worktree-algebraic-2018-spring/local/var/lib/sage/venv-python3.10/lib/python3.10/site-packages/mathics/core/load
type sage.libs.mpmath.ext_main.mpf doesn't define __round__ method
    Not able to load mathics.builtin.numbers.constants. Check your installation.
    mathics.builtin loads from /Users/mkoeppe/s/sage/sage-rebasing/worktree-algebraic-2018-spring/local/var/lib/sage/venv-python3.10/lib/python3.10/site-packages/mathics/core/load
/Users/mkoeppe/s/sage/sage-rebasing/worktree-algebraic-2018-spring/local/var/lib/sage/venv-python3.10/lib/python3.10/site-packages/mathics/core/parser/convert.py:41: DeprecationWarning: invalid escape sequence '\!'
  return s.encode("raw_unicode_escape").decode("unicode_escape")
Cos[Pi / 2]

@mkoeppe
Copy link
Contributor Author

mkoeppe commented May 29, 2024

sage: mathics.N(mathics.Cos(pi/2))
/Users/mkoeppe/s/sage/sage-rebasing/worktree-algebraic-2018-spring/src/sage/repl/rich_output/display_manager.py:595: RichReprWarning: Exception in _rich_repr_ while displaying object: type sage.libs.mpmath.ext_main.mpf doesn't define __round__ method
  warnings.warn(
<repr(<sage.interfaces.mathics.MathicsElement at 0x198046000>) failed: TypeError: type sage.libs.mpmath.ext_main.mpf doesn't define __round__ method>

@mkoeppe
Copy link
Contributor Author

mkoeppe commented May 29, 2024

This looks like something that we have to sort out in Sage first (unless there's a way in Mathics to disable the use of mpmath somehow).

@soehms
Copy link
Member

soehms commented Jun 3, 2024

I'm sorry I didn't notice this PR sooner and thanks for your efforts in getting this interface back up and running! I'll see how I can help with that in the next days and weeks.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jun 3, 2024

@soehms Thanks! I think a question would be if there's something we should do for the upcoming Sage 10.4 release. If you agree with my impression that the current packages in the Sage distribution for mathics do not work at all, should we be merging the present PR as is, which at least makes it "hackable" for developers?

@mmatera
Copy link
Contributor

mmatera commented Jun 3, 2024

@soehms Thanks! I think a question would be if there's something we should do for the upcoming Sage 10.4 release. If you agree with my impression that the current packages in the Sage distribution for mathics do not work at all, should we be merging the present PR as is, which at least makes it "hackable" for developers?

The root of the problem seems to be the SAGE builtin version of mpmath. In particular, the lack of the __round__ method in mpf and mpc. Maybe it would be possible to hack these classes to support the method.

@mmatera
Copy link
Contributor

mmatera commented Jun 4, 2024

I think that adding to this class

cdef class mpf_base(mpnumber):

something like this method

    def __round__(self, *args):
        return round(float(self), *args)

it would fix the crash.

@rocky
Copy link

rocky commented Jun 22, 2024

I did some testing with the current branch as well as with 36cc26d which was merged with #38113. In both cases I got the same doctest errors, most of which are just due to changes in printing the output, which now seems to use UTF-8 symbols:

...
**********************************************************************
File "src/sage/interfaces/mathics.py", line 138, in sage.interfaces.mathics
Failed example:
    eqn
Expected:
    5 + 3 x == 14
Got:
    5 + 3 x ⩵ 14
**********************************************************************
File "src/sage/interfaces/mathics.py", line 140, in sage.interfaces.mathics
Failed example:
    eqn.Solve('x')
Expected:
    {{x -> 3}}
Got:
    {{x → 3}}
....

@mmatera The symbol for the equals sign looks a bit unconventional. In a bash commandline session it looks like this:

grafik

@soehms I meant to respond to this a while ago... Set Environment variable MATHICS_CHARACTER_ENCODING="ASCII" and these Unicode symbols will disappear. The default is "UTF-8" which will include these characters.

@soehms
Copy link
Member

soehms commented Jun 23, 2024

@soehms I meant to respond to this a while ago... Set Environment variable MATHICS_CHARACTER_ENCODING="ASCII" and these Unicode symbols will disappear. The default is "UTF-8" which will include these characters.

Thanks! I'll see how to adjust the Sage code tomorrow. What about the other question?

Furthermore, is there a way to get rid of these deprecation warnings

@rocky
Copy link

rocky commented Jun 23, 2024

/Users/mkoeppe/s/sage/sage-rebasing/worktree-algebraic-2018-spring/local/var/lib/sage/venv-python3.10/lib/python3.10/site-packages/mathics_scanner/characters.py:11: DeprecationWarning: pkg_resources is deprecated as an API. See https://setuptools.pypa.io/en/latest/pkg_resources.html

The current master branch addresses this - converts to using toml for packaging. In fact, I think @mkoeppe made the change!

A 7.0.0 release of everything is a little overdue, but until then use the master branch in git.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jun 23, 2024

The current master branch addresses this - converts to using toml for packaging. In fact, I think @mkoeppe made the change!

I did change the metadata format, but that's unrelated to the use of pkg_resources

@soehms
Copy link
Member

soehms commented Jun 24, 2024

I'll see how to adjust the Sage code tomorrow.

@mkoeppe I made a relative PR mkoeppe#45 for this.

Copy link
Member

@soehms soehms left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After the two linter fixes it looks good to me!

src/sage/libs/mpmath/ext_main.pyx Outdated Show resolved Hide resolved
src/sage/libs/mpmath/ext_main.pyx Outdated Show resolved Hide resolved
@mkoeppe mkoeppe added this to the sage-10.4 milestone Jun 27, 2024
Copy link
Member

@soehms soehms left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, it should not remove the whole line. Just the whitespaces in it! However, lets get it in!

src/sage/libs/mpmath/ext_main.pyx Show resolved Hide resolved
@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jun 27, 2024

Thanks.

@mkoeppe mkoeppe removed this from the sage-10.4 milestone Jul 21, 2024
@vbraun vbraun merged commit 0c506c2 into sagemath:develop Jul 24, 2024
30 of 40 checks passed
@mkoeppe mkoeppe added this to the sage-10.5 milestone Jul 25, 2024
@mkoeppe mkoeppe deleted the mathics_reduce_to_pip branch July 25, 2024 09:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants