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: sage-8.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:

Status badges

Description

Bug reported by Jean Michel:

┌────────────────────────────────────────────────────────────────────┐
│ SageMath version 8.0.beta9, Release Date: 2017-05-31               │
│ Type "notebook()" for the browser-based 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 nthiery

  • Status changed from new to needs_review

comment:2 Changed 5 years ago by tscrim

Is there suppose to be a branch or marked as invalid?

comment:3 Changed 5 years ago by tscrim

Okay, it seems the commit got mixed in with #23132.

comment:4 Changed 5 years ago by nthiery

  • Branch set to u/nthiery/fix_missing_synchronisation_upon_starting_gap3

comment:5 Changed 5 years ago by nthiery

  • Commit set to fda6e26cb2965c8d863062eaacaf6ac288106b96

Oops; thanks for spotting this. Fixed!


New commits:

fda6e2623132: Fix missing synchronisation upon starting gap3 + doc + skip gap3 banner

comment:6 Changed 5 years ago by tscrim

  • Reviewers set to Travis Scrimshaw
  • Status changed from needs_review to positive_review

LGTM.

comment:7 Changed 5 years ago by nthiery

Thanks!

comment:8 Changed 5 years ago by chapoton

  • Status changed from positive_review to needs_work

hey, guys.. doctests do not pass...

comment:9 Changed 5 years ago by git

  • Commit changed from fda6e26cb2965c8d863062eaacaf6ac288106b96 to 50798771d9b30d558c9446699c2374667bf256ef

Branch pushed to git repo; I updated commit sha1. New commits:

507987723142: trivial doctest update

comment:10 Changed 5 years ago by git

  • Commit changed from 50798771d9b30d558c9446699c2374667bf256ef to 529e4d3cc40f80dda2022a5ef4336d0ea8ed35fc

Branch pushed to git repo; I updated commit sha1. New commits:

529e4d323142: another trivial doctest update

comment:11 Changed 5 years ago by nthiery

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.

Last edited 5 years ago by nthiery (previous) (diff)

comment:12 Changed 5 years ago by nthiery

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 2017-06-06-16-24-24-c343198f.
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 nthiery

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 tscrim

  • 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 vbraun

  • Branch changed from u/nthiery/fix_missing_synchronisation_upon_starting_gap3 to 529e4d3cc40f80dda2022a5ef4336d0ea8ed35fc
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.