Skip to content

Commit eef9eea

Browse files
author
Release Manager
committed
gh-35876: correct parent for square root of constant polynomial <!-- Please provide a concise, informative and self-explanatory title. --> <!-- Don't put issue numbers in the title. Put it in the Description below. --> <!-- For example, instead of "Fixes #12345", use "Add a new method to multiply two integers" --> ### 📚 Description <!-- Describe your changes here in detail. --> <!-- Why is this change required? What problem does it solve? --> <!-- If this PR resolves an open issue, please link to it here. For example "Fixes #12345". --> <!-- If your change requires a documentation PR, please link it appropriately. --> Fixes #35860. If `f` is a polynomial that is a perfect square, then `sqrt(f)` should also be a polynomial. However, this has not been the case for polynomials that happen to be constant: the code returned a constant in the base ring, not a constant polynomial. The PR fixes this (and adds a doctest for this bug). This change fixes the serious bug that was pointed out (and diagnosed) by @amithazi in [issue #35860](#35860). A single- variable Laurent polynomial is stored as a pair `(u, n)`, where `u` is an ordinary polynomial, and `n` is an integer that represents an offset (i.e., multiplication by a [usually negative] power of the variable). To add two Laurent polynomials, one of them needs to be shifted so they have the same offset: ``` f2 = right.__u << right.__n - left.__n ``` This code assumes that the parent of `right.__u` is a polynomial ring: if the parent of `right.__u` is `ZZ`, then the shift operator `<<` multiplies by a power of 2, instead of shifting the polynomial, so the result is nonsense. The PR also adds a doctest for this bug. ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. It should be `[x]` not `[x ]`. --> - [x] The title is concise, informative, and self-explanatory. - [x] The description explains in detail what this PR is about. - [x] I have linked a relevant issue or discussion. - [x] I have created tests covering the changes. ### ⌛ Dependencies <!-- List all open PRs that this PR logically depends on - #12345: short description why this is a dependency - #34567: ... --> <!-- If you're unsure about any of these, don't hesitate to ask. We're here to help! --> URL: #35876 Reported by: DaveWitteMorris Reviewer(s): Marc Mezzarobba
2 parents 6b170a4 + bdfdab9 commit eef9eea

File tree

2 files changed

+15
-1
lines changed

2 files changed

+15
-1
lines changed

src/sage/rings/laurent_series_ring_element.pyx

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -751,6 +751,14 @@ cdef class LaurentSeries(AlgebraElement):
751751
t^-3 + t^3 + O(t^9)
752752
753753
ALGORITHM: Shift the unit parts to align them, then add.
754+
755+
TESTS:
756+
757+
Verify that :trac:`35860` is fixed::
758+
759+
sage: R.<t> = LaurentPolynomialRing(ZZ)
760+
sage: sqrt(t^2) + t^-1
761+
t^-1 + t
754762
"""
755763
cdef LaurentSeries right = <LaurentSeries>right_m
756764
cdef long m

src/sage/rings/polynomial/polynomial_element.pyx

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1988,6 +1988,12 @@ cdef class Polynomial(CommutativePolynomial):
19881988
False
19891989
sage: R(0).is_square()
19901990
True
1991+
1992+
Make sure :trac:`35860` is fixed::
1993+
1994+
sage: S.<x> = PolynomialRing(ZZ)
1995+
sage: is_square(S(1), True)[1].parent()
1996+
Univariate Polynomial Ring in x over Integer Ring
19911997
"""
19921998
if self.is_zero():
19931999
return (True, self) if root else True
@@ -2000,7 +2006,7 @@ cdef class Polynomial(CommutativePolynomial):
20002006
u = self._parent.base_ring()(f.unit())
20012007

20022008
if all(a[1] % 2 == 0 for a in f) and u.is_square():
2003-
g = u.sqrt()
2009+
g = self._parent(u.sqrt())
20042010
for a in f:
20052011
g *= a[0] ** (a[1] // 2)
20062012
return (True, g) if root else True

0 commit comments

Comments
 (0)