optional doctest failure -- breakage in the sage<-->magma interface #6329

williamstein · 2009-06-16T15:06:55Z

sage -t -long --optional devel/sage/sage/rings/rational.pyx
**********************************************************************
File "/scratch/wstein/build/sage-4.0.2.alpha3/devel/sage-main/sage/rings/rational.pyx", line 3087:
    sage: magma(3/1).Type()             # optional
Expected:
    FldRatElt
Got:
    RngIntElt
**********************************************************************
1 items had failures:
   1 of   5 in __main__.example_84
***Test Failed*** 1 failures.

Apply only attachment: trac_6329.2.patch to the Sage library.

Component: interfaces

Keywords: sd32

Author: Mariah Lenox, William Stein

Reviewer: Mike Hansen

Merged: sage-4.7.2.alpha3

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

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

williamstein · 2009-06-16T15:10:31Z

comment:1

More failures:

sage -t -long --optional devel/sage/sage/schemes/hyperelliptic_curves/hyperelliptic_g2_generic.py
**********************************************************************
File "/scratch/wstein/build/sage-4.0.2.alpha3/devel/sage-main/sage/schemes/hyperelliptic_curves/hyperelliptic_g2_generic.py", line 218:
    sage: magma(HyperellipticCurve(f)).IgusaClebschInvariants() # optional - magma
Expected:
    [ 0, -2048/375, -4096/25, -4881645568/84375 ]
Got:
    [ -640, -20480, 1310720, 52160364544 ]
**********************************************************************
File "/scratch/wstein/build/sage-4.0.2.alpha3/devel/sage-main/sage/schemes/hyperelliptic_curves/hyperelliptic_g2_generic.py", line 220:
    sage: magma(HyperellipticCurve(f(2*x))).IgusaClebschInvariants() # optional - magma
Expected:
    [ 0, -8388608/375, -1073741824/25, -5241627016305836032/84375 ]
Got:
    [ -40960, -83886080, 343597383680, 56006764965979488256 ]
**********************************************************************
File "/scratch/wstein/build/sage-4.0.2.alpha3/devel/sage-main/sage/schemes/hyperelliptic_curves/hyperelliptic_g2_generic.py", line 222:
    sage: magma(HyperellipticCurve(f, x)).IgusaClebschInvariants() # optional - magma
Expected:
    [ -8/15, 17504/5625, -23162896/140625, -420832861216768/7119140625 ]
Got:
    [ -640, 17920, -1966656, 52409511936 ]
**********************************************************************
File "/scratch/wstein/build/sage-4.0.2.alpha3/devel/sage-main/sage/schemes/hyperelliptic_curves/hyperelliptic_g2_generic.py", line 224:
    sage: magma(HyperellipticCurve(f(2*x), 2*x)).IgusaClebschInvariants() # optional - magma
Expected:
    [ -512/15, 71696384/5625, -6072014209024/140625, -451865844002031331704832/7119140625 ]
Got:
    [ -40960, 73400320, -515547070464, 56274284941110411264 ]
**********************************************************************
1 items had failures:
   4 of  12 in __main__.example_7
***Test Failed*** 4 failures.

williamstein · 2009-06-16T15:19:18Z

comment:2

More failures:

sage -t -long --optional devel/sage/sage/interfaces/magma.py
**********************************************************************
File "/scratch/wstein/build/sage-4.0.2.alpha3/devel/sage-main/sage/interfaces/magma.py", line 856:
    sage: magma.attach('%s/data/extcode/magma/sage/basic.m'%Sage_ROOT)    # optional - magma
Exception raised:
    Traceback (most recent call last):
      File "/scratch/wstein/build/sage-4.0.2.alpha3/local/bin/ncadoctest.py", line 1231, in run_one_test
        self.run_one_example(test, example, filename, compileflags)
      File "/scratch/wstein/build/sage-4.0.2.alpha3/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.0.2.alpha3/local/bin/ncadoctest.py", line 1172, in run_one_example
        compileflags, 1) in test.globs
      File "<doctest __main__.example_20[2]>", line 1, in <module>
        magma.attach('%s/data/extcode/magma/sage/basic.m'%Sage_ROOT)    # optional - magma###line 856:
    sage: magma.attach('%s/data/extcode/magma/sage/basic.m'%Sage_ROOT)    # optional - magma
    NameError: name 'Sage_ROOT' is not defined
**********************************************************************
File "/scratch/wstein/build/sage-4.0.2.alpha3/devel/sage-main/sage/interfaces/magma.py", line 860:
    sage: magma.attach('%s/data/extcode/magma/sage/basic2.m'%Sage_ROOT)     # optional - magma
Expected:
    Traceback (most recent call last):
    ...
    RuntimeError: Error evaluating Magma code...
Got:
    Traceback (most recent call last):
      File "/scratch/wstein/build/sage-4.0.2.alpha3/local/bin/ncadoctest.py", line 1231, in run_one_test
        self.run_one_example(test, example, filename, compileflags)
      File "/scratch/wstein/build/sage-4.0.2.alpha3/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.0.2.alpha3/local/bin/ncadoctest.py", line 1172, in run_one_example
        compileflags, 1) in test.globs
      File "<doctest __main__.example_20[3]>", line 1, in <module>
        magma.attach('%s/data/extcode/magma/sage/basic2.m'%Sage_ROOT)     # optional - magma###line 860:
    sage: magma.attach('%s/data/extcode/magma/sage/basic2.m'%Sage_ROOT)     # optional - magma
    NameError: name 'Sage_ROOT' is not defined
**********************************************************************
File "/scratch/wstein/build/sage-4.0.2.alpha3/devel/sage-main/sage/interfaces/magma.py", line 917:
    sage: print magma.load(SAGE_TMP + 'a.m')      # optional - magma
Expected:
    Loading ".../.sage//temp/.../a.m"
    hi
Got:
    Loading "/scratch/wstein/sage//temp/sage.math.washington.edu/31604/a.m"
    hi
**********************************************************************
File "/scratch/wstein/build/sage-4.0.2.alpha3/devel/sage-main/sage/interfaces/magma.py", line 2148:
    sage: magma.eval('R<x> := PolynomialRing(RationalField()); f := (x-17/2)^3;')     # optional - magma
Expected:
    "
Got:
    ''
**********************************************************************
File "/scratch/wstein/build/sage-4.0.2.alpha3/devel/sage-main/sage/interfaces/magma.py", line 2160:
    sage: magma.eval('K<a> := CyclotomicField(11)')       # optional - magma
Expected:
    "
Got:
    ''
**********************************************************************
File "/scratch/wstein/build/sage-4.0.2.alpha3/devel/sage-main/sage/interfaces/magma.py", line 463:
    sage: magma.eval("a := %s;"%(10^10000))    # optional - magma
Expected:
    "
Got:
    ''
**********************************************************************
4 items had failures:
   2 of   4 in __main__.example_20
   1 of   5 in __main__.example_22
   2 of  28 in __main__.example_64
   1 of   4 in __main__.example_9
***Test Failed*** 6 failures.
For whitespace errors, see the file /home/wstein/build/sage-4.0.2.alpha3/tmp/.doctest_magma.py
	 [32.9 s]

sagetrac-mariah · 2011-05-25T18:24:06Z

Attachment: trac_6329.patch.gz

sagetrac-mariah · 2011-05-25T18:35:39Z

comment:3

[attachment: trac_6329.patch] corrects the test output in the problem in the description. The Got output is correct - there is not a breakage in the sage<-->magma interface. 'magma(3/1)' returns the integer '3' (after coercion), so Type() returns that
the value is an integer.

The other two reports of failures are no longer valid, as they seem to already be fixed in sage-4.7.rc4.

sagetrac-mariah · 2011-05-25T18:35:39Z

Author: Mariah Lenox

mwhansen · 2011-08-23T01:15:48Z

comment:5

It seems like the correct fix would be to always put the denominator in the _magma_init_ method.

        s = self.numerator()._magma_init_(magma)
        s += '/' + self.denominator()._magma_init_(magma)
        return s

instead of

        s = self.numerator()._magma_init_(magma)
        if not self.is_integral():
            s += '/' + self.denominator()._magma_init_(magma)
        return s

williamstein · 2011-08-23T05:04:52Z

comment:6

Mhansen, you're right, the patch Mariah posted is wrong. I'll post a correct patch, and hopefully you can review it.

williamstein · 2011-08-23T05:34:53Z

Attachment: trac_6329.2.patch.gz

apply only this one

williamstein · 2011-08-23T05:39:46Z

comment:7

Patch up that is ready for review (the .2 one). This was a result of some overzealous optimization on my part a few years ago. Reverting this change will barely slow things down.

mwhansen · 2011-08-23T05:49:03Z

comment:8

We should add a doctest for the integral case.

williamstein · 2011-08-24T04:49:28Z

comment:9

Replying to @mwhansen:

We should add a doctest for the integral case.

What do you mean by this? If you mean sage: magma(3/1).Type(), which is in the ticket description, then there is already a doctest for this case, which is how we found this bug in the first place.

Or do you mean adding something to integer.pyx???

williamstein · 2011-08-24T23:36:45Z

Changed keywords from none to sd32

mwhansen · 2011-08-24T23:40:24Z

comment:11

I was thinking about the old code and verifying that the "is_integral" code path worked (since that test was missing before). However, it is obviously not needed now.

Positive review.

nexttime · 2011-09-09T04:33:04Z

Reviewer: Mike Hansen

nexttime · 2011-09-09T04:33:04Z

Changed author from Mariah Lenox to Mariah Lenox, William Stein

nexttime · 2011-09-12T19:27:00Z

Merged: sage-4.7.2.alpha3

williamstein · 2012-01-22T21:57:34Z

comment:14

Shame on the idiot who wrote this with no tests... and who forgot that there are optional tests all over Sage that would be broken by this: see #12006, where I'm doing my penance.

williamstein added this to the sage-4.7 milestone Jun 16, 2009

williamstein added c: packages: optional labels Jun 16, 2009

williamstein changed the title ~~optional doctest failure -- bad data type breakage in the sage-->magma interface~~ optional doctest failure -- breakage in the sage<-->magma interface Jun 16, 2009

sagetrac-mariah mannequin modified the milestones: sage-4.7, sage-4.7.1 May 25, 2011

sagetrac-mariah mannequin added s: needs review and removed p: major / 3 labels May 25, 2011

kcrisman added c: interfaces and removed c: packages: optional labels Jun 14, 2011

mwhansen added s: needs info and removed s: needs review labels Aug 23, 2011

williamstein added s: needs review and removed s: needs info labels Aug 23, 2011

mwhansen removed the s: needs review label Aug 24, 2011

mwhansen added the s: positive review label Aug 24, 2011

This comment has been minimized.

Sign in to view

nexttime mannequin removed the s: positive review label Sep 12, 2011

nexttime mannequin closed this as completed Sep 12, 2011

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

optional doctest failure -- breakage in the sage<-->magma interface #6329

optional doctest failure -- breakage in the sage<-->magma interface #6329

williamstein commented Jun 16, 2009

williamstein commented Jun 16, 2009

williamstein commented Jun 16, 2009

sagetrac-mariah mannequin commented May 25, 2011

sagetrac-mariah mannequin commented May 25, 2011

sagetrac-mariah mannequin commented May 25, 2011

mwhansen commented Aug 23, 2011

williamstein commented Aug 23, 2011

williamstein commented Aug 23, 2011

williamstein commented Aug 23, 2011

mwhansen commented Aug 23, 2011

williamstein commented Aug 24, 2011

williamstein commented Aug 24, 2011

mwhansen commented Aug 24, 2011

nexttime mannequin commented Sep 9, 2011

This comment has been minimized.

nexttime mannequin commented Sep 9, 2011

nexttime mannequin commented Sep 12, 2011

williamstein commented Jan 22, 2012

optional doctest failure -- breakage in the sage<-->magma interface #6329

optional doctest failure -- breakage in the sage<-->magma interface #6329

Comments

williamstein commented Jun 16, 2009

williamstein commented Jun 16, 2009

williamstein commented Jun 16, 2009

sagetrac-mariah mannequin commented May 25, 2011

sagetrac-mariah mannequin commented May 25, 2011

sagetrac-mariah mannequin commented May 25, 2011

mwhansen commented Aug 23, 2011

williamstein commented Aug 23, 2011

williamstein commented Aug 23, 2011

williamstein commented Aug 23, 2011

mwhansen commented Aug 23, 2011

williamstein commented Aug 24, 2011

williamstein commented Aug 24, 2011

mwhansen commented Aug 24, 2011

nexttime mannequin commented Sep 9, 2011

This comment has been minimized.

nexttime mannequin commented Sep 9, 2011

nexttime mannequin commented Sep 12, 2011

williamstein commented Jan 22, 2012