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

Fix kernel basis computation for polynomial matrix in corner case #37208

Merged
merged 5 commits into from
Feb 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 12 additions & 5 deletions src/sage/matrix/matrix_polynomial_dense.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -675,7 +675,7 @@ cdef class Matrix_polynomial_dense(Matrix_generic_dense):
sage: M.reverse([2,3,-1])
Traceback (most recent call last):
...
OverflowError: can't convert negative value to unsigned long
ValueError: degree argument must be a non-negative integer, got -1

.. SEEALSO::

Expand Down Expand Up @@ -3980,6 +3980,13 @@ cdef class Matrix_polynomial_dense(Matrix_generic_dense):
sage: Matrix(pR, 3, 2, [[x,0],[1,0],[x+1,0]]).minimal_kernel_basis()
[6 x 0]
[6 6 1]

TESTS:

We check that PR #37208 is fixed::

sage: Matrix(pR, 2, 0).minimal_kernel_basis().is_sparse()
False
"""
from sage.matrix.constructor import matrix

Expand All @@ -4001,11 +4008,11 @@ cdef class Matrix_polynomial_dense(Matrix_generic_dense):
return matrix(self.base_ring(), 0, m)

if n == 0: # early exit: kernel is identity
return matrix.identity(self.base_ring(), m, m)
return matrix.identity(self.base_ring(), m)
vneiger marked this conversation as resolved.
Show resolved Hide resolved

d = self.degree() # well defined since m > 0 and n > 0
if d == -1: # matrix is zero: kernel is identity
return matrix.identity(self.base_ring(), m, m)
return matrix.identity(self.base_ring(), m)

# degree bounds on the kernel basis
degree_bound = min(m,n)*d+max(shifts)
Expand Down Expand Up @@ -4040,11 +4047,11 @@ cdef class Matrix_polynomial_dense(Matrix_generic_dense):
return matrix(self.base_ring(), n, 0)

if m == 0: # early exit: kernel is identity
return matrix.identity(self.base_ring(), n, n)
return matrix.identity(self.base_ring(), n)

d = self.degree() # well defined since m > 0 and n > 0
if d == -1: # matrix is zero
return matrix.identity(self.base_ring(), n, n)
return matrix.identity(self.base_ring(), n)

# degree bounds on the kernel basis
degree_bound = min(m,n)*d+max(shifts)
Expand Down
4 changes: 3 additions & 1 deletion src/sage/rings/polynomial/polynomial_element.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -7667,9 +7667,11 @@ cdef class Polynomial(CommutativePolynomial):

cdef unsigned long d
if degree is not None:
if degree < 0:
raise ValueError("degree argument must be a non-negative integer, got %s" % (degree))
d = degree
if d != degree:
raise ValueError("degree argument must be a non-negative integer, got %s"%(degree))
raise ValueError("degree argument must be a non-negative integer, got %s" % (degree))
if len(v) < degree+1:
v.reverse()
v = [self.base_ring().zero()]*(degree+1-len(v)) + v
Expand Down
4 changes: 3 additions & 1 deletion src/sage/rings/polynomial/polynomial_integer_dense_flint.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -1827,9 +1827,11 @@ cdef class Polynomial_integer_dense_flint(Polynomial):
cdef Polynomial_integer_dense_flint res = self._new()
cdef unsigned long d
if degree is not None:
if degree < 0:
raise ValueError("degree argument must be a non-negative integer, got %s" % (degree))
d = degree
if d != degree:
raise ValueError("degree argument must be a non-negative integer, got %s" % degree)
raise ValueError("degree argument must be a non-negative integer, got %s" % (degree))
# FLINT expects length
fmpz_poly_reverse(res._poly, self._poly, d+1)
else:
Expand Down
4 changes: 3 additions & 1 deletion src/sage/rings/polynomial/polynomial_zmod_flint.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -815,9 +815,11 @@ cdef class Polynomial_zmod_flint(Polynomial_template):
cdef Polynomial_zmod_flint res = self._new()
cdef unsigned long d
if degree is not None:
if degree < 0:
raise ValueError("degree argument must be a non-negative integer, got %s" % (degree))
d = degree
if d != degree:
raise ValueError("degree argument must be a non-negative integer, got %s"%(degree))
raise ValueError("degree argument must be a non-negative integer, got %s" % (degree))
nmod_poly_reverse(&res.x, &self.x, d+1) # FLINT expects length
else:
nmod_poly_reverse(&res.x, &self.x, nmod_poly_length(&self.x))
Expand Down
18 changes: 11 additions & 7 deletions src/sage/rings/polynomial/polynomial_zz_pex.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -513,13 +513,18 @@ cdef class Polynomial_ZZ_pEX(Polynomial_template):
sage: f.reverse(degree=200)
2*x^200 + 3*x^199 + 5*x^198 + 7*x^197 + 11*x^196 + 13*x^195 + 17*x^194 + 19*x^193
sage: f.reverse(degree=0)
Traceback (most recent call last):
...
ValueError: degree argument must be a non-negative integer, got 0
2
sage: f.reverse(degree=-5)
Traceback (most recent call last):
...
ValueError: degree argument must be a non-negative integer, got -5

Check that this implementation is compatible with the generic one::

sage: p = R([0,1,0,2])
sage: all(p.reverse(d) == Polynomial.reverse(p, d)
....: for d in [None, 0, 1, 2, 3, 4])
True
"""
self._parent._modulus.restore()

Expand All @@ -533,11 +538,10 @@ cdef class Polynomial_ZZ_pEX(Polynomial_template):
# When a degree has been supplied, ensure it is a valid input
cdef unsigned long d
if degree is not None:
if degree <= 0:
if degree < 0:
raise ValueError("degree argument must be a non-negative integer, got %s" % (degree))
try:
d = degree
except ValueError:
d = degree
if d != degree:
raise ValueError("degree argument must be a non-negative integer, got %s" % (degree))
ZZ_pEX_reverse_hi(r.x, (<Polynomial_ZZ_pEX> self).x, d)
else:
Expand Down
Loading