Opened 5 years ago

Last modified 3 years ago

#22626 closed enhancement

Upgrade to GAP 4.9 — at Version 18

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, ... Reviewers:
Report Upstream: N/A Work issues: Wait for gap 4.9 release
Branch: u/nthiery/upgrade_to_gap_4_9 (Commits, GitHub, GitLab) Commit: 234c54b4b7e0495e343c3ab1b925e2c99f37e391
Dependencies: Stopgaps:

Status badges

Description (last modified by nthiery)

GAP 4.9 will come with a completely rewritten build system that will simplify our packaging. In fact, it may well enable Sage to use a vanilla GAP installation as provided by the distribution. It will also enable building GAP as a library, so that we can get rid of our separate libgap package.

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.

See https://github.com/markuspf/gap/issues/2 for the few sticking points that could prevent using a vanilla GAP from the distribution (please edit further if you think about more of them).

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 (Foo -> libGAP_Foo). 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 worked around by duplicating and forcing their value in gap_include.pxd. Another conflict in objects.h currently needs to be worked around by hand (see below). Something similar was started 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: Most long test pass. Tentatively, the 38 remaining failing tests are due to changes in GAP since 4.8.6: Max mentioned that the library has been cleaned up to always use the same random generation source, and some of the group algorithms were changed as well, which can explain, e.g. change of orders in lists of elements. So those should be nothing to worry about. There is not much point in updating those doctests right away; we may as well wait for a more final version of 4.9 to be out.

TODO:

  • Automatic handling of headers (see below for how to do it by hand). GAP's build system will eventuall provide a rule to install headers which will make this trivial.
  • Update the documentation in sage.libs.gap.libgap.pyx to not mention the libgap_ prefix
  • Check against #19915 to see if any of the changes there should be ported here. Then close as won't fix.
  • Update doctests as needed
  • ???

Fetching Markus's GAP sources:

    git clone git@github.com:markuspf/gap.git $LIBGAP
    cd $LIBGAP
    git remote add markuspf git@github.com:markuspf/gap.git
    git fetch markuspf
    git checkout -b markuspf/hpc-merge-libgap
    ./autogen.sh
    ./configure
    make bootstrap-pkg-minimal

Testing libgap:

    ./configure --enable-libgap
    make -j4 libgap
    make test-libgap

Build and install a tardist for Sage, and rebuild the spkg:

    make distclean
    ./autogen.sh
    ./configure
    make manuals
    make clean

    (cd ..; tar zcvf $SAGE/upstream/$GAP.tar.gz --exclude .git $GAP)

    sage --package fix-checksum
    sage -f gap                      # -s

Header files:

  • Copy GAP's header files, as well as gen/config.h to $SAGE/local/include
  • Fix them to adapt the include path: #include <src/...> -> #include <gap/...>
  • Replace T_INT by 0 in TNUM_OBJ, around line 414 of objects.h

Run:

    sage -b

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: all tests pass!

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.

Change History (19)

comment:1 Changed 5 years ago by nthiery

  • Description modified (diff)

comment:2 Changed 5 years ago by nthiery

  • Description modified (diff)

comment:3 Changed 5 years ago by nthiery

  • Description modified (diff)

comment:4 Changed 5 years ago by nthiery

  • Description modified (diff)

comment:5 Changed 5 years ago by nthiery

  • Description modified (diff)

comment:6 Changed 5 years ago by nthiery

  • Branch set to u/nthiery/upgrade_to_gap_4_9

comment:7 Changed 5 years ago by git

  • Commit set to 3011ac0908d667c0f245ca21859e336511106b5f

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

1c645e0#22626: remove GAP's symbol prefixing in libgap: libGAP_Foo -> Foo
1ecc67b#22626: doctest update w.r.t. minor changes of output in GAP
e8ebbca#22626: GMP detection patch for cygwin should not be needed anymore
fd65b06#22626: Remove libgap spkg
9511733#22626: replace patch for GAP's startup script template in favor of a custom script
550625e#22626: remove GAP's symbol prefixing in libgap: libGAP_Foo -> Foo, and workaround GAP <-> Python symbol conflict
a9c859c#22626: updated gap spkg w.r.t. GAP's devel version and its new build system; also include compilation and installation of libgap
3011ac0Merge branch 'develop' into t/22626/upgrade_to_gap_4_9

comment:8 Changed 5 years ago by nthiery

  • Description modified (diff)

comment:9 Changed 5 years ago by nthiery

  • Description modified (diff)

comment:10 Changed 5 years ago by nthiery

  • Description modified (diff)

comment:11 Changed 5 years ago by nthiery

  • Description modified (diff)

comment:12 Changed 5 years ago by nthiery

  • Description modified (diff)

comment:13 Changed 5 years ago by nthiery

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

  • Description modified (diff)

comment:15 Changed 5 years ago by fbissey

  • Cc fbissey added

comment:16 Changed 5 years ago by git

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

  • Description modified (diff)

Changed 5 years ago by nthiery

comment:18 Changed 5 years ago by nthiery

  • Description modified (diff)
Note: See TracTickets for help on using tickets.