sagemath / sage

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

Fix Singular polynomial comparison #39018

Open user202729 opened 4 days ago

user202729 commented 4 days ago

Fixes #35681

Basically, I think this change makes the comparison makes more sense (apart from that, it speeds up the comparison a bit because it only calls cfGreater when cfEqual returns False, and get rid of the cfSub):

    TESTS::

        sage: R.<x, y> = ZZ[]
        sage: x^2+x > x^2
        True
        sage: x^2 > x^2-x                       # NEW
        True
        sage: x^2 > x^2-x                       # OLD
        False
        sage: x^2+x > x^2-x
        True
        sage: x^2 > 0
        True
        sage: -x^2 > 0                          # NEW
        False
        sage: -x^2 > 0                          # OLD
        True
        sage: x^2 > -x^2
        True

    ::

        sage: R.<x, y> = GF(7)[]
        sage: x^2+x > x^2
        True
        sage: x^2 > x^2-x                       # NEW
        True
        sage: x^2 > x^2-x                       # OLD
        False
        sage: x^2+x > x^2-x                     # NEW
        False
        sage: x^2+x > x^2-x                     # OLD
        True
        sage: x^2 > 0
        True
        sage: -x^2 > 0                          # NEW
        False
        sage: -x^2 > 0                          # OLD
        True
        sage: x^2 > -x^2                        # NEW
        False
        sage: x^2 > -x^2                        # OLD
        True

    ::

        sage: R.<x, y> = Integers(8)[]
        sage: x^2+x > x^2
        True
        sage: x^2 > x^2-x                       # NEW
        True
        sage: x^2 > x^2-x                       # OLD
        False
        sage: x^2+x > x^2-x
        True
        sage: x^2 > 0
        True
        sage: -x^2 > 0                          # NEW
        False
        sage: -x^2 > 0                          # OLD
        True
        sage: x^2 > -x^2
        True

    ::

        sage: R.<x, y> = Integers(10)[]
        sage: x^2+x > x^2
        True
        sage: x^2 > x^2-x
        False
        sage: x^2+x > x^2-x                     # NEW
        False
        sage: x^2+x > x^2-x                     # OLD
        True
        sage: x^2 > 0
        True
        sage: -x^2 > 0
        True
        sage: x^2 > -x^2                        # NEW
        False
        sage: x^2 > -x^2                        # OLD
        True

    ::

        sage: R.<x, y> = ZZ[]
        sage: l = [i*x+j*x^2 for i in range(-5, 5) for j in range(-5, 5)]
        sage: l.sort()
        sage: for i in range(len(l)):
        ....:     for b in l[:i]:
        ....:         assert b < l[i], (b, l[i])

    ::

        sage: R.<x, y> = Integers(10)[]
        sage: l = [i*x+j*y for i in range(7) for j in range(7)]
        sage: l.sort()
        sage: for i in range(len(l)):           # NEW
        ....:     for b in l[:i]:
        ....:         assert b < l[i], (b, l[i])
        sage: for i in range(len(l)):           # OLD
        ....:     for b in l[:i]:
        ....:         assert b < l[i], (b, l[i])
        Traceback (most recent call last):
        ...
        AssertionError: (y, 2*y)

Besides, after this change, as long as cfGreater defines a total order, you get a total order on the polynomials. For example now Integers(10)[] has a total order.

For unordered fields however, cfGreater may not define a total order anyway (see nfGreater, nrnGreater, nr2mGreater functions in Singular source code). What do you think? (Update: in newer Singular, cfGreater now defines a total order. https://github.com/Singular/Singular/issues/1253#issuecomment-2498161839)

The comparison function was added all the way back in 2007 without explanation e22dc86738f1adbc99f9c097ed233ced8d5f50ad

:memo: Checklist

:hourglass: Dependencies

github-actions[bot] commented 4 days ago

Documentation preview for this PR (built with commit 47c6fcfc96ef1c0a9e4afbf4cdb2e3b5044ac401; changes) is ready! :tada: This preview will update shortly after each push to this PR.