sagemath / sage

Main repository of SageMath
https://www.sagemath.org
Other
1.35k stars 461 forks source link

square multivariate polynomials are incorrectly detected for some term orders #38010

Open maxale opened 4 months ago

maxale commented 4 months ago

Steps To Reproduce

The following code prints False, which is clearly wrong. At the same time, removing order=f'degrevlex(1),degrevlex(2)' from the ring K definition makes things work as expected.

K = PolynomialRing(QQ, 3, 'x,u,v', order=f'degrevlex(1),degrevlex(2)')
x = K('x')
print( (x^2).is_square() )

Expected Behavior

The property of being a square for a polynomial does not depend on the term order. Sage should test it equally well for whatever term order in the underlying polynomial ring.

Actual Behavior

Currently squareness of polynomials is incorrectly detected in some term orders as illustrated by the given code.

Additional Information

No response

Environment

- **OS**: Ubuntu 22.04.4 LTS
- **Sage Version**: 10.3

Checklist

DaveWitteMorris commented 4 months ago

Thanks for the bug report. Here is the underlying problem:

sage: K = PolynomialRing(QQ, 3, 'x,u,v', order=f'degrevlex(1),degrevlex(2)')
sage: x = K('x')
sage: Qx = PolynomialRing(QQ, "x")
sage: Qx(x^2)
---------------------------------------------------------------------------
KeyError                                  Traceback (most recent call last)
File ~/development/sage/src/sage/structure/parent.pyx:895, in sage.structure.parent.Parent.__call__()
     <snip>
File ~/development/sage/src/sage/rings/polynomial/multi_polynomial_ring_base.pyx:333, in sage.rings.polynomial.multi_polynomial_ring_base.MPolynomialRing_base.remove_var()
    331                               order=self.term_order())
    332     except ValueError:
--> 333         raise ValueError("impossible to use the original term order (most likely because it was a block order). Please specify the term order for the subring")
    334 else:
    335     return PolynomialRing(self.base_ring(), vars, order=order)

ValueError: impossible to use the original term order (most likely because it was a block order). Please specify the term order for the subring

The failed conversion raises an unexpected ValueError, and is_square erroneously takes this to mean that there is no square root.

The problematic conversion pU = U(p) (where U = PolynomialRing(S.base_ring(), str(V[0]))) is invoked in line 2210 of src/sage/rings/polynomial/multi_polynomial.pyx. This needs to be replaced with something that succeeds.

maxale commented 1 month ago

The failed conversion raises an unexpected ValueError, and is_square erroneously takes this to mean that there is no square root.

Isn't that behavior of is_square unsafe? Putting aside a fix for the troublesome conversion, I think is_square should be fixed by making more careful decisions about the errors it catches.