sagemath / sage

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

Improvements and corrections to QuotientRingElement #13697

Open 89f39f15-88e8-4e79-9bc0-0739a7fc497c opened 11 years ago

89f39f15-88e8-4e79-9bc0-0739a7fc497c commented 11 years ago

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.


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

89f39f15-88e8-4e79-9bc0-0739a7fc497c commented 11 years ago

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

89f39f15-88e8-4e79-9bc0-0739a7fc497c commented 11 years ago
comment:2

Attachment: improve_quotient_ring.patch.gz

saraedum commented 11 years ago

Author: Charles Bouillaguet

saraedum commented 11 years ago
comment:4

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

saraedum commented 11 years ago

rebase of Bouillaguet's patch

saraedum commented 11 years ago
comment:5

Attachment: trac_13697.patch.gz

apply trac_13697.patch

saraedum commented 11 years ago

Description changed:

--- 
+++ 
@@ -36,3 +36,9 @@
    * 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](https://github.com/sagemath/sage-prod/files/10656599/trac_13697.patch.gz)
saraedum commented 11 years ago

Branch: u/saraedum/ticket/13697

saraedum commented 11 years ago

Description changed:

--- 
+++ 
@@ -19,7 +19,7 @@
   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   
+  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:

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 11 years ago

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
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 11 years ago

Commit: 756831a

saraedum commented 11 years ago
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 commented 10 years ago
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.

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 7 years ago

Changed commit from 756831a to 6276d0b

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 7 years ago

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

6276d0bMerge branch 'develop' into t/13697/ticket/13697
saraedum commented 7 years ago

Changed keywords from none to sd86.5