#15631 closed defect (fixed)
Random failures in sandpiles.py
Reported by:  vbraun  Owned by:  

Priority:  major  Milestone:  sage6.3 
Component:  interfaces  Keywords:  pexpect echo singular 
Cc:  Merged in:  
Authors:  Volker Braun  Reviewers:  Nils Bruin 
Report Upstream:  N/A  Work issues:  
Branch:  2528fb3 (Commits)  Commit:  
Dependencies:  Stopgaps: 
Description
As reported on https://groups.google.com/d/msg/sagedevel/ymsEq_f9MNU/0x6vympN2c0J
File "src/sage/sandpiles/sandpile.py", line 4684, in sage.sandpiles.sandpile.complete_sandpile Failed example: K.betti(verbose=False) ... File "multi_polynomial_libsingular.pyx", line 912, in sage.rings.polynomial.multi_polynomial_libsingular.MPolynomialRing_libsingular.__call__ (sage/rings/polynomial/multi_polynomial_libsingular.cpp:7912) TypeError: Could not find a mapping of the passed element to this ring.  (same place but different error:) File "multi_polynomial_libsingular.pyx", line 807, in sage.rings.polynomial.multi_polynomial_libsingular.MPolynomialRing_libsingular.__call__ (sage/rings/polynomial/multi_polynomial_libsingular.cpp:6207) File "/usr/local/sage/sagedev/local/lib/python2.7/sitepackages/sage/interfaces/singular.py", line 1245, in __repr__ elif self.type() == 'matrix': File "/usr/local/sage/sagedev/local/lib/python2.7/sitepackages/sage/interfaces/singular.py", line 1976, in type return m.group(1) AttributeError: 'NoneType' object has no attribute 'group'
This is because the singular pexpect interface has subtle bugs. The easy fix is to rewrite it with disabled terminal echo.
Change History (22)
comment:1 Changed 6 years ago by
 Branch set to u/vbraun/sandpile
comment:2 Changed 6 years ago by
 Commit set to 8d1705962533130ea1b0c1d7a533df3a06a4b06e
comment:3 Changed 6 years ago by
Its even worse, it seems one can't work around this without overriding _eval_line
. Which is a mess...
comment:4 Changed 6 years ago by
 Commit changed from 8d1705962533130ea1b0c1d7a533df3a06a4b06e to 773c907730dafd290b626588666a50053ed35777
Branch pushed to git repo; I updated commit sha1. New commits:
773c907  override the _eval_line method as well

comment:5 Changed 6 years ago by
 Status changed from new to needs_review
Now all tests pass for me (at least once, please try in loop)
comment:6 Changed 6 years ago by
Patch looks good and I haven't observed the problem any more. The patchbot doesn't see real problems. You can browse through the various plugin complaints yourself.
Question: as far as pexpect interface goes, singular is pretty vanilla if I'm not mistaken. That makes me think that whatever approach you have taken to make noecho work with singular might apply more generally. Would it be doable to include the new way of handling "noecho" in our pexpect wrapper and select it with another flag to be set on startup? Then the code could be reused for other interfaces too.
comment:7 Changed 6 years ago by
 Commit changed from 773c907730dafd290b626588666a50053ed35777 to f3e08b9ffdbbbe9a7d4fba6c7e6b1a76c6b68b19
I've added a terminal_echo=<bool>
optional keyword argument to Expect
, this saves some code but makes the _eval_line
method even more ugly.... oh well.
New commits:
f3e08b9  make terminal echo optional in Expect class

comment:8 Changed 6 years ago by
 Milestone changed from sage6.1 to sage6.2
comment:9 Changed 6 years ago by
Nils, are you still reviewing this ticket?
comment:10 Changed 5 years ago by
 Milestone changed from sage6.2 to sage6.3
comment:11 Changed 5 years ago by
 Status changed from needs_review to needs_work
 Work issues set to rebase
comment:12 Changed 5 years ago by
This also affects some patchbot doctesting, e.g. http://patchbot.sagemath.org/ticket/16199/
comment:13 Changed 5 years ago by
 Branch changed from u/vbraun/sandpile to u/rws/sandpile
comment:14 Changed 5 years ago by
 Commit changed from f3e08b9ffdbbbe9a7d4fba6c7e6b1a76c6b68b19 to 2528fb3579a0503f765f787384158f2478e00ed2
 Status changed from needs_work to needs_review
New commits:
2528fb3  Merge branch 'develop' into t/15631/sandpile

comment:15 Changed 5 years ago by
 Status changed from needs_review to positive_review
Ticket was already reviewed. Patchbot is happy also with current version.
comment:16 Changed 5 years ago by
 Reviewers set to Nils Bruin
 Work issues rebase deleted
Hope you don't mind me setting the name.
comment:17 Changed 5 years ago by
comment:18 Changed 5 years ago by
 Branch changed from u/rws/sandpile to 2528fb3579a0503f765f787384158f2478e00ed2
 Resolution set to fixed
 Status changed from positive_review to closed
comment:19 Changed 5 years ago by
 Commit 2528fb3579a0503f765f787384158f2478e00ed2 deleted
 Keywords pexpect echo singular added
It would have been nice if you mentioned pexpect in the ticket's title...
comment:20 Changed 5 years ago by
What's the precise motivation for
if out == '': # match bug with echo out = line
This is breaking a fix for #10476.
comment:21 Changed 5 years ago by
Followup: #17719
comment:22 Changed 5 years ago by
Don't remember, though clearly must have been because something further down is unhappy with an empty string.
Annoyingly, the pexpect stuff in Sage is built around terminal echo: everything up to the first return is presumed echo and ignored. So I had to revert to a hack...
Last 10 new commits:
Disable terminal echo in the Singular interface
Updated Sage version to 6.1.beta3
Trac #15444: Two algorithms for kcharge do not give same answer
Fixed bug in kcharge implementation for Jalgorithm
Trac #15596: Sdist fails with capitalP illow
sagesdist: copy upstream tarballs using sagespkg
Merge remotetracking branch 'trac/u/ohanar/pillow' into t/15596/sdist_fails_with_capital_p_illow
Trac #15602: Fix rigged_configurations.py documentation
Fixed docbuild.
Trac #15561: freetype 2.3.5 is too old for some fonts of current systems.