search

sagemath / sage

Main repository of SageMath
https://www.sagemath.org
Other
1.38k stars 469 forks source link

Upgrade to gap_packages 4.10 and remove database_gap #26856

Closed dimpase closed 5 years ago

dimpase commented 5 years ago

After the upgrade to GAP 4.10 in #22626, we revisit what GAP packages are available in Sage via the GAP standard SPKG and via extra optional SPGKs.

All GAP packages formerly in our "database_gap" optional SPKG, except tomlib, now get installed with GAP as part of our "gap" standard SPKG.

This ticket adds tomlib to the "gap" standard SPKG and removes the "database_gap" optional SPKG.

(All packages previously in "database_gap" have GPL-compatible licenses and can therefore be distributed as part of the "gap" standard SPKG without license worries.)

We also update the list of extra GAP packages in gap_packages, which remains a separate and optional SPKG.

Once the present ticket is merged, GAP packages shipped with the "gap" standard SPKG and with the "gap_packages" optional SPKG are as follows.

'''GAP packages now shipped with "gap" (standard SageMath package)'''

A basic package used by most GAP packages for their documentation

Five database packages formerly in "database_gap"

Twelve more packages

'''GAP packages now shipped with "gap_packages" (optional SageMath package)'''

Twenty-five "pure GAP" packages (not requiring compilation)

Four compiled GAP packages

No longer distributed

Depends on #22626 Depends on #26889 Depends on #26965

CC: @alex-konovalov @antonio-rojas @dimpase @embray @kiwifb @mantepse @miguelmarco @slel @tscrim @vbraun

Component: packages: optional

Keywords: GAP

Author: Dima Pasechnik, Erik Bray

Branch: 8bf1044

Reviewer: Erik Bray, Dima Pasechnik

Issue created by migration from https://trac.sagemath.org/ticket/26856

dimpase commented 5 years ago

Commit: 1ed2d00

dimpase commented 5 years ago

Author: Dima Pasechnik

dimpase commented 5 years ago

Last 10 new commits:

8b53d4fthis file is no longer needed since I'm patching GAP with this functionality instead
94f09a6speed up this code up a little bit
f571399use ViewObj instead of ViewString
b106ff9use more GAP_Enter() and GAP_Leave() where it seems appropriate
acc75fefix some tests whose output changed since
e588e7afix needed due to changes in record keys
e11e438minor test fixes and cleanup
b686acfuse changesignal to restore SIGCHLD and SIGINT handling
09d229fremove this debug function for now
1ed2d00fix doctests changed due to GAP formatting etc changes
dimpase commented 5 years ago

Branch: u/dimpase/GQ

dimpase commented 5 years ago

Dependencies: #22626

embray commented 5 years ago
comment:3

Is there a particular reason to merge database_gap into gap_packages?

dimpase commented 5 years ago
comment:4

historically, there were several reasons to have separate gap, database_gap and gap_packages, and do not install all the possible GAP packages

the 1st one, if we decide not to re-package original GAP tarball (and I am very much for not re-packaging), would be gone.

the 2nd one is (completely, from the point of view of GAP's LICENCE file) fixed since GAP 4.10 (or even 4.9).

the 3rd one is fixed by #22626, although this has to be verified.

embray commented 5 years ago
comment:5

Replying to @dimpase:

the 3rd one is fixed by #22626, although this has to be verified.

Last I checked this is broken actually, but it's mostly a problem with my install script I think. I need to tinker with that a bit more.

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 5 years ago

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

5d3fce1fixed noisy "#I MakeReadWriteGlobal" things
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 5 years ago

Changed commit from 1ed2d00 to 5d3fce1

dimpase commented 5 years ago
comment:7

Note that most of database_gap is already in our new gap (4.10) package, with exception of tomlib.

I'll add tomlib to gap 4.10, and get rid of database_gap.

dimpase commented 5 years ago
comment:8

I am checking whether database_gap is already in the branch on #22626, running

./sage -tp  --optional=dochtml,memlimit,mpir,python2,database_gap,sage src/sage/groups/perm_gps/permgroup.py

(that is, adding database_gap to the optional list) and get the following weird error:

Using --optional=database_gap,dochtml,memlimit,mpir,python2,sage
Doctesting 1 file using 4 threads.
sage -t src/sage/groups/perm_gps/permgroup.py
**********************************************************************
File "src/sage/groups/perm_gps/permgroup.py", line 733, in sage.groups.perm_gps.permgroup.PermutationGroup_generic._coerce_map_from_
Failed example:
    p = f(mg); p
Exception raised:
    Traceback (most recent call last):
      File "/home/dimpase/Sage/sagetrac-mirror/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 671, in _run
        self.compile_and_execute(example, compiler, test.globs)
      File "/home/dimpase/Sage/sagetrac-mirror/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 1086, in compile_and_execute
        exec(compiled, globs)
      File "<doctest sage.groups.perm_gps.permgroup.PermutationGroup_generic._coerce_map_from_[19]>", line 1, in <module>
        p = f(mg); p
    TypeError: 'NoneType' object is not callable
**********************************************************************
File "src/sage/groups/perm_gps/permgroup.py", line 735, in sage.groups.perm_gps.permgroup.PermutationGroup_generic._coerce_map_from_
Failed example:
    PG(p._gap_()) == p
Exception raised:
    Traceback (most recent call last):
      File "/home/dimpase/Sage/sagetrac-mirror/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 671, in _run
        self.compile_and_execute(example, compiler, test.globs)
      File "/home/dimpase/Sage/sagetrac-mirror/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 1086, in compile_and_execute
        exec(compiled, globs)
      File "<doctest sage.groups.perm_gps.permgroup.PermutationGroup_generic._coerce_map_from_[20]>", line 1, in <module>
        PG(p._gap_()) == p
    NameError: name 'p' is not defined
**********************************************************************
1 item had failures:
   2 of  27 in sage.groups.perm_gps.permgroup.PermutationGroup_generic._coerce_map_from_
    [862 tests, 2 failures, 18.36 s]
----------------------------------------------------------------------
sage -t src/sage/groups/perm_gps/permgroup.py  # 2 doctests failed
----------------------------------------------------------------------
Total time for all tests: 18.6 seconds

Without database_gap in optional, everything passes. The trouble is that the failing tests are not tagged as optional, i.e. it should not matter whether database_gap is passed or not.

Needless to say, these tests work at the Sage prompt just fine.

My hunch is that it might be related to the recent #25706, which unfortunately mixes libgap and gap. (Even if this were true, it would not take away the general flakiness of Sage's doctesting framework...)

dimpase commented 5 years ago

Changed commit from 5d3fce1 to none

dimpase commented 5 years ago

Changed branch from u/dimpase/GQ to none

jdemeyer commented 5 years ago
comment:9

Replying to @dimpase:

it would not take away the general flakiness of Sage's doctesting framework...

The "doctest framework" is not flaky. Code in Sage or its tests is flaky.

dimpase commented 5 years ago
comment:10

One way or another, I have no idea where to even start looking, and what kind of side effects might cause this, and whether it's side-effects in Sage alone, or in Sage interacting with the doctest framework---in the latter case this means it's not possible to reproduce on "standalone" Sage.

dimpase commented 5 years ago

Commit: ef2230a

dimpase commented 5 years ago
comment:11

The issue in comment 8 disappears (I dare not call this a fix, cause the only proof I have is that tests pass!) with the latest commit on this branch, which reduces insanity in src/sage/groups/matrix_gps/finitely_generated.py, see #26889, where it should be posted as an independent thing.

At least I can say that my hunch seems to have been right...

Last 10 new commits:

60d7eb6Allow Gap.__getattr__ to return GAP objects other than just functions
0cf2047updated GAP_Enter patch that does not require gapstate.h in libgap-api.h
2de776dMerge branch 'u/embray/spkgs/gap-410' of trac.sagemath.org:sage into HEAD
eefc0fcfix warning from cython
9292d47add sig_on/off() around GAP_Initialize, just in case unhandled segfaults or the like occur
132a7c3copy the current environment before passing it to GAP
29d4c84Merge branch 'u/embray/spkgs/gap-410' of trac.sagemath.org:sage into HEAD
d7c85cdWhen running doctests, use the current_randstate()'s seed for Interface.rand_seed()
e6a7397Merge branch 'u/embray/doctests/interfaces-rand-seed' of trac.sagemath.org:sage into HEAD
ef2230amore libGAP in as_permutation(), less pexpect mess
dimpase commented 5 years ago

Branch: u/dimpase/GQ

jdemeyer commented 5 years ago
comment:12

Replying to @dimpase:

One way or another, I have no idea where to even start looking, and what kind of side effects might cause this

In many cases, the mysterious failure of doctests is due to some changing interaction with the Python garbage collector. For example, adding an extra test might change the time when the garbage collector kicks in, causing these strange failures. This is especially true when weak references are involved. And the failing test involves the coercion model, which is using weak references.

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 5 years ago

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

9266fedadjusting to the new bigger GAP database, better messages
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 5 years ago

Changed commit from ef2230a to 9266fed

dimpase commented 5 years ago
comment:14

All that is needed to have the doctests pass with database_gap in --optional is the latest commit.

We still need to make sure that tables of marks (GAP 4.8 tomlib package) are there, and then remove #optional : database_gap tags and database_gap package.

dimpase commented 5 years ago
comment:15

OK, I'll make our default GAP, without gap_packages installed, to be the same as vanilla GAP 4.10, which is, package-wise, as follows:

$ ./gap
 ┌───────┐   GAP 4.10.0 of 01-Nov-2018
 │  GAP  │   https://www.gap-system.org
 └───────┘   Architecture: x86_64-pc-linux-gnu-default64
 Configuration:  gmp 6.1.2, readline
 Loading the library and packages ...
 Packages:   AClib 1.3.1, Alnuth 3.1.0, AtlasRep 1.5.1, AutoDoc 2018.09.20, AutPGrp 1.10, CRISP 1.4.4, Cryst 4.1.18, CrystCat 1.1.8, CTblLib 1.2.2, 
             FactInt 1.6.2, FGA 1.4.0, GAPDoc 1.6.2, IRREDSOL 1.4, LAGUNA 3.9.0, Polenta 1.3.8, Polycyclic 2.14, PrimGrp 3.3.2, RadiRoot 2.8, 
             ResClasses 4.7.1, SmallGrp 1.3, Sophus 1.24, SpinSym 1.5, TomLib 1.2.7, TransGrp 2.0.4, utils 0.59

This covers everything that is in database_gap, and more - so we'll move some things out of gap_packages into the main gap package.

dimpase commented 5 years ago
comment:16

I thought that we should remove build/pkgs/gap/patches/nodefaultpackages.patch, but this breaks pexpect interface to GAP. Note that database_gap and gap_packages do not not attempt to change the GAP package autoloading settings, so we should leave them as they are (it does not seem to be easy to fix).

It does not really matter. We can modify stripts starting gap_console() to enable autoloading (or not)...

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 5 years ago

Changed commit from 9266fed to 01665fa

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 5 years ago

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

01665farm #optional - database_gap tags, doc updates, few GAP->libGAP changes
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 5 years ago

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

a0915b5another doctest with different format
2908fc4trimming backspaces, in part due to previous mass edit
9a81e52removing last traces of database_gap
90f2fearemaining places dealing with database_gap
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 5 years ago

Changed commit from 01665fa to 90f2fea

dimpase commented 5 years ago
comment:19

done getting rid of database_gap

Now on to gap_packages...

dimpase commented 5 years ago

Changed dependencies from #22626 to #22626, #26889

dimpase commented 5 years ago
comment:21

opened #26901 to deal with changes in the only (apart from gap_packages) package of Sage dependent on database_gap.

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 5 years ago

Changed commit from 90f2fea to 9407fba

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 5 years ago

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

9407fbareviewer's example and fix
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 5 years ago

Changed commit from 9407fba to ca7f6ff

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 5 years ago

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

68a50fcMerge branch 'u/embray/doctests/interfaces-rand-seed' of trac.sagemath.org:sage into g410
38e687binstall only those packages that are required by GAP to run
8b0d0adImplement GAPElement_List.__bool__ to work like a Python list
6a5d34afix the test for available operations on an object
72df280clean up gap_root() a bit more; remove obsolete warning message
4e6eb3eMerge branch 'u/embray/spkgs/gap-410' of trac.sagemath.org:sage into g410
ca7f6ffMerge branch 'u/dimpase/GQ' of trac.sagemath.org:sage into g410
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 5 years ago

Changed commit from ca7f6ff to b63db23

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 5 years ago

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

b63db23allow GAP default packages and install them
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 5 years ago

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

d305273install atlasrep, as it is a dependency of tomlib
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 5 years ago

Changed commit from b63db23 to d305273

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 5 years ago

Changed commit from d305273 to 1b71099

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 5 years ago

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

3a7b9f7less libgap.eval usage
1b71099adjust doctests to accommodate more GAP methods
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 5 years ago

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

5daf576get rid of explicit GAP package list
d879657attempt to replicate GAP packages from 4.8.6
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 5 years ago

Changed commit from 1b71099 to d879657

dimpase commented 5 years ago
comment:28

The latest bit does not work, as I don't know how to build GAP packages in the current setup. We have built them in place before, so this needs some work.

dimpase commented 5 years ago

Work Issues: work out where/how to build GAP packages

dimpase commented 5 years ago
comment:30

Certainly, we can keep the present arrangement, where binaries of GAP packages are installed in place, into package's bin/ subdirectories.

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 5 years ago

Changed commit from d879657 to 785b9de

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 5 years ago

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

5e61d7bpass -A option to GAP to disable autoloading of default packages for both GAP interfaces
18f2f43instead of disabling autoloading of default packages, just install all default packages as a standard part of the GAP SPKG
da909efmore fixes to the GAP SPKG installation
d1c98f3also include the atlasrep package, which is a dependency of tomlib
785b9deMerge branch 'u/embray/spkgs/gap-410' of trac.sagemath.org:sage into gap410pkg
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 5 years ago

Changed commit from 785b9de to 67ca321