From 3bbf8aac4fe4ddd972c023631222587db161fab6 Mon Sep 17 00:00:00 2001 From: Vincent Neiger Date: Tue, 30 Jan 2024 23:12:47 +0100 Subject: [PATCH 1/5] fixing the pex case, but in fact others don't return the correct error for negative degree input --- .../rings/polynomial/polynomial_zz_pex.pyx | 20 ++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/src/sage/rings/polynomial/polynomial_zz_pex.pyx b/src/sage/rings/polynomial/polynomial_zz_pex.pyx index 60968b412da..10f608e7c63 100644 --- a/src/sage/rings/polynomial/polynomial_zz_pex.pyx +++ b/src/sage/rings/polynomial/polynomial_zz_pex.pyx @@ -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() @@ -533,12 +538,9 @@ 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: - raise ValueError("degree argument must be a non-negative integer, got %s" % (degree)) - try: - d = degree - except ValueError: - 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)) ZZ_pEX_reverse_hi(r.x, ( self).x, d) else: ZZ_pEX_reverse(r.x, ( self).x) From fedfcdc7cd98effcc20232b466b84169ec7c5256 Mon Sep 17 00:00:00 2001 From: Vincent Neiger Date: Tue, 30 Jan 2024 23:20:59 +0100 Subject: [PATCH 2/5] fix in other similar places --- src/sage/rings/polynomial/polynomial_element.pyx | 9 ++++++--- .../rings/polynomial/polynomial_integer_dense_flint.pyx | 9 ++++++--- src/sage/rings/polynomial/polynomial_zmod_flint.pyx | 9 ++++++--- src/sage/rings/polynomial/polynomial_zz_pex.pyx | 9 ++++++--- 4 files changed, 24 insertions(+), 12 deletions(-) diff --git a/src/sage/rings/polynomial/polynomial_element.pyx b/src/sage/rings/polynomial/polynomial_element.pyx index 4c9cf2f7e0d..42a0b34b5fc 100644 --- a/src/sage/rings/polynomial/polynomial_element.pyx +++ b/src/sage/rings/polynomial/polynomial_element.pyx @@ -7667,9 +7667,12 @@ cdef class Polynomial(CommutativePolynomial): cdef unsigned long d if degree is not None: - d = degree - if d != degree: - raise ValueError("degree argument must be a non-negative integer, got %s"%(degree)) + if degree <= 0: + raise ValueError("degree argument must be a non-negative integer, got %s" % (degree)) + try: + d = degree + except ValueError: + raise ValueError("degree argument must be a non-negative inte ger, got %s" % (degree)) if len(v) < degree+1: v.reverse() v = [self.base_ring().zero()]*(degree+1-len(v)) + v diff --git a/src/sage/rings/polynomial/polynomial_integer_dense_flint.pyx b/src/sage/rings/polynomial/polynomial_integer_dense_flint.pyx index fc0e2431b9c..4b0e0ac38ee 100644 --- a/src/sage/rings/polynomial/polynomial_integer_dense_flint.pyx +++ b/src/sage/rings/polynomial/polynomial_integer_dense_flint.pyx @@ -1827,9 +1827,12 @@ cdef class Polynomial_integer_dense_flint(Polynomial): cdef Polynomial_integer_dense_flint res = self._new() cdef unsigned long d if degree is not None: - d = degree - if d != degree: - raise ValueError("degree argument must be a non-negative integer, got %s" % degree) + if degree <= 0: + raise ValueError("degree argument must be a non-negative integer, got %s" % (degree)) + try: + d = degree + except ValueError: + raise ValueError("degree argument must be a non-negative inte ger, got %s" % (degree)) # FLINT expects length fmpz_poly_reverse(res._poly, self._poly, d+1) else: diff --git a/src/sage/rings/polynomial/polynomial_zmod_flint.pyx b/src/sage/rings/polynomial/polynomial_zmod_flint.pyx index 3a66198d568..ba3c3c1f3fd 100644 --- a/src/sage/rings/polynomial/polynomial_zmod_flint.pyx +++ b/src/sage/rings/polynomial/polynomial_zmod_flint.pyx @@ -815,9 +815,12 @@ cdef class Polynomial_zmod_flint(Polynomial_template): cdef Polynomial_zmod_flint res = self._new() cdef unsigned long d if degree is not None: - d = degree - if d != degree: - raise ValueError("degree argument must be a non-negative integer, got %s"%(degree)) + if degree <= 0: + raise ValueError("degree argument must be a non-negative integer, got %s" % (degree)) + try: + d = degree + except ValueError: + raise ValueError("degree argument must be a non-negative inte ger, 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)) diff --git a/src/sage/rings/polynomial/polynomial_zz_pex.pyx b/src/sage/rings/polynomial/polynomial_zz_pex.pyx index 10f608e7c63..d06e06a052b 100644 --- a/src/sage/rings/polynomial/polynomial_zz_pex.pyx +++ b/src/sage/rings/polynomial/polynomial_zz_pex.pyx @@ -538,9 +538,12 @@ 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: - d = degree - if d != degree: - raise ValueError("degree argument must be a non-negative integer, got %s"%(degree)) + if degree <= 0: + raise ValueError("degree argument must be a non-negative integer, got %s" % (degree)) + try: + d = degree + except ValueError: + raise ValueError("degree argument must be a non-negative inte ger, got %s" % (degree)) ZZ_pEX_reverse_hi(r.x, ( self).x, d) else: ZZ_pEX_reverse(r.x, ( self).x) From 29f855570159da19999815427206ccbcce46eb6f Mon Sep 17 00:00:00 2001 From: Vincent Neiger Date: Tue, 30 Jan 2024 23:55:41 +0100 Subject: [PATCH 3/5] fix d <= 0 -> d < 0 --- src/sage/rings/polynomial/polynomial_element.pyx | 9 ++++----- .../rings/polynomial/polynomial_integer_dense_flint.pyx | 9 ++++----- src/sage/rings/polynomial/polynomial_zmod_flint.pyx | 9 ++++----- src/sage/rings/polynomial/polynomial_zz_pex.pyx | 9 ++++----- 4 files changed, 16 insertions(+), 20 deletions(-) diff --git a/src/sage/rings/polynomial/polynomial_element.pyx b/src/sage/rings/polynomial/polynomial_element.pyx index 42a0b34b5fc..d28d5d1e543 100644 --- a/src/sage/rings/polynomial/polynomial_element.pyx +++ b/src/sage/rings/polynomial/polynomial_element.pyx @@ -7667,12 +7667,11 @@ cdef class Polynomial(CommutativePolynomial): 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)) + d = degree + if d != degree: raise ValueError("degree argument must be a non-negative integer, got %s" % (degree)) - try: - d = degree - except ValueError: - raise ValueError("degree argument must be a non-negative inte ger, got %s" % (degree)) if len(v) < degree+1: v.reverse() v = [self.base_ring().zero()]*(degree+1-len(v)) + v diff --git a/src/sage/rings/polynomial/polynomial_integer_dense_flint.pyx b/src/sage/rings/polynomial/polynomial_integer_dense_flint.pyx index 4b0e0ac38ee..8498f3dd760 100644 --- a/src/sage/rings/polynomial/polynomial_integer_dense_flint.pyx +++ b/src/sage/rings/polynomial/polynomial_integer_dense_flint.pyx @@ -1827,12 +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: + 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)) - try: - d = degree - except ValueError: - raise ValueError("degree argument must be a non-negative inte ger, got %s" % (degree)) # FLINT expects length fmpz_poly_reverse(res._poly, self._poly, d+1) else: diff --git a/src/sage/rings/polynomial/polynomial_zmod_flint.pyx b/src/sage/rings/polynomial/polynomial_zmod_flint.pyx index ba3c3c1f3fd..1df2497b4ad 100644 --- a/src/sage/rings/polynomial/polynomial_zmod_flint.pyx +++ b/src/sage/rings/polynomial/polynomial_zmod_flint.pyx @@ -815,12 +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: + 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)) - try: - d = degree - except ValueError: - raise ValueError("degree argument must be a non-negative inte ger, 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)) diff --git a/src/sage/rings/polynomial/polynomial_zz_pex.pyx b/src/sage/rings/polynomial/polynomial_zz_pex.pyx index d06e06a052b..295bc4fccbe 100644 --- a/src/sage/rings/polynomial/polynomial_zz_pex.pyx +++ b/src/sage/rings/polynomial/polynomial_zz_pex.pyx @@ -538,12 +538,11 @@ 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)) + d = degree + if d != degree: raise ValueError("degree argument must be a non-negative integer, got %s" % (degree)) - try: - d = degree - except ValueError: - raise ValueError("degree argument must be a non-negative inte ger, got %s" % (degree)) ZZ_pEX_reverse_hi(r.x, ( self).x, d) else: ZZ_pEX_reverse(r.x, ( self).x) From 9f15f694bfd4d42ebaa63cc5eebc79b66089c96b Mon Sep 17 00:00:00 2001 From: Vincent Neiger Date: Wed, 31 Jan 2024 09:24:53 +0100 Subject: [PATCH 4/5] fix error message in related reverse method for univariate polynomial matrices --- src/sage/matrix/matrix_polynomial_dense.pyx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/sage/matrix/matrix_polynomial_dense.pyx b/src/sage/matrix/matrix_polynomial_dense.pyx index 31a13cd83d6..79e9eae4b1b 100644 --- a/src/sage/matrix/matrix_polynomial_dense.pyx +++ b/src/sage/matrix/matrix_polynomial_dense.pyx @@ -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:: From f503b52f181bdb223766cee296a5fd59335e86fb Mon Sep 17 00:00:00 2001 From: Vincent Neiger Date: Wed, 31 Jan 2024 15:08:18 +0100 Subject: [PATCH 5/5] add test --- src/sage/matrix/matrix_polynomial_dense.pyx | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/src/sage/matrix/matrix_polynomial_dense.pyx b/src/sage/matrix/matrix_polynomial_dense.pyx index 79e9eae4b1b..996e061f485 100644 --- a/src/sage/matrix/matrix_polynomial_dense.pyx +++ b/src/sage/matrix/matrix_polynomial_dense.pyx @@ -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 @@ -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) 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) @@ -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)