Skip to content

Commit b0aaac8

Browse files
author
Release Manager
committed
gh-35210: Refactor subs() of multivariate polynomials for readability and efficiency <!-- ^^^^^ Please provide a concise, informative and self-explanatory title. Don't put issue numbers in there, do this in the PR body below. For example, instead of "Fixes #1234" use "Introduce new method to calculate 1+1" --> ### 📚 Description Fixes #34591 <!-- Describe your changes here in detail --> <!-- Why is this change required? What problem does it solve? --> <!-- If it resolves an open issue, please link to the issue here. For example "Closes #1337" --> ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. --> <!-- If your change requires a documentation PR, please link it appropriately --> <!-- If you're unsure about any of these, don't hesitate to ask. We're here to help! --> - [x] I have made sure that the title is self-explanatory and the description concisely explains the PR. - [x] I have linked an issue or discussion. - [ ] I have created tests covering the changes. - [ ] I have updated the documentation accordingly. ### ⌛ Dependencies <!-- List all open pull requests that this PR logically depends on --> <!-- - #xyz: short description why this is a dependency - #abc: ... --> URL: #35210 Reported by: Kwankyu Lee Reviewer(s): Kwankyu Lee, Matthias Köppe
2 parents 0ae9698 + 8e03691 commit b0aaac8

File tree

1 file changed

+85
-128
lines changed

1 file changed

+85
-128
lines changed

src/sage/rings/polynomial/multi_polynomial_libsingular.pyx

+85-128
Original file line numberDiff line numberDiff line change
@@ -2090,20 +2090,19 @@ cdef class MPolynomial_libsingular(MPolynomial_libsingular_base):
20902090
res_parent = coercion_model.common_parent(parent._base, *x)
20912091
cdef poly *res # ownership will be transferred to us in the else block
20922092
try:
2093-
# Attempt evaluation via singular.
20942093
coerced_x = [parent.coerce(e) for e in x]
20952094
except TypeError:
20962095
# give up, evaluate functional
20972096
sage_res = parent.base_ring().zero()
2098-
for (m,c) in self.dict().iteritems():
2097+
for m, c in self.dict().iteritems():
20992098
sage_res += c * mul([x[i] ** m[i] for i in m.nonzero_positions()])
21002099
else:
21012100
singular_polynomial_call(&res, self._poly, _ring, coerced_x, MPolynomial_libsingular_get_element)
21022101

21032102
if res == NULL:
21042103
return res_parent(0)
21052104
if p_LmIsConstant(res, _ring):
2106-
sage_res = si2sa( p_GetCoeff(res, _ring), _ring, parent._base )
2105+
sage_res = si2sa(p_GetCoeff(res, _ring), _ring, parent._base)
21072106
p_Delete(&res, _ring) # sage_res contains copy
21082107
else:
21092108
sage_res = new_MP(parent, res) # pass on ownership of res to sage_res
@@ -3401,8 +3400,7 @@ cdef class MPolynomial_libsingular(MPolynomial_libsingular_base):
34013400
- ``fixed`` - (optional) dict with variable:value pairs
34023401
- ``**kw`` - names parameters
34033402
3404-
OUTPUT:
3405-
a new multivariate polynomial
3403+
OUTPUT: a new multivariate polynomial
34063404
34073405
EXAMPLES::
34083406
@@ -3462,7 +3460,7 @@ cdef class MPolynomial_libsingular(MPolynomial_libsingular_base):
34623460
34633461
sage: P = QQ['x,y']
34643462
sage: x = var('x')
3465-
sage: parent(P.zero() / x)
3463+
sage: parent(P.zero().subs(x=x))
34663464
Symbolic Ring
34673465
34683466
We are catching overflows::
@@ -3513,163 +3511,122 @@ cdef class MPolynomial_libsingular(MPolynomial_libsingular_base):
35133511
c*x^2*y + d*x^2 + (2*c)*x*y + (2*d)*x + c*y + d
35143512
35153513
"""
3516-
cdef int mi, i, need_map, try_symbolic
3514+
cdef int mi, i
3515+
cdef bint change_ring = False # indicates the need to change the parent ring
3516+
cdef bint need_map = False
35173517

35183518
cdef unsigned long degree = 0
35193519
cdef MPolynomialRing_libsingular parent = self._parent
35203520
cdef ring *_ring = parent._ring
35213521

3522-
if(_ring != currRing): rChangeCurrRing(_ring)
3522+
if (_ring != currRing):
3523+
rChangeCurrRing(_ring)
35233524

35243525
cdef poly *_p = p_Copy(self._poly, _ring)
35253526
cdef poly *_f
35263527

3527-
cdef ideal *to_id = idInit(_ring.N,1)
3528+
cdef ideal *to_id = idInit(_ring.N, 1)
35283529
cdef ideal *from_id
35293530
cdef ideal *res_id
3530-
need_map = 0
3531-
try_symbolic = 0
3531+
cdef dict gd
35323532

3533-
if _p == NULL:
3534-
# the polynomial is 0. There is nothing to do except to change the
3535-
# ring
3536-
try_symbolic = 1
3537-
3538-
if not try_symbolic and fixed is not None:
3539-
for m,v in fixed.items():
3540-
if isinstance(m, (int, Integer)):
3541-
mi = m+1
3542-
elif isinstance(m,MPolynomial_libsingular) and m.parent() is parent:
3543-
for i from 0 < i <= _ring.N:
3544-
if p_GetExp((<MPolynomial_libsingular>m)._poly, i, _ring) != 0:
3545-
mi = i
3546-
break
3547-
if i > _ring.N:
3548-
id_Delete(&to_id, _ring)
3549-
p_Delete(&_p, _ring)
3550-
raise TypeError("key does not match")
3551-
else:
3552-
id_Delete(&to_id, _ring)
3553-
p_Delete(&_p, _ring)
3554-
raise TypeError("keys do not match self's parent")
3555-
try:
3556-
v = parent.coerce(v)
3557-
except TypeError:
3558-
try_symbolic = 1
3559-
break
3560-
_f = (<MPolynomial_libsingular>v)._poly
3561-
if p_IsConstant(_f, _ring):
3562-
singular_polynomial_subst(&_p, mi-1, _f, _ring)
3563-
else:
3564-
need_map = 1
3565-
degree = <unsigned long>p_GetExp(_p, mi, _ring) * <unsigned long>p_GetMaxExp(_f, _ring)
3566-
if degree > _ring.bitmask:
3533+
cdef list g = list(parent.gens())
3534+
cdef list items = []
3535+
3536+
if not change_ring:
3537+
if fixed is not None:
3538+
for m, v in fixed.items():
3539+
if isinstance(m, (int, Integer)):
3540+
mi = m + 1
3541+
elif isinstance(m, MPolynomial_libsingular) and m.parent() is parent:
3542+
for i from 0 < i <= _ring.N:
3543+
if p_GetExp((<MPolynomial_libsingular> m)._poly, i, _ring) != 0:
3544+
mi = i
3545+
break
3546+
if i > _ring.N:
3547+
id_Delete(&to_id, _ring)
3548+
p_Delete(&_p, _ring)
3549+
raise TypeError("key does not match")
3550+
else:
35673551
id_Delete(&to_id, _ring)
35683552
p_Delete(&_p, _ring)
3569-
raise OverflowError("exponent overflow (%d)"%(degree))
3570-
to_id.m[mi-1] = p_Copy(_f, _ring)
3571-
3553+
raise TypeError("keys do not match self's parent")
3554+
3555+
items.append((g[mi - 1], v))
3556+
if kw:
3557+
gd = parent.gens_dict(copy=False)
3558+
for m, v in kw.items():
3559+
items.append((gd[m], v))
3560+
for m, v in items:
35723561
if _p == NULL:
3573-
# polynomial becomes 0 after some substitution
3574-
try_symbolic = 1
3562+
change_ring = True
35753563
break
3576-
3577-
cdef dict gd
3578-
3579-
if not try_symbolic:
3580-
gd = parent.gens_dict(copy=False)
3581-
for m,v in kw.iteritems():
3582-
m = gd[m]
3583-
for i from 0 < i <= _ring.N:
3584-
if p_GetExp((<MPolynomial_libsingular>m)._poly, i, _ring) != 0:
3585-
mi = i
3586-
break
3587-
if i > _ring.N:
3588-
id_Delete(&to_id, _ring)
3589-
p_Delete(&_p, _ring)
3590-
raise TypeError("key does not match")
3564+
i = g.index(m)
35913565
try:
35923566
v = parent.coerce(v)
3593-
except TypeError:
3594-
try_symbolic = 1
3595-
break
3596-
_f = (<MPolynomial_libsingular>v)._poly
3567+
except TypeError: # give up, evaluate symbolically
3568+
id_Delete(&to_id, _ring)
3569+
p_Delete(&_p, _ring)
3570+
3571+
gg = list(parent.gens())
3572+
for m, v in items:
3573+
gg[g.index(m)] = v
3574+
y = parent.base_ring().zero()
3575+
for (m,c) in self.dict().items():
3576+
y += c*mul([ gg[i]**m[i] for i in m.nonzero_positions()])
3577+
return y
3578+
_f = (<MPolynomial_libsingular> v)._poly
35973579
if p_IsConstant(_f, _ring):
3598-
singular_polynomial_subst(&_p, mi-1, _f, _ring)
3580+
singular_polynomial_subst(&_p, i, _f, _ring)
35993581
else:
3600-
if to_id.m[mi-1] != NULL:
3601-
p_Delete(&to_id.m[mi-1],_ring)
3602-
to_id.m[mi-1] = p_Copy(_f, _ring)
3603-
degree = <unsigned long>p_GetExp(_p, mi, _ring) * <unsigned long>p_GetMaxExp(_f, _ring)
3604-
if degree > _ring.bitmask:
3582+
need_map = True
3583+
degree = (<unsigned long> p_GetExp(_p, i + 1, _ring)) * (<unsigned long> p_GetMaxExp(_f, _ring))
3584+
if degree > _ring.bitmask:
36053585
id_Delete(&to_id, _ring)
36063586
p_Delete(&_p, _ring)
3607-
raise OverflowError("exponent overflow (%d)"%(degree))
3608-
need_map = 1
3609-
3610-
if _p == NULL:
3611-
# the polynomial is 0
3612-
try_symbolic = 1
3613-
break
3587+
raise OverflowError("exponent overflow (%d)" % (degree,))
3588+
to_id.m[i] = p_Copy(_f, _ring)
36143589

3615-
if need_map:
3616-
for mi from 0 <= mi < _ring.N:
3617-
if to_id.m[mi] == NULL:
3618-
to_id.m[mi] = p_ISet(1,_ring)
3619-
p_SetExp(to_id.m[mi], mi+1, 1, _ring)
3620-
p_Setm(to_id.m[mi], _ring)
3590+
if not change_ring and need_map:
3591+
for mi from 0 <= mi < _ring.N:
3592+
if to_id.m[mi] == NULL:
3593+
to_id.m[mi] = p_ISet(1,_ring)
3594+
p_SetExp(to_id.m[mi], mi + 1, 1, _ring)
3595+
p_Setm(to_id.m[mi], _ring)
36213596

3622-
from_id=idInit(1,1)
3623-
from_id.m[0] = _p
3597+
from_id = idInit(1, 1)
3598+
from_id.m[0] = _p
36243599

3625-
rChangeCurrRing(_ring)
3626-
res_id = fast_map_common_subexp(from_id, _ring, to_id, _ring)
3627-
_p = res_id.m[0]
3600+
rChangeCurrRing(_ring)
3601+
res_id = fast_map_common_subexp(from_id, _ring, to_id, _ring)
3602+
_p = res_id.m[0]
36283603

3629-
from_id.m[0] = NULL
3630-
res_id.m[0] = NULL
3604+
from_id.m[0] = NULL
3605+
res_id.m[0] = NULL
36313606

3632-
id_Delete(&from_id, _ring)
3633-
id_Delete(&res_id, _ring)
3607+
id_Delete(&from_id, _ring)
3608+
id_Delete(&res_id, _ring)
36343609

36353610
id_Delete(&to_id, _ring)
36363611

3637-
if not try_symbolic:
3612+
if not change_ring:
36383613
return new_MP(parent,_p)
36393614

3640-
# now as everything else failed, try to do it symbolically with call
3641-
3642-
cdef list g = list(parent.gens())
3643-
3644-
if fixed is not None:
3645-
for m,v in fixed.items():
3646-
if isinstance(m, (int, Integer)):
3647-
mi = m+1
3648-
elif isinstance(m, MPolynomial_libsingular) and m.parent() is parent:
3649-
for i from 0 < i <= _ring.N:
3650-
if p_GetExp((<MPolynomial_libsingular>m)._poly, i, _ring) != 0:
3651-
mi = i
3652-
break
3653-
if i > _ring.N:
3654-
raise TypeError("key does not match")
3655-
else:
3656-
raise TypeError("keys do not match self's parent")
3657-
3658-
g[mi-1] = v
3615+
# finally change the parent of the result
36593616

3660-
gd = parent.gens_dict(copy=False)
3661-
for m,v in kw.iteritems():
3662-
m = gd[m]
3663-
for i from 0 < i <= _ring.N:
3664-
if p_GetExp((<MPolynomial_libsingular>m)._poly, i, _ring) != 0:
3665-
mi = i
3666-
break
3667-
if i > _ring.N:
3668-
raise TypeError("key does not match")
3617+
res_parent = coercion_model.common_parent(parent._base, *[v for _, v in items])
36693618

3670-
g[mi-1] = v
3619+
if _p == NULL:
3620+
return res_parent(0)
36713621

3672-
return self(*g)
3622+
if p_LmIsConstant(_p, _ring):
3623+
res = si2sa(p_GetCoeff(_p, _ring), _ring, parent._base)
3624+
p_Delete(&_p, _ring) # safe to delete; res contains copy
3625+
else:
3626+
res = new_MP(parent, _p)
3627+
if parent(res) is not res_parent:
3628+
res = res_parent(res)
3629+
return res
36733630

36743631
def monomials(self):
36753632
"""

0 commit comments

Comments
 (0)