Opened 5 years ago
Last modified 4 years ago
#22626 closed enhancement
Upgrade to GAP 4.10 — at Version 138
Reported by: | nthiery | Owned by: | |
---|---|---|---|
Priority: | critical | Milestone: | sage-8.6 |
Component: | interfaces | Keywords: | days85, libgap |
Cc: | alexk, dimpase, embray, fbissey, arojas, gh-sebasguts, jpflori, markuspf, nthiery, slelievre, vbraun, wstein, gh-timokau | Merged in: | |
Authors: | Nicolas M. Thiéry, Dima Pasechnik, Erik Bray, Jeroen Demeyer | Reviewers: | |
Report Upstream: | N/A | Work issues: | work... |
Branch: | u/dimpase/WIP/libgap410 | Commit: | 406cae67f059056df29413b8fa36ac9964eb3c5b |
Dependencies: | Stopgaps: | error handling in libgap, documentation display |
Description (last modified by )
GAP 4.9 comes with a completely rewritten build system that will simplify our packaging. In particular, libGAP no longer needs to be a separate package.
What the branch does:
- Remove the libgap spkg
- Update the gap spkg to the new build system and build and install libgap
- Replace
gap.shi.patch
by a plain gap startup script for Sage
Rationale: GAP used to provide a startup shell script. The GAP devs are in the process of getting rid of it and provide a very minimal one. They recommend to just write our own rather than patching it.
- Update a few doctests w.r.t. changes of output of some GAP functions
- Possibly controversial: The new libgap currently *does not come*
with symbol rewriting (
foobar
->libGAP_foobar
). This avoids messing around with GAP's sources; in particular opening the door for using a stock GAP from the OS distribution. However there always is a risk of name conflict. And indeed, GAP's constants (actually cpp macros) T_INT, T_FLOAT, ... conflict with Python's constants. This is currently worked around by forcing the inclusion of Python'sstructmember.h
before the gap headers.
Something similar was started by Volker at #19915.
- Removes configure.patch: it was patching configure for better GMP detection under Cygwin (#13954). This should not be needed anymore with the new build system and use of --with-gmp. If it is, upstream asked for it to be reported and they will fix it.
- Revert #19726 (not needed anymore)
Status: currently broken - crashes deep inside GAP error handling system after few simple commands.
Basic tests on libgap:
sage: libgap.eval("GAPInfo.Version") sage: libgap.DihedralGroup(10).CharacterTable() CharacterTable( <pc group of size 10 with 2 generators> ) sage: libgap.Group(libgap.eval("[(1,2,3),(1,2)]")).Size() 6
Running most relevant tests:
sage -tp 8 sage/groups sage/libs/gap
Current status: lots of errors
Testing packages with dynamic loading (e.g. IO):
Install IO:
cd $SAGE/local/gap/latest/pkg wget http://www.gap-system.org/pub/gap/gap4/tar.gz/packages/io-4.4.6.tar.gz tar xvf /tmp/io-4.4.6.tar.gz mv io-4.4.6 io cd io ./configure make
Test it locally:
cd ../.. ./gap -l . gap> LoadPackage("IO"); true
This does not yet work:
sage: libgap.LoadPackage("IO") ValueError: libGAP: Error, module '/opt/sage-git/local/gap/latest/pkg/io/bin/x86_64-pc-linux-gnu-gcc-default64/io.so' not found
This should be fixed once GAP's gap binary is built on top of libgap. See: https://github.com/markuspf/gap/issues/1.
Note:
- Max Horn reviewed the list of GAP symbols we use in Sage; some have already changed in 4.9. See this pad for notes.
Tarball: https://www.gap-system.org/pub/gap/gap-4.10/tar.bz2/gap-4.10.0.tar.bz2
Change History (139)
comment:1 Changed 5 years ago by
- Description modified (diff)
comment:2 Changed 5 years ago by
- Description modified (diff)
comment:3 Changed 5 years ago by
- Description modified (diff)
comment:4 Changed 5 years ago by
- Description modified (diff)
comment:5 Changed 5 years ago by
- Description modified (diff)
comment:6 Changed 5 years ago by
- Branch set to u/nthiery/upgrade_to_gap_4_9
comment:7 Changed 5 years ago by
- Commit set to 3011ac0908d667c0f245ca21859e336511106b5f
comment:8 Changed 5 years ago by
- Description modified (diff)
comment:9 Changed 5 years ago by
- Description modified (diff)
comment:10 Changed 5 years ago by
- Description modified (diff)
comment:11 Changed 5 years ago by
- Description modified (diff)
comment:12 Changed 5 years ago by
- Description modified (diff)
comment:13 Changed 5 years ago by
- Cc vbraun dimpase wstein added
- Keywords days85 libgap added
- Work issues set to Wait for gap 4.9 release
comment:14 Changed 5 years ago by
- Description modified (diff)
comment:15 Changed 5 years ago by
- Cc fbissey added
comment:16 Changed 5 years ago by
- Commit changed from 3011ac0908d667c0f245ca21859e336511106b5f to 234c54b4b7e0495e343c3ab1b925e2c99f37e391
Branch pushed to git repo; I updated commit sha1. New commits:
234c54b | #22626: revert #19726 as it won't be needed for gap 4.9
|
comment:17 Changed 5 years ago by
- Description modified (diff)
Changed 5 years ago by
comment:18 Changed 5 years ago by
- Description modified (diff)
comment:19 Changed 5 years ago by
- Commit changed from 234c54b4b7e0495e343c3ab1b925e2c99f37e391 to 431845f6da27d20c78b5e7a42b5f47bea866201c
comment:20 Changed 5 years ago by
- Description modified (diff)
comment:21 Changed 5 years ago by
- Commit changed from 431845f6da27d20c78b5e7a42b5f47bea866201c to 7c04025083d61cab671df3302c32f353c4e28313
Branch pushed to git repo; I updated commit sha1. New commits:
7c04025 | 22626: document the work around for the GAP-Python name clash on T_INT, ...
|
comment:22 follow-ups: ↓ 23 ↓ 24 ↓ 40 Changed 5 years ago by
So if I understand the mechanics involved, at the moment you have to configure and install gap. Then clean and configure and install for libgap. So it is still behaving like "two packages" with the same source code. Do we have an idea on when we'll have just have gap linking on libgap which would also solve this particular problem?
comment:23 in reply to: ↑ 22 Changed 5 years ago by
Salut François,
Replying to fbissey:
So if I understand the mechanics involved, at the moment you have to configure and install gap. Then clean and configure and install for libgap. So it is still behaving like "two packages" with the same source code.
Exactly.
Do we have an idea on when we'll have just have gap linking on libgap which would also solve this particular problem?
The GAP developers are well aware of that and planning to implement it. They have not yet done yet just to be more incremental; their new build system is a big PR already :-) I would assume that this will be done before GAP 4.9 which they are planning for a couple months from now.
See: https://github.com/markuspf/gap/issues/2
Btw: feel free to comment / expand on the list there. I'll advertise this issue for additional feedback on the sage-packaging mailing list.
comment:24 in reply to: ↑ 22 ; follow-ups: ↓ 25 ↓ 27 Changed 5 years ago by
Replying to fbissey:
So if I understand the mechanics involved, at the moment you have to configure and install gap. Then clean and configure and install for libgap. So it is still behaving like "two packages" with the same source code.
For this reason, I wonder why you didn't keep the separation GAP + libGAP in Sage too. If it effectively behaves as two packages, it seems more natural to keep it as two packages in Sage too. Imagine for example that we need to patch GAP but not libGAP or conversely, that would be harder with the current approach. That being said, this is mostly bikeshedding. So, if the current setup works well, there might be no reason to change it.
comment:25 in reply to: ↑ 24 ; follow-up: ↓ 26 Changed 5 years ago by
Replying to jdemeyer:
Replying to fbissey:
So if I understand the mechanics involved, at the moment you have to configure and install gap. Then clean and configure and install for libgap. So it is still behaving like "two packages" with the same source code.
For this reason, I wonder why you didn't keep the separation GAP + libGAP in Sage too. If it effectively behaves as two packages, it seems more natural to keep it as two packages in Sage too. Imagine for example that we need to patch GAP but not libGAP or conversely, that would be harder with the current approach. That being said, this is mostly bikeshedding. So, if the current setup works well, there might be no reason to change it.
Two completely separate packages would also mean that gap_packages
should come in two different flavours, etc. I'd rather keep it simple and do not multiply these instances (but rather hope that libGAP
will support all of GAP packages soon).
comment:26 in reply to: ↑ 25 ; follow-up: ↓ 28 Changed 5 years ago by
Replying to dimpase:
Two completely separate packages would also mean that
gap_packages
should come in two different flavours, etc.
So, you are saying that we need to install gap_packages
twice too? Once for GAP and once for libGAP?
comment:27 in reply to: ↑ 24 Changed 5 years ago by
Replying to jdemeyer:
For this reason, I wonder why you didn't keep the separation GAP + libGAP in Sage too.
I wanted to experiment with how far we were from having a proper single package, so as to give early feedback to the gap developers on any sticking points. And indeed, linking gap to libgap is basically the only sticking point. Since this point will be most likely resolved by the time this ticket gets merged in, we might as well shoot directly for the "right thing".
comment:28 in reply to: ↑ 26 ; follow-up: ↓ 29 Changed 5 years ago by
Replying to jdemeyer:
Replying to dimpase:
Two completely separate packages would also mean that
gap_packages
should come in two different flavours, etc.So, you are saying that we need to install
gap_packages
twice too? Once for GAP and once for libGAP?
no, not at all---I mean to say that there exist GAP packages (not part of Sage ATM) that break libGAP now. As some of these packages are very useful, it's important to have this fixed (but not at expense of having 2 separate gap_packages
).
comment:29 in reply to: ↑ 28 Changed 5 years ago by
Replying to dimpase:
As some of these packages are very useful, it's important to have this fixed
YES. In fact that is my main motivation to work on the libgap integration: I badly want to use Semigroups that requires IO! We went through the details with Markus and Max, and the shared aim is indeed to have this resolved with the upgrade to 4.9.
comment:30 Changed 5 years ago by
- Cc jpflori added
comment:31 Changed 5 years ago by
Is there any news here?
comment:32 Changed 5 years ago by
- Cc slelievre added
GAP 4.9.0, a beta release for GAP 4.9, has been released.
This means we can start working on building Sage with it.
comment:33 follow-up: ↓ 35 Changed 5 years ago by
Do you know if they now provide a replacement for libGAP
?
comment:34 Changed 5 years ago by
- Cc alexk markuspf added
- Description modified (diff)
Cc-ing @alexk and @markuspf.
comment:35 in reply to: ↑ 33 Changed 5 years ago by
Replying to dimpase:
Do you know if they now provide a replacement for
libGAP
?
In principle yes: the branch I posted here last April was using their stock alpha version. Now the next step is to revive the branch and adapt it if needed.
I can work on this, but probably not at once. Any volunteer to take over welcome.
Markus: do you foresee any major changes in how GAP's lib is to be built that could affect what I was doing last April?
comment:36 follow-up: ↓ 39 Changed 4 years ago by
I just noticed that in 4.9 smallgrps and transgrps are released under a GPL-compatible license. This is great! We really should make them (i.e. database_gap) standard Sage package, and do away with the extra hurdle of having database_gap optional.
comment:37 follow-up: ↓ 38 Changed 4 years ago by
If you're willing to wait 2 months, this might be a good task for the Cernay workshop https://github.com/OpenDreamKit/OpenDreamKit/issues/251
comment:38 in reply to: ↑ 37 Changed 4 years ago by
Replying to jdemeyer:
If you're willing to wait 2 months, this might be a good task for the Cernay workshop https://github.com/OpenDreamKit/OpenDreamKit/issues/251
Yes indeed; it will be helpful to have GAP people under hand. I may just become impatient before that for some research project of mine, in which case I'll have a head start :-)
comment:39 in reply to: ↑ 36 Changed 4 years ago by
Replying to dimpase:
I just noticed that in 4.9 smallgrps and transgrps are released under a GPL-compatible license. This is great! We really should make them (i.e. database_gap) standard Sage package, and do away with the extra hurdle of having database_gap optional.
Yes! I can't wait for all the simplifications this will bring to us!
comment:40 in reply to: ↑ 22 ; follow-up: ↓ 42 Changed 4 years ago by
Replying to fbissey:
So if I understand the mechanics involved, at the moment you have to configure and install gap. Then clean and configure and install for libgap. So it is still behaving like "two packages".
With the past and future modifications to the Sage build system in mind, I now object more strongly than before to treating two separate packages (GAP + libGAP) as one package.
The reason is that we now try to separate the build and install stages of packages. But that is incompatible with the recipe here of first "build and install GAP" and then "build and install libGAP".
comment:41 Changed 4 years ago by
For an example of two Sage packages with the same sources, see gcc
and gfortran
. IMHO, you should do the same for GAP and libGAP (unless they are merged upstream but my impression is that this has not happened yet).
comment:42 in reply to: ↑ 40 Changed 4 years ago by
Replying to jdemeyer:
Replying to fbissey:
So if I understand the mechanics involved, at the moment you have to configure and install gap. Then clean and configure and install for libgap. So it is still behaving like "two packages".
With the past and future modifications to the Sage build system in mind, I now object more strongly than before to treating two separate packages (GAP + libGAP) as one package.
The reason is that we now try to separate the build and install stages of packages. But that is incompatible with the recipe here of first "build and install GAP" and then "build and install libGAP".
Hopefully we will be getting something more serious at final release time, like with pari where you build libpari and then the gp executable. That would be the ideal scenario where it is really one package. Have to see how far they have gone with 4.9.
comment:43 Changed 4 years ago by
A lot of changes on this ticket (also the changes likely to lead to merge conflicts) are related to the unprefixing of libGAP symbols. To ease development, maybe we should try to do that separately from the upgrade to GAP 4.9.
I was thinking to do something like #19915 but changing only the Sage source code (not libGAP) and undoing the prefixing with macros like #define SomeGapFunction libGAP_SomeGapFunction
. What do you think?
comment:44 follow-up: ↓ 45 Changed 4 years ago by
there is also libgap unprefixing in Python/Cython? that has to be done. Perhaps separating these from C level unprefixing would help making it smoother.
comment:45 in reply to: ↑ 44 ; follow-up: ↓ 46 Changed 4 years ago by
Replying to dimpase:
there is also libgap unprefixing in Python/Cython? that has to be done. Perhaps separating these from C level unprefixing would help making it smoother.
Yes, that is what I meant. Creating a separate ticket for unprefixing only at the Cython level by using macros to translate between the unprefixed names in Cython and the prefixed names in libGAP.
comment:46 in reply to: ↑ 45 ; follow-up: ↓ 47 Changed 4 years ago by
Replying to jdemeyer:
Replying to dimpase:
there is also libgap unprefixing in Python/Cython? that has to be done. Perhaps separating these from C level unprefixing would help making it smoother.
Yes, that is what I meant. Creating a separate ticket for unprefixing only at the Cython level by using macros to translate between the unprefixed names in Cython and the prefixed names in libGAP.
I am not sure I understand. Currently in Cython on can do something like
from sage.libs.gap.libgap import libgap g=libgap.ProjectiveGeneralLinearGroup(3,3)
How will this change under what you propose?
comment:47 in reply to: ↑ 46 Changed 4 years ago by
Replying to dimpase:
from sage.libs.gap.libgap import libgap g=libgap.ProjectiveGeneralLinearGroup(3,3)How will this change under what you propose?
The Python interface won't change at all. I am talking about how sage.libs.gap
calls libGAP.
comment:48 Changed 4 years ago by
As far as I can tell, GAP 4.9 has not been released. Does anybody know how close we are to an actual release?
comment:49 Changed 4 years ago by
I think GAP 4.9.0 is considered a public beta for GAP 4.9.
The list of releases at
still displays GAP 4.8.10 as the latest release.
In the list of past releases there, the GAP 4.8, 4.7, 4.6, 4.5 series start at GAP 4.8.2, 4.7.2, 4.6.2, 4.5.4, so probably GAP 4.x.y with those x and lower y were considered beta.
Still, seeing how 4.9.0 works with Sage would be nice.
comment:50 Changed 4 years ago by
- Dependencies set to #25273
comment:51 follow-up: ↓ 54 Changed 4 years ago by
Where is the GAP source tarball which is supposed to be used here?
comment:52 Changed 4 years ago by
- Branch changed from u/nthiery/upgrade_to_gap_4_9 to u/jdemeyer/upgrade_to_gap_4_9
comment:53 Changed 4 years ago by
- Commit changed from 7c04025083d61cab671df3302c32f353c4e28313 to b7278a120c5db710d1e11297b3dd1411d69d302b
comment:54 in reply to: ↑ 51 Changed 4 years ago by
Replying to jdemeyer:
Where is the GAP source tarball which is supposed to be used here?
comment:55 Changed 4 years ago by
- Description modified (diff)
comment:56 in reply to: ↑ description Changed 4 years ago by
Replying to nthiery:
The branch attached to this ticket updates Sage to run on top of a branch of GAP by Markus Pfeiffer that adds libgap compilation and will be merged soon in the devel version of GAP.
It seems that this branch which "will be merged soon" is still not merged. It would be good to have an idea of whether or not that will happen.
comment:57 Changed 4 years ago by
- Commit changed from b7278a120c5db710d1e11297b3dd1411d69d302b to 40e644a16bafa77e4b93e72c215f53a4afb19cc1
comment:58 Changed 4 years ago by
According to https://github.com/gap-system/gap/pull/1205#issuecomment-385670578, Markus's libgap branch won't be merged in GAP 4.9: it's not yet ready enough. Bummer.
We are discussing with Sebastian and Thomas what we can do to move forward here. They are very interested as well by libgap for the integration between GAP and Julia.
Tentative steps:
- As a warm up, upgrade our current gap and libgap spkg to 4.8.10
- Upgrade our current gap and libgap spkg to 4.9; with the many changes to the build system, this may get more tricky
- To make the switch to GAP's libgap less urgent, investigate whether we could change our current libgap spkg to not prefix GAP symbols, in order to be able to use GAP's dynamic packages; see #25273
- On the GAP side: move toward more incremental changes. In particular, extract the changesets from Markus branch that are non controversial and ready, and get them merged in GAP (4.9? 4.10?)
comment:59 Changed 4 years ago by
- Cc ghsebasguts added
comment:60 Changed 4 years ago by
- Cc gh-antonio-rojas added
comment:61 Changed 4 years ago by
- Description modified (diff)
- Milestone changed from sage-8.0 to sage-8.3
GAP 4.9.1 was released in May 2018.
This is the first stable release of GAP 4.9 (GAP 4.9.0 was a beta for GAP 4.9).
comment:62 Changed 4 years ago by
- Cc embray gh-sebasguts nthiery added; ghsebasguts removed
comment:63 Changed 4 years ago by
I'm confused - I just built GAP 4.9.1 from the upstream tarball and 'make install' *does* install /usr/lib/libgap.so and libgap's headers. Is this a different libgap than the one mentioned here to be included in 4.10?
comment:64 Changed 4 years ago by
One comment at pull request 1205 in GAP's github repo said GAP won't ship libGAP before GAP 4.10:
Maybe this was changed though.
comment:65 Changed 4 years ago by
- Description modified (diff)
comment:66 follow-up: ↓ 71 Changed 4 years ago by
Has anybody asked GAP developers for a policy on the prefixing?
In Cernay, I proposed that GAP should prefix some/all of their names like T_INT
-> GAP_T_INT
. For compiling GAP (+ GAP packages) itself, we can use a file with defines like
#define T_INT GAP_T_INT
to ensure backwards compatibility. This way, in GAP itself, one could use either T_INT
or GAP_T_INT
. Outside of GAP, only GAP_T_INT
would work.
I think this is relatively easy to implement (I'm willing to do it) but I don't know what upstream GAP thinks of it.
comment:67 Changed 4 years ago by
- Cc arojas added; gh-antonio-rojas removed
comment:69 Changed 4 years ago by
- Cc gh-timokau added
comment:70 Changed 4 years ago by
Hi,
For the record: I experimented a bit a month ago and a bit more
this morning with Dima with the latest gap master that
has make libgap
and make install-libgap
rules.
See the following branch, which builds libgap from GAP's pristine
sources:
https://git.sagemath.org/sage.git/log/?h=u/nthiery/gap-libgap
Note that, at this stage, Sage won't start with it; maybe even not compile.
comment:71 in reply to: ↑ 66 Changed 4 years ago by
Has anybody asked GAP developers for a policy on the prefixing?
In Cernay, I proposed that GAP should prefix some/all of their names like
T_INT
->GAP_T_INT
. For compiling GAP (+ GAP packages) itself, we can use a file with defines like#define T_INT GAP_T_INTto ensure backwards compatibility. This way, in GAP itself, one could use either
T_INT
orGAP_T_INT
. Outside of GAP, onlyGAP_T_INT
would work.I think this is relatively easy to implement (I'm willing to do it) but I don't know what upstream GAP thinks of it.
That's the idea indeed. Eventually they will provide a standalone header file that will define the official GAP API, and where all symbols will be prefixed. To bootstrap the process, we (well mostly Max!) reviewed and annotated the symbols that are used in Sage:
comment:72 Changed 4 years ago by
Thank you for working on this! Sage using an up-to-date gap and doing away with the additional libgap package will simplify packaging.
comment:73 Changed 4 years ago by
- Milestone changed from sage-8.3 to sage-8.4
update milestone 8.3 -> 8.4
comment:74 Changed 4 years ago by
GAP 4.9.3 was released.
comment:75 Changed 4 years ago by
- Description modified (diff)
- Summary changed from Upgrade to GAP 4.9 to Upgrade to GAP 4.10
- Work issues changed from Wait for gap 4.9 release to Wait for gap 4.10 release
comment:76 Changed 4 years ago by
- Description modified (diff)
comment:77 Changed 4 years ago by
- Description modified (diff)
note that the prefix of the tarball should start with src/
- else one gets weird error messages from the python's tar.
GAPs make install
target now works in a preliminary form - on Linux for sure.
comment:78 Changed 4 years ago by
- Branch changed from u/jdemeyer/upgrade_to_gap_4_9 to u/dimpase/WIP/libgap410
- Commit changed from 40e644a16bafa77e4b93e72c215f53a4afb19cc1 to f06912253d01557727041fe3b4d03e459fb90a11
This is WIP - builds, but libgap.pyx interaction loop has to be rewritten using new API.
Also, sage --gap
is broken
gap: hmm, I cannot find 'lib/init.g' maybe use option '-l <gaproot>'?
New commits:
c766ad6 | Merge branch 'u/jdemeyer/upgrade_to_gap_4_9' of trac.sagemath.org:sage into libgap
|
5152637 | building GAP and libGAP from GAP
|
6bed83a | changes for using libGAP from GAP - part 1
|
f069122 | removing util*
|
comment:79 Changed 4 years ago by
oops, one still needs initialize() from util.pyx, in some form...
comment:80 Changed 4 years ago by
- Commit changed from f06912253d01557727041fe3b4d03e459fb90a11 to 9c22e727f7a87271c26885e9cdc9b52583f598c5
comment:81 Changed 4 years ago by
Great to see the progress! Sounds like a productive week in Siegen. Thanks so much.
comment:82 Changed 4 years ago by
- Description modified (diff)
comment:83 Changed 4 years ago by
- Commit changed from 9c22e727f7a87271c26885e9cdc9b52583f598c5 to 14919d3d32074d36ece1da26815e5f5d7a24812b
Branch pushed to git repo; I updated commit sha1. New commits:
14919d3 | update util and the GAP package
|
comment:84 Changed 4 years ago by
- Commit changed from 14919d3d32074d36ece1da26815e5f5d7a24812b to 8b5ccbe6323feb8ded8cf71cf1d00faa71da59c5
comment:85 Changed 4 years ago by
- Commit changed from 8b5ccbe6323feb8ded8cf71cf1d00faa71da59c5 to a0616deb562afcf9d5b1a8a4e02406b4a0479038
Branch pushed to git repo; I updated commit sha1. New commits:
a0616de | adding --nointeract to libgap startup options
|
comment:86 Changed 4 years ago by
at this point I am at
sage: libgap(1) Error, Variable: '_rich_repr_' must have a value Error, Variable: '_ipython_canary_method_should_not_exist_' must have a\ value [... several more errors like above] gap: panic, could not open *errout*!
comment:87 Changed 4 years ago by
- Commit changed from a0616deb562afcf9d5b1a8a4e02406b4a0479038 to 9cc327f6004c1a16819774fd34cf5fc48ff4a001
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
9cc327f | adding --nointeract to libgap startup options
|
comment:88 follow-up: ↓ 89 Changed 4 years ago by
In the current branch I am able to run
sage: g=libgap.SymmetricGroup(4) sage: print(g.Order()) 24 sage: i=g.Order().sage() sage: i 24
So it's outputting GAP object that is broken ATM.
I'll try fixing the "old" GAP interface, perhaps it will help.
comment:89 in reply to: ↑ 88 ; follow-up: ↓ 90 Changed 4 years ago by
Replying to dimpase:
In the current branch I am able to run
sage: g=libgap.SymmetricGroup(4) sage: print(g.Order()) 24 sage: i=g.Order().sage() sage: i 24
Nice! Making good progress!
So it's outputting GAP object that is broken ATM. I'll try fixing the "old" GAP interface, perhaps it will help.
I am not sure what's broken above?
comment:90 in reply to: ↑ 89 Changed 4 years ago by
Replying to nthiery:
So it's outputting GAP object that is broken ATM. I'll try fixing the "old" GAP interface, perhaps it will help.
I am not sure what's broken above?
see comment:86 - displaying (lib)GAP objects is broken. Also,
sage: gap(1) ** Gap crashed or quit executing 'SetUserPreference("HistoryMaxLines", 30);' ** Restarting Gap and trying again ** Gap crashed or quit executing '\$sage1:=1;;' ** ...
the latter is probably due to some scripts gotten messed up by the upgrade.
And error handling in libgap interface needs work, too.
comment:91 Changed 4 years ago by
libgap's display problem seems to be ipython-specific. Indeed
$ ./sage --python Python 2.7.15 (default, Sep 15 2018, 05:05:36) [GCC 8.2.0] on linux2 Type "help", "copyright", "credits" or "license" for more information. >>> from sage.all import * >>> libgap(1) 1
comment:92 Changed 4 years ago by
Some discussions related to libgap API:
and an accepted PR to fix a bug that was breaking libgap/Sage interface:
comment:93 Changed 4 years ago by
- Commit changed from 9cc327f6004c1a16819774fd34cf5fc48ff4a001 to ece8cfc39e5f4258a5ec2c8dc2a307f988218891
Branch pushed to git repo; I updated commit sha1. New commits:
ece8cfc | fix gap() interface and gap_console, removed some old stuff
|
comment:94 Changed 4 years ago by
- Commit changed from ece8cfc39e5f4258a5ec2c8dc2a307f988218891 to 043fb95c1ee6360ef5df9ac879d643a662bf6dbc
Branch pushed to git repo; I updated commit sha1. New commits:
043fb95 | fixing all but one doctests in interfaces/gap.py
|
comment:95 Changed 4 years ago by
One error - tab completion does not work on GAP objects:
sage -t --long --warn-long 49.3 src/sage/interfaces/gap.py ********************************************************************** File "src/sage/interfaces/gap.py", line 1628, in sage.interfaces.gap.GapElement._tab_completion Failed example: 'Centralizer' in s5._tab_completion() Exception raised: Traceback (most recent call last): File "/mnt/opt/Sage/sage-dev/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 659, in _run self.compile_and_execute(example, compiler, test.globs) File "/mnt/opt/Sage/sage-dev/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 1070, in compile_and_execute exec(compiled, globs) File "<doctest sage.interfaces.gap.GapElement._tab_completion[1]>", line 1, in <module> 'Centralizer' in s5._tab_completion() File "sage/misc/cachefunc.pyx", line 2316, in sage.misc.cachefunc.CachedMethodCallerNoArgs.__call__ (build/cythonized/sage/misc/cachefunc.c:13467) self.cache = f(self._instance) File "/mnt/opt/Sage/sage-dev/local/lib/python2.7/site-packages/sage/interfaces/gap.py", line 1633, in _tab_completion v = P.eval(r'\$SAGE.OperationsAdmittingFirstArgument(%s)'%self.name()) File "/mnt/opt/Sage/sage-dev/local/lib/python2.7/site-packages/sage/interfaces/gap.py", line 583, in eval result = Expect.eval(self, input_line, **kwds) File "/mnt/opt/Sage/sage-dev/local/lib/python2.7/site-packages/sage/interfaces/expect.py", line 1354, in eval for L in code.split('\n') if L != '']) File "/mnt/opt/Sage/sage-dev/local/lib/python2.7/site-packages/sage/interfaces/gap.py", line 789, in _eval_line raise RuntimeError(message) RuntimeError: Gap produced error output Error, no method found! For debugging hints type ?Recovery from NoMethodFound Error, no 1st choice method found for `Iterator' on 1 arguments executing \$SAGE.OperationsAdmittingFirstArgument(x); ********************************************************************** 1 item had failures: 1 of 3 in sage.interfaces.gap.GapElement._tab_completion
comment:96 Changed 4 years ago by
- Stopgaps set to tab completion on GAP objects, error handling in libgap
comment:97 Changed 4 years ago by
- Commit changed from 043fb95c1ee6360ef5df9ac879d643a662bf6dbc to 0128112b501a6c66d21b570dde09526ecf63c9b1
Branch pushed to git repo; I updated commit sha1. New commits:
0128112 | few important Cython types fixed
|
comment:98 Changed 4 years ago by
- Commit changed from 0128112b501a6c66d21b570dde09526ecf63c9b1 to 732cbdd54e24ac35c3fd52c75fb7608ad41603ce
comment:99 Changed 4 years ago by
in gap.*
interface, tab completion is broken due to outdated code in src/ext/gap/sage.g
: with GAP 4.10 one gets
gap> Read("sage.g"); gap> s5:=SymmetricGroup(5); Sym( [ 1 .. 5 ] ) gap> \$SAGE.OperationsAdmittingFirstArgument(s5); Error, no method found! For debugging hints type ?Recovery from NoMethodFound Error, no 1st choice method found for `Iterator' on 1 arguments at /mnt/opt/Sage/sage-dev/local/gap/latest/lib/methsel2.g:250 called from OPERATIONS[i + 1] at sage.g:69 called from <function "LastReadValue">( <arguments> ) called from read-eval loop at *stdin*:4 type 'quit;' to quit to outer loop
in GAP 4.8.6 this works:
gap> Read("sage.g"); gap> s5:=SymmetricGroup(5); Sym( [ 1 .. 5 ] ) gap> \$SAGE.OperationsAdmittingFirstArgument(s5); [ <Operation "ViewObj">, <Operation "ViewString">, <Operation "NameFunction">, <Operation "SetNameFunction">, ...
I've asked here, as this is due to format change of internal GAP data...
comment:100 Changed 4 years ago by
Very similar outdated code is in OperationInspector.operations
in sage/libs/gap/operations.py
.
comment:101 Changed 4 years ago by
- Commit changed from 732cbdd54e24ac35c3fd52c75fb7608ad41603ce to 41218248115922fdae5c4ebb01a03a7df7bafc91
Branch pushed to git repo; I updated commit sha1. New commits:
4121824 | fix for tab completion for GAP interface
|
comment:102 Changed 4 years ago by
- Commit changed from 41218248115922fdae5c4ebb01a03a7df7bafc91 to 1d60fdd386b98b11474df00074039f3b3ef26bca
Branch pushed to git repo; I updated commit sha1. New commits:
1d60fdd | fix tab completion in libgap interface
|
comment:103 Changed 4 years ago by
- Dependencies #25273 deleted
- Stopgaps changed from tab completion on GAP objects, error handling in libgap to error handling in libgap
broken error handling:
sage: a=libgap(1) sage: getattr(a,'_rich_repr_',None) Error, Variable: '_rich_repr_' must have a value sage:
(with a default argument, getattr
should just return it, with raising anything)
comment:104 Changed 4 years ago by
- Stopgaps changed from error handling in libgap to error handling in libgap, documentation display
Please see https://github.com/gap-system/gap/issues/2874 for the current sticking point, which is error handling and processing of error messages. Basically, the question is whether it's possible to retain the behaviour of libGAP without patches to the GAP's kernel.
I'd like someone who knows more about libGAP error handling to have a look at badly butchered by me parts in the branch dealing with it, and suggest what should be done.
Another (easier) thing to fix is GAP and libgap documentation display.
comment:105 Changed 4 years ago by
- Status changed from new to needs_info
comment:106 Changed 4 years ago by
- Commit changed from 1d60fdd386b98b11474df00074039f3b3ef26bca to e352f7e4ad34dfc1422afc04506332ef7674755a
Branch pushed to git repo; I updated commit sha1. New commits:
e352f7e | modest progress - can do libgap(1)
|
comment:107 Changed 4 years ago by
- Status changed from needs_info to needs_work
- Work issues changed from Wait for gap 4.10 release to work...
the interaction with ipython (which tries to get things like _rich_repr_
from GAP to display GAP objects) is far from done. The current problem:
sage: a=libgap(41) sage: a 41 sage: b=libgap(42) --------------------------------------------------------------------------- ValueError Traceback (most recent call last) ... ValueError: libGAP: PrintTo: cannot open stream for output Error, Length: <list> must be a list (not a boolean or fail) ...
this looks like a bug in my new (bad) implementation of GAP objects printing in Sage.
comment:108 Changed 4 years ago by
perhaps we should rather try rebasing libGAP to GAP 4.10, which looks a less daunting task now than a total replacement with "native" GAP's libgap, get it working, and then replace all the libgap_*
functions by the native ones.
comment:109 follow-up: ↓ 128 Changed 4 years ago by
- Commit changed from e352f7e4ad34dfc1422afc04506332ef7674755a to 406cae67f059056df29413b8fa36ac9964eb3c5b
Branch pushed to git repo; I updated commit sha1. New commits:
406cae6 | still the interaction with ipython is broken
|
comment:110 Changed 4 years ago by
On the present branch, I get
sage: a=libgap(1) sage: a *** trouble in PRINT_OR_APPEND_TO_STREAM*** ... *** trouble in PRINT_OR_APPEND_TO_STREAM*** 1
after this GAP is pretty much nuked. This happens during ipython's attempts to pretty-print a
, so that it tries to find _rich_repr_
etc. Specifically, at least on the system I am running this, the trouble starts not immediately, but where _rich_png_
(or the previous thing in the loop) is checked for by get_real_method
in IPython/utils/dir2.py
.
If I run the same from ./sage --python
(after loading Sage there as usual) at least this much (and more) works.
I don't have a good idea how to debug this, so I'd like to pass the token now.
comment:111 Changed 4 years ago by
- Status changed from needs_work to needs_review
oops, wrong ticket
comment:112 Changed 4 years ago by
- Status changed from needs_review to needs_work
comment:113 Changed 4 years ago by
comment:114 Changed 4 years ago by
Okay, I am again confused as to what upstream tarball to use, because the one in the ticket description no longer matches the checksum.
comment:115 Changed 4 years ago by
I just checked that md5=017738e0ba9e166a19084d68123d3e1f as in GAP package checksum matches the md5 of https://github.com/dimpase/gap/releases/download/dev/gap-4.dev.tar.gz
Perhaps your download just got corrupted?
comment:116 follow-up: ↓ 117 Changed 4 years ago by
GAP 4.9.3 was uploaded to Debian unstable yesterday, the sagemath Debian package is broken until we can use GAP 4.9. Is the patch here already enough to use libgap from GAP 4.9 directly? It does not apply cleanly to sage 8.4. Or would you say updating the separate libgap to 4.9 is a better idea?
comment:117 in reply to: ↑ 116 Changed 4 years ago by
Replying to thansen:
GAP 4.9.3 was uploaded to Debian unstable yesterday, the sagemath Debian package is broken until we can use GAP 4.9. Is the patch here already enough to use libgap from GAP 4.9 directly? It does not apply cleanly to sage 8.4.
the patch here is very far from a working one.
Or would you say updating the separate libgap to 4.9 is a better idea?
If you could manage this, it would be great and very useful, but it's a very nontrivial amount of work, due to substantial changes in GAP internals that happened in GAP 4.9.
comment:118 follow-up: ↓ 119 Changed 4 years ago by
I see. If there was progress on this from you guys it would be very much appreciated. The freeze period for the next Debian release starts in January. If we can't fix the problem in time, sage will not be in the next Debian release, which will be the stable release for two years.
comment:119 in reply to: ↑ 118 Changed 4 years ago by
Replying to thansen:
The freeze period for the next Debian release starts in January.
For those of us who are not Debian experts, where is this documented? And what kind of changes are still allowed during the "freeze" period? For example, suppose for the sake of argument that this SageMath ticket is only merged in February, could it still be added as a patch to the Debian SageMath package?
comment:120 Changed 4 years ago by
It is documented here: https://release.debian.org/buster/freeze_policy.html
The broken sagemath will eventually be automatically removed from Debian testing, which will become the next Debian release. After February 12 it cannot migrate back to testing even if it is fixed. The earlier transition freeze on January 12 means that no more library updates with SONAME change can be done (which might be relevant for a sage update too).
comment:121 follow-up: ↓ 123 Changed 4 years ago by
About the example you mentioned: This ticket does not need to be merged, we just need a usable patch. It seems that the gap maintainer is open to providing libgap.la, see https://bugs.debian.org/912862 . It is not yet clear if gap 4.10 is released in time to be included before the freeze.
comment:122 Changed 4 years ago by
- Milestone changed from sage-8.4 to sage-8.5
This ticket mentions GAP 4.10, but should we not be focusing instead on GAP 4.9? Or would getting Sage working against GAP 4.10 imply support for 4.9 as well? I would want to check against both, and starting with the existing release (4.9) seems simpler...
Is there something at the system integration level I could be able to help with? ISTM this needs to be a high priority.
comment:123 in reply to: ↑ 121 Changed 4 years ago by
Replying to thansen:
About the example you mentioned: This ticket does not need to be merged, we just need a usable patch. It seems that the gap maintainer is open to providing libgap.la, see https://bugs.debian.org/912862 . It is not yet clear if gap 4.10 is released in time to be included before the freeze.
If that's the case, for the sake of integration, it might make more sense to pin GAP in Debia to 4.9 and focus on getting a Sage out that works against GAP 4.9 instead of focusing on 4.10. Given the amount of time left to do this, waiting on an uncertain GAP 4.10 package(s) landing in Debian seems a bit risky...
comment:124 Changed 4 years ago by
- Description modified (diff)
- Priority changed from major to critical
comment:125 Changed 4 years ago by
I think getting Sage's libGAP
work with GAP 4.9 and/or 4.10 is much harder than in used to be in 4.8.x, due to major changes to GAP internals (although I have not tried it yet).
comment:126 follow-up: ↓ 132 Changed 4 years ago by
Posting on behalf of Alexander Konovalov who does not yet have a trac account:
has now GAP 4.10.0. The website will be updated in due course, but that means that GAP 4.10 is out.
That's excellent news. Now it's up to Sage to catch up.
comment:127 Changed 4 years ago by
Great! From what I saw yesterday it seemed like there were some doubts about how soon 4.10 would be released. But this being the case I rescind my previous comment.
comment:128 in reply to: ↑ 109 ; follow-up: ↓ 129 Changed 4 years ago by
Replying to git:
Branch pushed to git repo; I updated commit sha1. New commits:
406cae6 still the interaction with ipython is broken
If I'm to understand correctly, you just added this patch for debugging purposes, right? It's not intended to go into the final product is it? I'm going to look into this problem.
comment:129 in reply to: ↑ 128 Changed 4 years ago by
Replying to embray:
Replying to git:
Branch pushed to git repo; I updated commit sha1. New commits:
406cae6 still the interaction with ipython is broken
If I'm to understand correctly, you just added this patch for debugging purposes, right? It's not intended to go into the final product is it? I'm going to look into this problem.
That's right, it's for debugging purposes (although it merely shows that something is going wrong, not more :-)). You might like to coordinate this with Jeroen, as he's going to work on this branch too...
comment:130 follow-up: ↓ 136 Changed 4 years ago by
I'm updating it right now to the 4.10 release. Jeroen is probably a better person to work on the LibGAP integration just because he has more experience with it. But if there any specific segfaults or anything I could look at I will. I'll take a look, for example, at the tab-completion issue...
comment:131 Changed 4 years ago by
I'm also rebasing this branch on current develop, which needs a bit of work...
comment:132 in reply to: ↑ 126 Changed 4 years ago by
Replying to nthiery:
Posting on behalf of Alexander Konovalov who does not yet have a trac account:
Alex should be aware that he can post on trac using his github account!
comment:133 Changed 4 years ago by
Another thing that it seems still hasn't been done is to update the spkg-install for GAP to use DESTDIR-based installation. I'll just work on that simultaneously with this. If we have to upgrade GAP anyways that should be part of the process (and may expose some flaws in GAP's new installation process...)
comment:134 Changed 4 years ago by
Needed to modify build/make/deps
to directly make GAP a build dependency of sagelib as well, which is now needed (and remove the obsolete reference to the libgap spkg).
comment:135 Changed 4 years ago by
- Description modified (diff)
comment:136 in reply to: ↑ 130 Changed 4 years ago by
Replying to embray:
I'm updating it right now to the 4.10 release.
Are you actually doing that? I haven't seen any update on the branch here.
comment:137 Changed 4 years ago by
comment:138 Changed 4 years ago by
- Description modified (diff)
Branch pushed to git repo; I updated commit sha1. New commits:
#22626: remove GAP's symbol prefixing in libgap: libGAP_Foo -> Foo
#22626: doctest update w.r.t. minor changes of output in GAP
#22626: GMP detection patch for cygwin should not be needed anymore
#22626: Remove libgap spkg
#22626: replace patch for GAP's startup script template in favor of a custom script
#22626: remove GAP's symbol prefixing in libgap: libGAP_Foo -> Foo, and workaround GAP <-> Python symbol conflict
#22626: updated gap spkg w.r.t. GAP's devel version and its new build system; also include compilation and installation of libgap
Merge branch 'develop' into t/22626/upgrade_to_gap_4_9