Can't construct fraction field #5982

jmbr · 2009-05-04T23:19:45Z

----------------------------------------------------------------------
| Sage Version 3.4.2, Release Date: 2009-05-04                       |
| Type notebook() for the GUI, and license() for information.        |
----------------------------------------------------------------------
sage: R.<x, y> = PolynomialRing(QQ, 2) 
sage: I = (x^2 + y^2 - 1)*R
sage: Q = R.quotient(I); Q
Quotient of Multivariate Polynomial Ring in x, y over Rational Field by the ideal (x^2 + y^2 - 1)
sage: Q.fraction_field()
---------------------------------------------------------------------------
NotImplementedError                       Traceback (most recent call last)
<SNIP>

Apply

CC: @malb

Component: algebra

Author: Nick Alexander, Juan M. Bello Rivas

Reviewer: William Stein, Mike Hansen, Marco Streng

Merged: sage-4.7.1.alpha0

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

The text was updated successfully, but these errors were encountered:

jmbr · 2009-05-04T23:22:22Z

Attachment: sage+multi-polynomial-fraction-fields2.diff.gz

Fix/workaround using Macaulay 2

jmbr · 2009-05-04T23:23:26Z

comment:2

[with patch, needs review]

----------------------------------------------------------------------
| Sage Version 3.4, Release Date: 2009-03-11 | | Type notebook() for
the GUI, and license() for information.  |
----------------------------------------------------------------------

sage: R.<x, y> = PolynomialRing(QQ, 2) sage: I = (x^2 + y^2 - 1)*R
sage: Q = R.quotient(I) sage: Q Quotient of Multivariate Polynomial
Ring in x, y over Rational Field by the ideal (x^2 + y^2 - 1) sage:
Q.fraction_field()
---------------------------------------------------------------------------
NotImplementedError Traceback (most recent call last)

...

sagetrac-mabshoff · 2009-05-05T01:28:51Z

comment:4

Hi,

M2 isn't a default part of Sage, so the doctest must be made optional. I also don't see a reason why this cannot be implemented via libSingular for example, but maybe I am overlooking things.

You also posted a diff, so please check in the code and post an hg patch to properly preserve credit.

And since you are not listed at http://trac.sagemath.org/sage_trac/wiki please add yourself there.

Cheers,

Michael

jmbr · 2009-05-06T16:59:07Z

comment:6

Hi Michael,

Attached is a Mercurial bundle with the same patch plus the docstring modifications you requested. I'd like to write a native Sage implementation of this in the future but for now this works well enough for me.

Regards,

sagetrac-mabshoff · 2009-05-06T21:47:17Z

comment:7

Hi Juan,

a patch would have been better, but someone can extra the changeset from the bundle and post the patch.

One more thing: When you post an updated patch you need to change the summary to read "needs review" again.

Cheers,

Michael

ncalexan · 2009-06-19T22:24:11Z

Attachment: trac_5982.patch.gz

Attachment: trac_5982-ncalexan.patch.gz

ncalexan · 2009-06-19T22:25:03Z

comment:8

trac_5982-ncalexan.patch replaces all others. This version doesn't use Macaulay2, but it could be really slow.

ncalexan · 2009-06-19T22:25:03Z

Author: Nick Alexander

craigcitro · 2009-06-20T21:43:00Z

comment:9

Would it be reasonable to have both versions of is_prime added, with a flag for which to use (defaulting to native, but with M2 an option)? This way, if there were cases that one cared about but were slow, one could call off to M2 and (hopefully) get an answer back faster ...

ncalexan · 2009-06-21T08:17:45Z

comment:10

I thought about doing exactly that, but if you're clever enough to know one is better than the other, aren't you clever enough to call _macaulay2_().isPrime()? Also, as far as I can tell, the Macaulay2 algorithm is the same as the Singular.

jmbr · 2009-06-21T10:20:29Z

comment:11

Hi Nick,

Could you point me to the location of Singular's ideal primality testing algorithm? Thanks

ncalexan · 2009-06-21T18:39:51Z

comment:12

Replying to @jmbr:

Hi Nick,

Could you point me to the location of Singular's ideal primality testing algorithm? Thanks

AFAICT, Singular does not have a toplevel isprime() (or ismaximal()) function. Reading some mailing list posts regarding Singular and Macaulay2, I believe both implement the same two algorithms for calculating the complete primary decomposition of an ideal. I don't understand enough to say definitively and I may be misusing the complete primary decomposition for testing primality. I was hoping that Martin Albrecht would review and clarify the situation.

malb · 2009-06-21T20:36:14Z

comment:13

The patch looks good to me, I didn't run doctests though. I can't really clarify the situation any more than what Nick wrote above.

jmbr · 2009-06-21T20:53:42Z

comment:14

As far as I can recall, Macaulay2 implements the Gianni-Trager-Zacharias algorithm for primality testing instead of relying on primary decomposition.

Best regards

williamstein · 2009-07-21T04:51:47Z

comment:15

Positive review modulo fixing this one trivial change to a doctest:

wstein@sage:~/build/sage-4.1$ ./sage -t  devel/sage/sage/rings/quotient_ring.py
sage -t  "devel/sage/sage/rings/quotient_ring.py"           
**********************************************************************
File "/scratch/wstein/build/sage-4.1/devel/sage/sage/rings/quotient_ring.py", line 442:
    sage: Q.is_integral_domain()
Expected:
    Traceback (most recent call last):
    ...
    NotImplementedError
Got:
    Traceback (most recent call last):
      File "/scratch/wstein/build/sage-4.1/local/bin/ncadoctest.py", line 1231, in run_one_test
        self.run_one_example(test, example, filename, compileflags)
      File "/scratch/wstein/build/sage-4.1/local/bin/sagedoctest.py", line 38, in run_one_example
        OrigDocTestRunner.run_one_example(self, test, example, filename, compileflags)
      File "/scratch/wstein/build/sage-4.1/local/bin/ncadoctest.py", line 1172, in run_one_example
        compileflags, 1) in test.globs
      File "<doctest __main__.example_13[7]>", line 1, in <module>
        Q.is_integral_domain()###line 442:
    sage: Q.is_integral_domain()
      File "/scratch/wstein/build/sage-4.1/local/lib/python/site-packages/sage/rings/quotient_ring.py", line 447, in is_integral_domain
        return self.defining_ideal().is_prime()
      File "/scratch/wstein/build/sage-4.1/local/lib/python/site-packages/sage/rings/polynomial/multi_polynomial_ideal.py", line 854, in is_prime
        CPD = self.complete_primary_decomposition(**kwds)
      File "/scratch/wstein/build/sage-4.1/local/lib/python/site-packages/sage/rings/polynomial/multi_polynomial_ideal.py", line 401, in __call__
        raise ValueError("Coefficient ring must be a field for function '%s'."%(self.f.__name__))
    ValueError: Coefficient ring must be a field for function 'complete_primary_decomposition'.
**********************************************************************
1 items had failures:
   1 of   8 in __main__.example_13
***Test Failed*** 1 failures.
For whitespace errors, see the file /scratch/wstein/build/sage-4.1/tmp/.doctest_quotient_ring.py
         [1.8 s]
exit code: 1024
 
----------------------------------------------------------------------
The following tests failed:


        sage -t  "devel/sage/sage/rings/quotient_ring.py"
Total time for all tests: 1.8 seconds
wstein@sage:~/build/sage-4.1$

mwhansen · 2009-10-16T08:40:17Z

comment:22

In 4.1.2, I get a number of failures arising from NotImplementedErrors in is_integral_domain.

loefflerd · 2011-01-23T18:36:18Z

comment:24

Curiously, all tests in sage/rings and sage/schemes pass for me under sage 4.6.2.alpha1. I'm going to reinstate the "needs review", since I can't replicate mhansen's problem. We'll see if patchbot agrees :-)

Apply trac_5982-ncalexan-with-check.patch, trac_5982-reviewer.patch

loefflerd · 2011-01-23T19:30:00Z

comment:25

Patchbot seems to be happy with it, but a full ptestlong run on my 4.6.2.alpha1 build revealed a couple of failures: sage/algebras/quatalg/quaternion_algebra.py and sage/matrix/tests.py. Both of these are to do with the symbolic ring, which (rather oddly) knows it's a field but isn't sure if it's an integral domain. These can be squashed by simply moving the field check to before the integral domain check.

mstreng · 2011-02-14T09:41:26Z

comment:26

Attachment: trac_5982_review2.patch.gz

Why is this

        The following is Trac #5982.  Note that the quotient ring 
        is not recognized as being a field at this time, so the 
        fraction field is not the quotient ring itself:: 
 
            sage: Q = R.quotient(I); Q 
            Quotient of Multivariate Polynomial Ring in x, y over Rational Field by the ideal (x^2 - y^2 - 1) 
            sage: Q.fraction_field() 
            Fraction Field of Quotient of Multivariate Polynomial Ring in x, y over Rational Field by the ideal (x^2 - y^2 - 1)

part of the documentation of is_prime and not of fraction_field?

All tests pass with these patches on sage-4.6.2.alpha4. Patches fix the problem in the ticket description, and all changes look good.

mstreng · 2011-02-14T09:41:26Z

Changed reviewer from William Stein, Mike Hansen to William Stein, Mike Hansen, Marco Streng

JohnCremona · 2011-04-22T15:20:28Z

comment:27

Marco, did you forget to mark this as "positive review"?

mstreng · 2011-04-22T15:45:44Z

comment:28

Replying to @JohnCremona:

Marco, did you forget to mark this as "positive review"?

Actually, I was waiting for an answer to my question first, and then forgot all about the ticket.

If I recall correctly, the question was not that important.

jdemeyer · 2011-04-25T13:18:41Z

comment:30

attachment: trac_5982-ncalexan-with-check.patch is not a proper hg changeset. Patches should be made using hg export tip and not hg diff.

mstreng · 2011-04-28T10:38:14Z

proper hg changeset version of trac_5982-ncalexan-with-check.patch

mstreng · 2011-04-28T10:41:08Z

comment:31

Attachment: trac_5982-ncalexan-with-check2.patch.gz

I made that patch into a proper hg changeset. All tests still pass, and as the new patch does exactly the same as the old one, I put it back to positive_review.

(apply attachment: trac_5982-ncalexan-with-check2.patch, attachment: trac_5982-reviewer.patch, attachment: trac_5982_review2.patch)

mstreng · 2011-04-28T10:46:32Z

Attachment: patches.diff.gz

the difference between trac_5982-ncalexan-with-check.patch and trac_5982-ncalexan-with-check2.patch

mstreng · 2011-04-28T10:47:39Z

comment:32

(apply trac_5982-ncalexan-with-check2.patch, trac_5982-reviewer.patch, trac_5982_review2.patch)

mstreng · 2011-04-28T10:50:02Z

comment:33

(apply trac_5982-ncalexan-with-check2.patch, trac_5982-reviewer.patch, trac_5982_review2.patch )

jdemeyer · 2011-05-03T12:28:22Z

Merged: sage-4.7.1.alpha0

jmbr added this to the sage-4.7 milestone May 4, 2009

jmbr added c: algebra labels May 4, 2009