Opened 5 years ago
Closed 5 years ago
#23142 closed defect (fixed)
Fix missing synchronisation upon starting gap3
Reported by:  nthiery  Owned by:  

Priority:  major  Milestone:  sage8.0 
Component:  interfaces  Keywords:  
Cc:  saliola, stumpc5, jmichel  Merged in:  
Authors:  Nicolas M. Thiéry  Reviewers:  Travis Scrimshaw 
Report Upstream:  N/A  Work issues:  
Branch:  529e4d3 (Commits, GitHub, GitLab)  Commit:  529e4d3cc40f80dda2022a5ef4336d0ea8ed35fc 
Dependencies:  Stopgaps: 
Description
Bug reported by Jean Michel:
┌────────────────────────────────────────────────────────────────────┐ │ SageMath version 8.0.beta9, Release Date: 20170531 │ │ Type "notebook()" for the browserbased notebook interface. │ │ Type "help()" for help. │ └────────────────────────────────────────────────────────────────────┘ ┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┓ ┃ Warning: this is a prerelease version, and it may be unstable. ┃ ┗━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┛ sage: gap3.eval("1+1") '' sage: gap3.eval("1+2") 2 sage: gap3.eval("1+3") 3
Equivalently, the above can be reproduced with the %%gap3
magic.
Analysis: unlike the gap
interface, no synchronization was done upon starting the gap3
interface. This did not show up when calling gap3(cmd)
as the latter triggers an explicit synchronization.
This ticket adds the missing synchronization and, upon suggestion by Jean, adds the option b
in the call to gap3 (no banner).
Change History (15)
comment:1 Changed 5 years ago by
 Status changed from new to needs_review
comment:2 Changed 5 years ago by
comment:3 Changed 5 years ago by
Okay, it seems the commit got mixed in with #23132.
comment:4 Changed 5 years ago by
 Branch set to u/nthiery/fix_missing_synchronisation_upon_starting_gap3
comment:5 Changed 5 years ago by
 Commit set to fda6e26cb2965c8d863062eaacaf6ac288106b96
Oops; thanks for spotting this. Fixed!
New commits:
fda6e26  23132: Fix missing synchronisation upon starting gap3 + doc + skip gap3 banner

comment:6 Changed 5 years ago by
 Reviewers set to Travis Scrimshaw
 Status changed from needs_review to positive_review
LGTM.
comment:7 Changed 5 years ago by
Thanks!
comment:8 Changed 5 years ago by
 Status changed from positive_review to needs_work
hey, guys.. doctests do not pass...
comment:9 Changed 5 years ago by
 Commit changed from fda6e26cb2965c8d863062eaacaf6ac288106b96 to 50798771d9b30d558c9446699c2374667bf256ef
Branch pushed to git repo; I updated commit sha1. New commits:
5079877  23142: trivial doctest update

comment:10 Changed 5 years ago by
 Commit changed from 50798771d9b30d558c9446699c2374667bf256ef to 529e4d3cc40f80dda2022a5ef4336d0ea8ed35fc
Branch pushed to git repo; I updated commit sha1. New commits:
529e4d3  23142: another trivial doctest update

comment:11 Changed 5 years ago by
Arr, sorry! Apparently I am getting lazy due to being used to having a continuous integration tool immediately running the tests for me ... I can't wait until we really have this for Sage (e.g. more patchbots; we really need to set up a couple of them in Orsay). In the mean time, I indeed should have waited for my Sage to compile and run the tests for real before setting this for review.
comment:12 Changed 5 years ago by
The following failures already appear with develop:
mistral/opt/sage/src/sage/interfaces>sage t optional gap3 gap3.py too many failed tests, not using stored timings Running doctests with ID 20170606162424c343198f. Git branch: develop Using optional=gap3 Doctesting 1 file. sage t gap3.py ********************************************************************** File "gap3.py", line 441, in sage.interfaces.gap3.Gap3.help Failed example: m #optional  gap3 Expected: [ [ 1, 2, 3 ], [ 4, 5, 6 ] ] Got: <BLANKLINE> ********************************************************************** File "gap3.py", line 443, in sage.interfaces.gap3.Gap3.help Failed example: m.Print() #optional  gap3 Expected: [ [ 1, 2, 3 ], [ 4, 5, 6 ] ] Got: "__SAGE_LAST__" ********************************************************************** File "gap3.py", line 447, in sage.interfaces.gap3.Gap3.help Failed example: m #optional  gap3 Expected: [ [ 1, 2, 3 ], [ 4, 5, 6 ] ] Got: <BLANKLINE> ********************************************************************** File "gap3.py", line 449, in sage.interfaces.gap3.Gap3.help Failed example: m.Print() #optional  gap3 Expected: [ [ 1, 2, 3 ], [ 4, 5, 6 ] ] Got: "__SAGE_LAST__" ********************************************************************** 1 item had failures: 4 of 10 in sage.interfaces.gap3.Gap3.help [95 tests, 4 failures, 11.90 s]  sage t gap3.py # 4 doctests failed  Total time for all tests: 18.0 seconds cpu time: 4.2 seconds cumulative wall time: 11.9 seconds
Something should be done about them, but it's an independent problem. Hence back to needs review.
comment:13 Changed 5 years ago by
Out of curiosity, I just tried fixing the above problem by adding a self._expect.expect("@i")
at the end of
Gap3.help
. This in case there would be a trivial fix to include in this ticket. This seemed at first to work, but not quite; it triggers another error
sage t gap3.py ********************************************************************** File "gap3.py", line 717, in sage.interfaces.gap3.GAP3Element.__getitem__ Failed example: a[1] #optional  gap3 Expected: 1 Got: 2
I am therefore dropping the case.
comment:14 Changed 5 years ago by
 Status changed from needs_work to positive_review
Ah, right, gap3 is not optional but experimental. I also get the same errors with
./sage tp optional=sage,gap3 src/sage/interfaces/gap3.py
on both develop
and this branch.
comment:15 Changed 5 years ago by
 Branch changed from u/nthiery/fix_missing_synchronisation_upon_starting_gap3 to 529e4d3cc40f80dda2022a5ef4336d0ea8ed35fc
 Resolution set to fixed
 Status changed from positive_review to closed
Is there suppose to be a branch or marked as invalid?