sagemath / sage

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

trouble with long lines in notebook magma mode #9705

Closed nbruin closed 13 years ago

nbruin commented 14 years ago

The following are three cells in a magma-mode notebook worksheet, together with output (there should be none). Each is valid magma code, but the first one produces an error. The other two examples show that shortening the line by changing its content or by placing a line break make the error go away.

_<x>:=PolynomialRing(Rationals());
repeat
  g:=3*b*x^4+18*c*x^3-6*b^2*x^2-6*b*c*x-b^3-9*c^2 where b:=Random([-10..10]) where c:=Random([-10..10]);
until Roots(g) ne [];
///
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "_sage_input_6.py", line 10, in <module>
    exec compile(u"print _support_.syseval(magma, u'_<x>:=PolynomialRing(Rationals());\\nrepeat\\n  g:=3*b*x^4+18*c*x^3-6*b^2*x^2-6*b*c*x-b^3-9*c^2 where b:=Random([-10..10]) where c:=Random([-10..10]);\\nuntil Roots(g) ne [];', __SAGE_TMP_DIR__)" + '\n', '', 'single')
  File "", line 1, in <module>

  File "/usr/local/sage/4.4.4/local/lib/python2.6/site-packages/sagenb-0.8-py2.6.egg/sagenb/misc/support.py", line 473, in syseval
    return system.eval(cmd, sage_globals, locals = sage_globals)
  File "/usr/local/sage/4.4.4/local/lib/python2.6/site-packages/sage/interfaces/magma.py", line 523, in eval
    raise RuntimeError, "Error evaluating Magma code.\nIN:%s\nOUT:%s"%(x, ans)
RuntimeError: Error evaluating Magma code.
IN:_<x>:=PolynomialRing(Rationals());
repeat
  g:=3*b*x^4+18*c*x^3-6*b^2*x^2-6*b*c*x-b^3-9*c^2 where b:=Random([-10..10]) where c:=Random([-10..10]);
until Roots(g) ne [];
OUT:

>> load "/home/nobody/.sage//temp/ella.cecm.sfu.ca/21960//interface//tmp21960"
   ^
User error: bad syntax

>> until Roots(g) ne [];
   ^
User error: bad syntax
_<x>:=PolynomialRing(Rationals());
repeat
  g:=x^3+b*x+c where b:=Random([-10..10]) where c:=Random([-10..10]);
until Roots(g) ne [];
///
_<x>:=PolynomialRing(Rationals());
repeat
  g:=3*b*x^4+18*c*x^3-6*b^2*x^2-6*b*c*x-b^3-9*c^2
     where b:=Random([-10..10]) where c:=Random([-10..10]);
until Roots(g) ne [];
///

Apply:

  1. attachment: trac_9705.patch
  2. attachment: trac-9705-magma_block_evaluation-doc.patch
  3. attachment: trac-9705-magma_block_evaluation-linebreak.patch

Component: notebook

Author: William Stein

Reviewer: Martin Raum

Merged: sage-4.7.alpha5

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

kwankyu commented 14 years ago
comment:1

If I remember correctly, my own investigation into this bug revealed that this is caused by Sage's use of temporary files for lengthy input, which causes truncations of input at wrong syntactical places.

As you see in the first input, a temporary file is used for the first three lines, and the last line "until ..." is input separately. Both chunks of Magma codes are of course syntactically wrong. Thus Magma complains.

kedlaya commented 13 years ago
comment:2

Here's a similar but even stranger example. This valid Magma which should return nothing, but it returns an error when evaluated in a notebook cell:

MR:=MatrixRing(Rationals(),4); 
embed:=function(A); 
return MR!Matrix([[A[1][1],A[1][2],0,0],[A[2][1],A[2][2],0,0],[0,0,A[1][1],A[1][2]],[0,0,0,A[2][2]]]); 
end function;
///
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "_sage_input_27.py", line 10, in <module>
    exec compile(u"print _support_.syseval(magma, u'MR:=MatrixRing(Rationals(),4); \\nembed:=function(A); \\nreturn MR!Matrix([[A[1][1],A[1][2],0,0],[A[2][1],A[2][2],0,0],[0,0,A[1][1],A[1][2]],[0,0,0,A[2][2]]]); \\nend function;', __SAGE_TMP_DIR__)" + '\n', '', 'single')
  File "", line 1, in <module>

  File "/scratch/sage-x64/devel/sagenb-main/sagenb/misc/support.py", line 473, in syseval
    return system.eval(cmd, sage_globals, locals = sage_globals)
  File "/scratch/sage-x64/local/lib/python2.6/site-packages/sage/interfaces/magma.py", line 523, in eval
    raise RuntimeError, "Error evaluating Magma code.\nIN:%s\nOUT:%s"%(x, ans)
RuntimeError: Error evaluating Magma code.
IN:MR:=MatrixRing(Rationals(),4); 
embed:=function(A); 
return MR!Matrix([[A[1][1],A[1][2],0,0],[A[2][1],A[2][2],0,0],[0,0,A[1][1],A[1][2]],[0,0,0,A[2][2]]]); 
end function;
OUT:

>> load "/home/r1/kedlaya/.sage//temp/dwork.mit.edu/32584//interface//tmp32588
   ^
User error: bad syntax

>> end function;
   ^
User error: bad syntax

Apparently the last line got separated from the rest by mistake. The same sort of thing happens if I concatenate the first two commands into one line and the other two into a second line. But if I make the line even longer by concatenating all four commands together, the error vanishes!

MR:=MatrixRing(Rationals(),4); embed:=function(A); return MR!Matrix([[A[1][1],A[1][2],0,0],[A[2][1],A[2][2],0,0],[0,0,A[1][1],A[1][2]],[0,0,0,A[2][2]]]); end function;
///

I can also avoid the error by slightly shortening the third line in the original (e.g., by changing the last nonzero matrix entry to 0).

I suppose what's going on is that Sage never breaks up an individual line, but it does or does not concatenate multiple lines into the same temporary file based on length considerations. This clearly needs to be done more intelligently.

williamstein commented 13 years ago
comment:4

Hi,

I'm attaching a patch that fixes this issue. It's hard to test because of other issues with the sage/magma interface being broken.

The idea of the fix is simply that for Magma we just evaluate the input block as one big block instead of splitting things up into separate lines. I implemented this by adding an option to the base class in expect.py. Obviously, this same sort of bug could happen with some other interfaces, and I'm not addressing that problem in this ticket.

williamstein commented 13 years ago

Attachment: trac_9705.patch.gz

18d65770-dc1e-4813-89a9-4828e4aae4a9 commented 13 years ago

Author: William Stein

18d65770-dc1e-4813-89a9-4828e4aae4a9 commented 13 years ago
comment:6

This looks good for; Tests are OK.

18d65770-dc1e-4813-89a9-4828e4aae4a9 commented 13 years ago

Description changed:

--- 
+++ 
@@ -49,3 +49,5 @@
 ///

+Apply: +1. trac_9705.patch

18d65770-dc1e-4813-89a9-4828e4aae4a9 commented 13 years ago

Reviewer: Martin Raum

jdemeyer commented 13 years ago
comment:7

Problems while building the documentation:

dochtml.log:/mnt/usb1/scratch/jdemeyer/merger/sage-4.7.alpha3/local/lib/python2.6/site-packages/sage/interfaces/magma.py:docstring of sage.interfaces.magma.Magma.eval:33: (WARNING/2) Block quote ends without a blank line; unexpected unindent.
dochtml.log:/mnt/usb1/scratch/jdemeyer/merger/sage-4.7.alpha3/local/lib/python2.6/site-packages/sage/interfaces/magma.py:docstring of sage.interfaces.magma:33: (ERROR/3) Unexpected indentation.
18d65770-dc1e-4813-89a9-4828e4aae4a9 commented 13 years ago
comment:8

Attachment: trac-9705-magma_block_evaluation-doc.patch.gz

I reformated the old part of the function's documentation, that was incorrect. Now everything builds fine with the right output.

Sorry for this trouble!

18d65770-dc1e-4813-89a9-4828e4aae4a9 commented 13 years ago

Description changed:

--- 
+++ 
@@ -50,4 +50,6 @@

Apply: -1. trac_9705.patch +1. attachment: trac_9705.patch +2. attachment: trac-9705-magma_block_evaluation-doc.patch +

jdemeyer commented 13 years ago
comment:9

I still have the same problems, even with the new patch. Are you sure your documentation built fine? Do make doc-html and look for WARNING or ERROR in the output.

18d65770-dc1e-4813-89a9-4828e4aae4a9 commented 13 years ago
comment:10

Jeroen, there really isn't anything like this on my computer (based on 4.7alpha1). I just have checked once more. If possible could you post the output?

William, if you have time to have a look at this, please do so. I somewhat have no clue why this should happen.

jdemeyer commented 13 years ago
comment:11

I will have a second look.

jdemeyer commented 13 years ago
comment:12

When applying your patches, I get:

$ ./sage -b
[...]
$ make doc-html
[...]
sphinx-build -b html -d /usr/local/src/sage-4.7.alpha2/devel/sage/doc/output/doctrees/en/reference    /usr/local/src/sage-4.7.alpha2/devel/sage/doc/en/reference /usr/local/src/sage-4.7.alpha2/devel/sage/doc/output/html/en/reference
Running Sphinx v1.0.4
loading pickled environment... done
building [html]: targets for 2 source files that are out of date
updating environment: 0 added, 2 changed, 0 removed
reading sources... [ 50%] sage/interfaces/expect
reading sources... [100%] sage/interfaces/magma

/usr/local/src/sage-4.7.alpha2/local/lib/python2.6/site-packages/sage/interfaces/magma.py:docstring of sage.interfaces.magma.Magma.eval:33: (WARNING/2) Block quote ends without a blank line; unexpected unindent.
/usr/local/src/sage-4.7.alpha2/local/lib/python2.6/site-packages/sage/interfaces/magma.py:docstring of sage.interfaces.magma:33: (ERROR/3) Unexpected indentation.
looking for now-outdated files... none found
pickling environment... done
checking consistency... done
preparing documents... done
writing output... [ 25%] index
writing output... [ 50%] interfaces
writing output... [ 75%] sage/interfaces/expect
writing output... [100%] sage/interfaces/magma

writing additional files... genindex py-modindex search
copying static files... done
dumping search index... done
dumping object inventory... done
build succeeded, 2 warnings.
Build finished.  The built documents can be found in /usr/local/src/sage-4.7.alpha2/devel/sage/doc/output/html/en/reference
[...]

I will have a look to fix this.

jdemeyer commented 13 years ago
comment:13

This one is more involved than I guessed: the problem seems to be the "\n" symbol in the magma command. I don't know what to do with this. Is there a way to rewrite the test such that no "\n" is needed? Otherwise I would suggest to ask sage-devel.

18d65770-dc1e-4813-89a9-4828e4aae4a9 commented 13 years ago
comment:14

Indeed, the \n are not necessary as they are replaced by '' anyway. The point of the patch was, that before it was split again later. You might already have a patch for this. If not, let me know, and I will prepare one.

jdemeyer commented 13 years ago
comment:15

Replying to @sagetrac-mraum:

Indeed, the \n are not necessary as they are replaced by '' anyway. The point of the patch was, that before it was split again later. You might already have a patch for this. If not, let me know, and I will prepare one.

You better do it as I don't know nor have Magma.

18d65770-dc1e-4813-89a9-4828e4aae4a9 commented 13 years ago

Description changed:

--- 
+++ 
@@ -52,4 +52,5 @@
 **Apply:**
 1. [attachment: trac_9705.patch](https://github.com/sagemath/sage-prod/files/10650383/trac_9705.patch.gz)
 2. [attachment: trac-9705-magma_block_evaluation-doc.patch](https://github.com/sagemath/sage-prod/files/10650384/trac-9705-magma_block_evaluation-doc.patch.gz)
+3. [attachment: trac-9705-magma_block_evaluation-linebreak.patch](https://github.com/sagemath/sage-prod/files/10650385/trac-9705-magma_block_evaluation-linebreak.patch.gz)
18d65770-dc1e-4813-89a9-4828e4aae4a9 commented 13 years ago
comment:16

Attachment: trac-9705-magma_block_evaluation-linebreak.patch.gz

I removed the linebreaks an the tests pass. The document builds without warnings, but this should be checked by others.

jdemeyer commented 13 years ago
comment:17

I will check that the documentation works.

jdemeyer commented 13 years ago

Merged: sage-4.7.alpha5