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 quotients of univariate polynomial rings over ZZ #23621

Open
koffie opened this issue Aug 14, 2017 · 10 comments
Open

Fix quotients of univariate polynomial rings over ZZ #23621

koffie opened this issue Aug 14, 2017 · 10 comments

Comments

@koffie
Copy link

koffie commented Aug 14, 2017

The quotient of ZZ[x] by the ideal (x, 2)
works fine using a multivariate polynomial ring:

sage: R.<x> = PolynomialRing(ZZ, 1)
sage: I = R.ideal([x, 2])
sage: I
Ideal (x, 2) of Multivariate Polynomial Ring in x over Integer Ring
sage: S = R.quo(I)
sage: [[S(a) == S(b) for b in (0, 2, x)] for a in (0, 2, x)]
[[True, True, True], [True, True, True], [True, True, True]]

but it fails using a univariate polynomial ring,
returning mathematically wrong answers:

sage: R.<x> = ZZ[]
sage: I = R.ideal([x, 2])
sage: I
Ideal (x, 2) of Univariate Polynomial Ring in x over Integer Ring
sage: S = R.quo(I)
sage: 
sage: [[S(a) == S(b) for b in (0, 2, x)] for a in (0, 2, x)]
[[True, False, False], [False, True, False], [False, False, True]]

Expected:

[[True, True, True], [True, True, True], [True, True, True]]

CC: @slel

Component: commutative algebra

Keywords: ideal

Issue created by migration from https://trac.sagemath.org/ticket/23621

@koffie koffie added this to the sage-8.1 milestone Aug 14, 2017
@roed314
Copy link
Contributor

roed314 commented Aug 18, 2017

comment:1

The problem is that I is just a generic ideal and doesn't implement a reduce method.

@roed314
Copy link
Contributor

roed314 commented Aug 18, 2017

comment:2

To solve this, I think one needs to implement a new class for ideals in ZZ['x'] and set _ideal_class_ appropriately on R. Of course, one can argue that the default behavior of the reduce method on a generic ideal should be to raise an error rather than just return the input unchanged.

@koffie
Copy link
Author

koffie commented Aug 31, 2017

comment:3

Yeah I totally agree that it should raise an error, because this implementation does not satisfy the assumption on reduce in other parts of the code. For example this is an excerpt from sage/rings/quotient_ring.py.

The only requirement is that the two-sided ideal `I`
provides a ``reduce`` method so that ``I.reduce(x)`` is the normal
form of an element `x` with respect to `I` (i.e., we have
``I.reduce(x) == I.reduce(y)`` if `x-y \in I`, and
``x - I.reduce(x) in I``). H

And I think that this is a logic requirement to put on the reduce method.

@koffie
Copy link
Author

koffie commented Aug 31, 2017

comment:4

Ok there are quite a few doctest failures. If I just make it raise an error. Ironically the first failure is

sage: sage: MS = MatrixSpace(GF(5),2,2)
....: sage: I = MS*[MS.0*MS.1,MS.2+MS.3]*MS
....: sage: Q = MS.quo(I)
....: sage: Q.0*Q.1   # indirect doctest
....: 
---------------------------------------------------------------------------
NotImplementedError                       Traceback (most recent call last)
...
NotImplementedError: reduce not implemented for Twosided Ideal 
(
  [0 1]
  [0 0],

  [0 0]
  [1 1]
)
 of Full MatrixSpace of 2 by 2 dense matrices over Finite Field of size 5

which was added to test that #11068 is fixed, the ticket where the above text about "The only requirement is that the two-sided ideal I..." comes from.

@koffie
Copy link
Author

koffie commented Aug 31, 2017

comment:5

The second failure points at #13999 of which this ticket basically is a dupe.

@koffie
Copy link
Author

koffie commented Aug 31, 2017

comment:6

All failures will probably be fixed if these three tests pass

sage: MS = MatrixSpace(GF(5),2,2)
sage: I = MS*[MS.0*MS.1,MS.2+MS.3]*MS
sage: Q = MS.quo(I)
sage: Q.0*Q.1   # indirect doctest
[0 1]
[0 0]
sage: R.<x> = PolynomialRing(ZZ)
sage: I = R.ideal([4 + 3*x + x^2, 1 + x^2])
sage: S = R.quotient_ring(I);
sage: TestSuite(S).run(skip=['_test_nonzero_equal', '_test_elements', '_test_zero'])
sage: S = SteenrodAlgebra(2)
sage: I = S*[S.0+S.1]*S
sage: Q = S.quo(I)
sage: Q.0
Sq(1)

I consider all three of them bugs, so this strengthens my believe that it is better to raise a NotImplementedError.

@koffie
Copy link
Author

koffie commented Sep 2, 2017

comment:7

I think that all the matrix space examples will not give any interesting doctest, since matrix rings over fields are simple and hence there are no two sided ideals. Although this means that the reduce function is very easy to implement! I don't know enough about Steenrod algebra's in order to create a meaningful reduce method.

@koffie

This comment has been minimized.

@slel

This comment has been minimized.

@slel
Copy link
Member

slel commented Dec 7, 2021

Changed keywords from none to ideal

@slel slel modified the milestones: sage-8.1, sage-9.5 Dec 7, 2021
@slel slel changed the title Quotients of univariate polynomial rings over ZZ return mathematical incorrect answers Fix quotients of univariate polynomial rings over ZZ Dec 7, 2021
@mkoeppe mkoeppe modified the milestones: sage-9.5, sage-9.6 Jan 10, 2022
@mkoeppe mkoeppe modified the milestones: sage-9.6, sage-9.7 May 3, 2022
@mkoeppe mkoeppe modified the milestones: sage-9.7, sage-9.8 Sep 19, 2022
@mkoeppe mkoeppe removed this from the sage-9.8 milestone Jan 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants