Opened 9 years ago
Last modified 4 years ago
#13697 needs_info enhancement
Improvements and corrections to QuotientRingElement
Reported by:  Bouillaguet  Owned by:  malb 

Priority:  minor  Milestone:  sage6.4 
Component:  commutative algebra  Keywords:  sd86.5 
Cc:  Merged in:  
Authors:  Charles Bouillaguet  Reviewers:  
Report Upstream:  N/A  Work issues:  
Branch:  u/saraedum/ticket/13697 (Commits, GitHub, GitLab)  Commit:  6276d0b8134008954f31b66c3055ae946078c657 
Dependencies:  #13670,#13671,#13675,#13714  Stopgaps: 
Description (last modified by )
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 whereR
is a multivariate polynomial ring. In particular, the implementation of division first checks ifR
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 createMPolynomialQuotientRing
andMPolynomialQuotientRingElement
, and host multivariatepolynomialspecific code here.
 The
is_unit()
function does almost nothing (it checks if its argument is a unit inR
). 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 byx
should only be allowed ifx
is regular.
 The present implementation of division has problems. It contains multivariatepolynomialspecific code, which is bad. Furthermore, it allows division by zerodivisors, 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 zerodivisor
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
Attachments (2)
Change History (19)
comment:1 Changed 9 years ago by
 Dependencies changed from #13670,#13671,#13675 to #13670,#13671,#13675,#13714
Changed 9 years ago by
comment:2 Changed 8 years ago by
 Status changed from new to needs_review
comment:3 Changed 8 years ago by
comment:4 Changed 8 years ago by
comment:6 Changed 8 years ago by
 Milestone changed from sage5.11 to sage5.12
comment:7 Changed 8 years ago by
 Branch set to u/saraedum/ticket/13697
 Created changed from 11/10/12 16:44:59 to 11/10/12 16:44:59
 Modified changed from 08/13/13 15:35:53 to 08/13/13 15:35:53
comment:8 Changed 8 years ago by
 Description modified (diff)
 Milestone changed from sage5.12 to sage6.0
comment:9 Changed 8 years ago by
 Commit set to 756831a00ba9a0a1624e9b516621d5420a7cf2f7
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 
comment:10 Changed 8 years ago by
 Modified changed from 09/12/13 15:07:35 to 09/12/13 15:07:35
 Status changed from needs_review to needs_info
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?
comment:11 Changed 8 years ago by
 Milestone changed from sage6.0 to sage6.1
comment:12 Changed 8 years ago by
 Milestone changed from sage6.1 to sage6.2
comment:13 Changed 7 years ago by
I ran into the following:
sage: R.<x,y>=QQ[] sage: Q.<xx,yy>=R.quotient(x^2y^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.
comment:14 Changed 7 years ago by
 Milestone changed from sage6.2 to sage6.3
comment:15 Changed 7 years ago by
 Milestone changed from sage6.3 to sage6.4
comment:16 Changed 4 years ago by
 Commit changed from 756831a00ba9a0a1624e9b516621d5420a7cf2f7 to 6276d0b8134008954f31b66c3055ae946078c657
Branch pushed to git repo; I updated commit sha1. New commits:
6276d0b  Merge branch 'develop' into t/13697/ticket/13697

comment:17 Changed 4 years ago by
 Keywords sd86.5 added
I'll rebase this patch and try to review it.