Skip to content

Commit

Permalink
gh-36127: Bandaid for polynomial evaluation
Browse files Browse the repository at this point in the history
    
There was a problem when evaluating polynomials: the following was
failing
```
sage: x,y=polygens(QQ,'x,y')
sage: t=PolynomialRing(x.parent(),'t').gen()
sage: F=x*y*t
sage: F(x=1)
y*t
```
I propose a fix that hopefully does not break anything else.
We look at a non-zero coefficient to try to guess the correct new base
ring, instead of looking at the constant term.

This does not fix the following inconsistent behaviour:

```
sage: x.parent().zero()(x=1)
0
sage: parent(_)
Rational Field
sage: x.parent().one()(x=1)
1
sage: parent(_)
Multivariate Polynomial Ring in x, y over Rational Field

```

### 📝 Checklist

- [x] The title is concise, informative, and self-explanatory.
- [x] The description explains in detail what this PR is about.
- [ ] I have linked a relevant issue or discussion.
- [x] I have created tests covering the changes.
- [ ] I have updated the documentation accordingly.
    
URL: #36127
Reported by: Frédéric Chapoton
Reviewer(s): Frédéric Chapoton, Marc Mezzarobba
  • Loading branch information
Release Manager committed Aug 31, 2023
2 parents 52c7956 + 956aaf0 commit 33c99ab
Showing 1 changed file with 25 additions and 9 deletions.
34 changes: 25 additions & 9 deletions src/sage/rings/polynomial/polynomial_element.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -605,6 +605,14 @@ cdef class Polynomial(CommutativePolynomial):
TESTS:
One test for a simple evaluation::
sage: x, y = polygens(ZZ, 'x,y')
sage: t = polygen(x.parent(), 't')
sage: F = x*y*t
sage: F(y=1)
x*t
The following shows that :trac:`2360` is indeed fixed. ::
sage: R.<x,y> = ZZ[]
Expand Down Expand Up @@ -783,14 +791,20 @@ cdef class Polynomial(CommutativePolynomial):
- Francis Clarke (2012-08-26): fix keyword substitution in the
leading coefficient.
"""
cdef long i, j
cdef long i, j, d, deg
cdef Polynomial pol = self
cdef long d
cdef ETuple etup
cdef list cs
cdef dict coeff_sparse, coeff_dict

cst = self._parent._base.zero() if self.degree() < 0 else self.get_unsafe(0)
deg = self.degree()
if deg < 0:
top = self._parent._base.one()
cst = self._parent._base.zero()
else:
top = self.get_unsafe(deg)
cst = self.get_unsafe(0)

a = args[0] if len(args) == 1 else None
if kwds or not (isinstance(a, Element) or PyNumber_Check(a)):
# slow path
Expand All @@ -816,18 +830,22 @@ cdef class Polynomial(CommutativePolynomial):
try:
# Note that we may be calling a different implementation that
# is more permissive about its arguments than we are.
cst = cst(*args, **kwds)
eval_coeffs = True
top = top(*args, **kwds)
except TypeError:
if args: # bwd compat: nonsense *keyword* arguments are okay
raise TypeError("Wrong number of arguments")
else:
eval_coeffs = True

# Evaluate the coefficients, then fall through to evaluate the
# resulting univariate polynomial

if eval_coeffs:
new_base = parent(top)
# tentative common parent of the evaluated coefficients
pol = pol.map_coefficients(lambda c: c(*args, **kwds),
new_base_ring=parent(cst))
new_base_ring=new_base)
cst = cst(*args, **kwds)

R = parent(a)

Expand All @@ -840,8 +858,6 @@ cdef class Polynomial(CommutativePolynomial):
if isinstance(a, Polynomial) and a.base_ring() is pol._parent._base:
if (<Polynomial> a).is_gen():
return R(pol)
if (<Polynomial> a).is_zero():
return R(cst)
d = (<Polynomial> a).degree()
if d < 0: # f(0)
return R(cst)
Expand Down Expand Up @@ -10210,7 +10226,7 @@ cdef class Polynomial(CommutativePolynomial):
R = R.change_ring(new_base_ring)
elif isinstance(f, Map):
R = R.change_ring(f.codomain())
return R({k: f(v) for (k,v) in self.dict().items()})
return R({k: f(v) for k, v in self.dict().items()})

def is_cyclotomic(self, certificate=False, algorithm="pari"):
r"""
Expand Down

0 comments on commit 33c99ab

Please sign in to comment.