Improvements and corrections to QuotientRingElement

[sagetrac-Bouillaguet]

The class QuotientRingElement, which implements the operations an element of the quotient R/I of a ring R by an ideal I, suffers from several problems and limitations. Most of these were uncovered while working on #13670 and #13675.

• While QuotientRingElement aims to be generic, it contains code dedicated to the case where R is a multivariate polynomial ring. In particular, the implementation of division first checks if R supports the computation of Groebner bases. This is not the proper way to go ; a special class should be created, following the approach taken for univariate polynomial quotient rings (PolynomialQuotientRing, PolynomialQuotientRingElement). We should create MPolynomialQuotientRing and MPolynomialQuotientRingElement, and host multivariate-polynomial-specific code here.

• The is_unit() function does almost nothing (it checks if its argument is a unit in R). In the case of (multivariate) polynomial rings, an actual test can be implemented.

• The class lacks an is_regular() methods (that detects zero divisors).

• In the case of (multivariate) polynomial rings, is_regular() can be implemented.

• The interest of is_regular() is that division by x should only be allowed if x is regular.

• The present implementation of division has problems. It contains multivariate-polynomial-specific code, which is bad. Furthermore, it allows division by zero-divisors, even tough the result is not defined :

  sage: R.<x,y> = QQ[]
  sage: S = R.quotient_ring(R.ideal(x^2, y))
  sage: S(2*x)/S(x)
  S(2)
  sage: S(2) * S(x) == S(2*x)  # indeed, division works correctly....
  True
  sage: S(2+x) * S(x) == S(2*x) # but several "quotients" are possible, because ``S(x)`` is a zero-divisor
  

  In contrast, univariate polynomial rings behave more rigorously:

  sage: P.<x> = QQ[]
  sage: S = P.quotient_ring(x^2)
  sage: S(2*x)/S(x)
  ZeroDivisionError: element xbar of quotient polynomial ring not invertible
  

• This raises the question of how we want division to proceed:

  • ignore the problem? (current status, no overhead)
  • test for regularity before dividing (mathematically better, may be much slower)

• Clarifying all this would then open the possibility to have, for example, special code to deal with ideals given by a regular chain instead of a Groebner basis

---

Apply

1. attachment: trac_13697.patch

Depends on #13670
Depends on #13671
Depends on #13675
Depends on #13714

Component: commutative algebra

Keywords: sd86.5

Author: Charles Bouillaguet

Branch/Commit: u/saraedum/ticket/13697 @ 6276d0b

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

[sagetrac-Bouillaguet]

Changed dependencies from #13670,#13671,#13675 to #13670,#13671,#13675,#13714

[sagetrac-Bouillaguet]

comment:2

Attachment: improve_quotient_ring.patch.gz

[saraedum]

Author: Charles Bouillaguet

[saraedum]

comment:4

I'll rebase this patch and try to review it.

[saraedum]

rebase of Bouillaguet's patch

[saraedum]

comment:5

Attachment: trac_13697.patch.gz

apply trac_13697.patch

[saraedum]

Branch: u/saraedum/ticket/13697

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. New commits:

[changeset:756831a]	added a doctest for QuotientRingElement.is_unit()
[changeset:1967e3b]	removed trailing whitespace and semicolons from docstrings

[sagetrac-git]

Commit: 756831a

[saraedum]

comment:10

Your patch does not address all the questions in the Ticket summary. How do you want to proceed with this? Should we split this and move the issues to new tickets?

[pjbruin]

comment:13

I ran into the following:

sage: R.<x,y>=QQ[]
sage: Q.<xx,yy>=R.quotient(x^2-y^3)
sage: xx/yy
...
ArithmeticError: Division failed. The numerator is not a multiple of the denominator.

It would be nice if in this situation, the quotient ring could check if it is a domain, and if so, return xx/yy as an element of the fraction field.

[sagetrac-git]

Changed commit from 756831a to 6276d0b

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. New commits:

6276d0b

Merge branch 'develop' into t/13697/ticket/13697

[saraedum]

Changed keywords from none to sd86.5