sagemath / sage

Main repository of SageMath. Now open for Issues and Pull Requests.
https://www.sagemath.org
Other
1.19k stars 411 forks source link

reading multivariate polynomials from strings hits RecursionError #37641

Open alexjbest opened 3 months ago

alexjbest commented 3 months ago

Steps To Reproduce

sage: R = PolynomialRing(QQ,"x,y")
....: R("+".join(["x"]*10000))

Expected Behavior

10000*x

Actual Behavior

---------------------------------------------------------------------------
RecursionError                            Traceback (most recent call last)
Cell In[8], line 2
      1 R = PolynomialRing(QQ,"x,y")
----> 2 R("+".join(["x"]*Integer(10000)))

File ~/mambaforge/envs/sage/lib/python3.10/site-packages/sage/structure/parent.pyx:901, in sage.structure.parent.Parent.__call__ (build/cythonized/sage/structure/parent.c:12682)()
    899 if mor is not None:
    900     if no_extra_args:
--> 901         return mor._call_(x)
    902     else:
    903         return mor._call_with_args(x, args, kwds)

File ~/mambaforge/envs/sage/lib/python3.10/site-packages/sage/structure/coerce_maps.pyx:163, in sage.structure.coerce_maps.DefaultConvertMap_unique._call_ (build/cythonized/sage/structure/coerce_maps.c:6958)()
    161             print(type(C), C)
    162             print(type(C._element_constructor), C._element_constructor)
--> 163         raise
    164 
    165 cpdef Element _call_with_args(self, x, args=(), kwds={}) noexcept:

File ~/mambaforge/envs/sage/lib/python3.10/site-packages/sage/structure/coerce_maps.pyx:158, in sage.structure.coerce_maps.DefaultConvertMap_unique._call_ (build/cythonized/sage/structure/coerce_maps.c:6850)()
    156 cdef Parent C = self._codomain
    157 try:
--> 158     return C._element_constructor(x)
    159 except Exception:
    160     if print_warnings:

File ~/mambaforge/envs/sage/lib/python3.10/site-packages/sage/rings/polynomial/multi_polynomial_libsingular.pyx:989, in sage.rings.polynomial.multi_polynomial_libsingular.MPolynomialRing_libsingular._element_constructor_ (build/cythonized/sage/rings/polynomial/multi_polynomial_libsingular.cpp:19501)()
    987     else:
    988         element = element.replace("^","**")
--> 989         element = eval(element, d, {})
    990 except (SyntaxError, NameError):
    991     raise TypeError("Could not find a mapping of the passed element to this ring.")

RecursionError: maximum recursion depth exceeded during compilation

Additional Information

works ok for single variable polynomials

Environment

- **OS**: osx
- **Sage Version**: 10.3

Checklist

grhkm21 commented 3 months ago

This is a Python issue:

sage: eval("+".join(["1"]*10000))
Traceback (most recent call last):
...
RecursionError: maximum recursion depth exceeded during compilation

So unless we do our own parsing with a buffer stack (which this comment suggests that it's actively avoided, though someone can definitely take on this task...) this should be unavoidable.

alexjbest commented 3 months ago

Yes I would think we should replace eval with something more direct at least in some cases, it feels quite ugly to use eval for this to me, I assume that comment means it's just for simplicity / genericity. In this case the polynomial is already in a normal form with no brackets so we can for example split the string on + and - and then polynomialize each piece and then sum them. I think implementing a procedure like this would cover many use cases (and importantly would cover the case of parsing polynomials coming from external systems, e.g. via sage's msolve wrapper).

edit: actually one would need more than this in order for this mwe to work. But the fact that univariate polynomials handle this ok to me seems something must be possible, and to me it seems worth having