#26856 closed enhancement (fixed)

update gap_packages and database_gap to GAP 4.10

Reported by: dimpase Owned by:
Priority: critical Milestone: sage-8.6
Component: packages: optional Keywords: GAP
Cc: alexk, arojas, dimpase, embray, fbissey, mantepse, mmarco, slelievre, tscrim, vbraun Merged in:
Authors: Dima Pasechnik, Erik Bray Reviewers: Erik Bray, Dima Pasechnik
Report Upstream: N/A Work issues: work out where/how to build GAP packages
Branch: 8bf1044 (Commits) Commit: 8bf1044efda750d8a767d88aa6a2afda69edadbe
Dependencies: #22626, #26889, #26965 Stopgaps:

Description (last modified by slelievre)

Now everything needed is already in GAP's tarball.

There is also no more copyright-induced hassle of having database_gap separate and optional, although gap_packages still need a look at the respective copyrights (if we wish to make it standard).

Attachments (1)

gap_packages-4.10.0.log (79.4 KB) - added by mantepse 17 months ago.

Download all attachments as: .zip

Change History (138)

comment:1 Changed 18 months ago by dimpase

  • Authors set to Dima Pasechnik
  • Branch set to u/dimpase/GQ
  • Commit set to 1ed2d00b452155e803c79040460edcd55c120320

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

comment:2 Changed 18 months ago by dimpase

  • Dependencies set to #22626

comment:3 Changed 18 months ago by embray

Is there a particular reason to merge database_gap into gap_packages?

comment:4 follow-up: Changed 18 months ago by dimpase

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

  • largish sizes of downloads
  • licensing issues---stuff in database_gap is not GPL-compatible before GAP 4.9 or 4.10
  • non-working with libgap GAP kernel extensions

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.

comment:5 in reply to: ↑ 4 Changed 18 months ago by embray

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.

comment:6 Changed 18 months ago by git

  • Commit changed from 1ed2d00b452155e803c79040460edcd55c120320 to 5d3fce1000f67cfea8be984df8959933c403e83c

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

5d3fce1fixed noisy "#I MakeReadWriteGlobal" things

comment:7 Changed 18 months ago by dimpase

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.

comment:8 follow-ups: Changed 18 months ago by dimpase

  • Branch u/dimpase/GQ deleted
  • Commit 5d3fce1000f67cfea8be984df8959933c403e83c deleted

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...)

Last edited 18 months ago by dimpase (previous) (diff)

comment:9 in reply to: ↑ 8 Changed 18 months ago by jdemeyer

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.

comment:10 follow-up: Changed 18 months ago by dimpase

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.

Last edited 18 months ago by dimpase (previous) (diff)

comment:11 Changed 18 months ago by dimpase

  • Branch set to u/dimpase/GQ
  • Commit set to ef2230a3c98cb6d8b500e5ba6b925199c30a5647
  • Status changed from new to needs_review

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
Last edited 18 months ago by dimpase (previous) (diff)

comment:12 in reply to: ↑ 10 Changed 18 months ago by jdemeyer

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.

comment:13 Changed 18 months ago by git

  • Commit changed from ef2230a3c98cb6d8b500e5ba6b925199c30a5647 to 9266fed2e972f870d0c4ba891993bbfbb13f2100

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

9266fedadjusting to the new bigger GAP database, better messages

comment:14 Changed 18 months ago by dimpase

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.

comment:15 Changed 18 months ago by dimpase

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.

Last edited 18 months ago by dimpase (previous) (diff)

comment:16 Changed 18 months ago by dimpase

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)...

comment:17 Changed 18 months ago by git

  • Commit changed from 9266fed2e972f870d0c4ba891993bbfbb13f2100 to 01665fa57aa56458b2d43765fd17b88944186f23

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

01665farm #optional - database_gap tags, doc updates, few GAP->libGAP changes

comment:18 Changed 18 months ago by git

  • Commit changed from 01665fa57aa56458b2d43765fd17b88944186f23 to 90f2fea00aa8aa088c619ac128faf2e5ec5cecbd

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

comment:19 Changed 18 months ago by dimpase

done getting rid of database_gap

Now on to gap_packages...

comment:20 Changed 18 months ago by dimpase

  • Dependencies changed from #22626 to #22626, #26889

comment:21 Changed 18 months ago by dimpase

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

Last edited 18 months ago by dimpase (previous) (diff)

comment:22 Changed 18 months ago by git

  • Commit changed from 90f2fea00aa8aa088c619ac128faf2e5ec5cecbd to 9407fba12c12b60c77eb47d11046964be2dd2c74

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

9407fbareviewer's example and fix

comment:23 Changed 17 months ago by git

  • Commit changed from 9407fba12c12b60c77eb47d11046964be2dd2c74 to ca7f6ff6bf514e3b640ab39d8f51760eacc75a88

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

comment:24 Changed 17 months ago by git

  • Commit changed from ca7f6ff6bf514e3b640ab39d8f51760eacc75a88 to b63db23cde4ed4800a93caec12edec29d4c24aab

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

b63db23allow GAP default packages and install them

comment:25 Changed 17 months ago by git

  • Commit changed from b63db23cde4ed4800a93caec12edec29d4c24aab to d305273fcdd2c247a0c50b8432e2939bc9c6a0f6

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

d305273install atlasrep, as it is a dependency of tomlib

comment:26 Changed 17 months ago by git

  • Commit changed from d305273fcdd2c247a0c50b8432e2939bc9c6a0f6 to 1b7109983cb030067c6a7e471da076edf8fe6f64

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

3a7b9f7less libgap.eval usage
1b71099adjust doctests to accommodate more GAP methods

comment:27 Changed 17 months ago by git

  • Commit changed from 1b7109983cb030067c6a7e471da076edf8fe6f64 to d879657c584c866152fe795888ee7d02c8668f12

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

comment:28 Changed 17 months ago by dimpase

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.

comment:29 Changed 17 months ago by dimpase

  • Status changed from needs_review to needs_work
  • Work issues set to work out where/how to build GAP packages

comment:30 Changed 17 months ago by dimpase

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

comment:31 Changed 17 months ago by git

  • Commit changed from d879657c584c866152fe795888ee7d02c8668f12 to 785b9dedc352d33a74f8f5d54f6c91c436b225d8

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

comment:32 Changed 17 months ago by git

  • Commit changed from 785b9dedc352d33a74f8f5d54f6c91c436b225d8 to 67ca32109f47388f820d95f4171f01bc050e32c8

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

67ca321fixed gap_packages, also package compiling/installing

comment:33 Changed 17 months ago by git

  • Commit changed from 67ca32109f47388f820d95f4171f01bc050e32c8 to a7ef9e015984e48785b80d95ff422bcd3f6ae232

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

a7ef9e0backport guava patch to allow no gcc

comment:34 Changed 17 months ago by dimpase

It turns out that most of the changes so far here should go to #22626. Once this is done I'll update the branch accordingly.

comment:35 follow-up: Changed 17 months ago by embray

Is there any reason the IO package isn't installed here? It seems to be mentioned quite often, but we don't provide a means to install it.

comment:36 Changed 17 months ago by git

  • Commit changed from a7ef9e015984e48785b80d95ff422bcd3f6ae232 to 01d85a7b1ebe5b95cb8a84d273dafd13c62f4a65

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

53f07ccdisable parallel make for gap
b446ebbadjust doctests to accommodate more GAP methods
01d85a7Merge branch 'u/embray/spkgs/gap-410' of trac.sagemath.org:sage into gap410pkg

comment:37 in reply to: ↑ 35 Changed 17 months ago by dimpase

Replying to embray:

Is there any reason the IO package isn't installed here? It seems to be mentioned quite often, but we don't provide a means to install it.

I'm currently going for the minimal thing: get gap_packages spkg working the way it was. We never had io package there, as it was breaking libgap in a bad way. (there are open tickets about io package).

Does this sound like a good plan to you? We can postpone io package, it's not needed to be ready by the Debian freeze deadline.

comment:38 Changed 17 months ago by dimpase

I need to adjust formatting in a dozen of tests, after that should be ready for review.

comment:39 Changed 17 months ago by embray

OK regarding the IO package. If we weren't installing it in the first place I'm fine just sticking to the minimum set of packages that we were always installing in the first place.

I am also making some updates to this: In particular I'm not happy with the way the compiled packages are installed. Let me know when you're done your changes (set needs_review) and I'll add my changes on top of that.

comment:40 follow-up: Changed 17 months ago by dimpase

I'm getting an unpleasant surprise from the doctest framework here - it mangles long lines in this file, e.g. here:

File "src/sage/algebras/quantum_groups/quantum_group_gap.py", line 757, in sage.algebras.quantum_groups.quantum_group_gap.QuantumGroup.coproduct
Failed example:
    [Q.coproduct(f, 2)._latex_() for f in Q.F_simple()]  # optional - gap_packages
Expected:
    ['1 {(1<x>1<x>F_{\\alpha_{1}})}+-q^{2}+q^{-2} {(1<x>F_{\\alpha_{1}}<x>[ K_{1} ; 1 ])}+1 {(1<x>F_{\\alpha_{1}}<x>K_{1})}+q^{4}-2+q^{-4} {(F_{\\alpha_{1}}<x>[ K_{1} ; 1 ]<x>[ K_{1} ; 1 ])}+-q^{2}+q^{-2} {(F_{\\alpha_{1}}<x>[ K_{1} ; 1 ]<x>K_{1})}+-q^{2}+q^{-2} {(F_{\\alpha_{1}}<x>K_{1}<x>[ K_{1} ; 1 ])}+1 {(F_{\\alpha_{1}}<x>K_{1}<x>K_{1})}',
     '1 {(1<x>1<x>F_{\\alpha_{2}})}+-q+q^{-1} {(1<x>F_{\\alpha_{2}}<x>[ K_{2} ; 1 ])}+1 {(1<x>F_{\\alpha_{2}}<x>K_{2})}+q^{2}-2+q^{-2} {(F_{\\alpha_{2}}<x>[ K_{2} ; 1 ]<x>[ K_{2} ; 1 ])}+-q+q^{-1} {(F_{\\alpha_{2}}<x>[ K_{2} ; 1 ]<x>K_{2})}+-q+q^{-1} {(F_{\\alpha_{2}}<x>K_{2}<x>[ K_{2} ; 1 ])}+1 {(F_{\\alpha_{2}}<x>K_{2}<x>K_{2})}']
Got:
    ['1 {(1<x>1<x>F_{\\alpha_{1}})}+-q^{2}+q^{-2} {(1<x>F_{\\alpha_{1}}<x>[ K_{1} ; 1 ])}+1 {(1<x>F_{\\alpha_{1}}<x>K_{1})}+q^{4}-2+q^{-4} {(F\n1<x>[ K_{1} ; 1 ]<x>[ K_{1} ; 1 ])}+-q^{2}+q^{-2} {(F_{\\alpha_{1}}<x>[ K_{1} ; 1 ]<x>K_{1})}+-q^{2}+q^{-2} {(F\n1<x>K_{1}<x>[ K_{1} ; 1 ])}+1 {(F_{\\alpha_{1}}<x>K_{1}<x>K_{1})}',
     '1 {(1<x>1<x>F_{\\alpha_{2}})}+-q+q^{-1} {(1<x>F_{\\alpha_{2}}<x>[ K_{2} ; 1 ])}+1 {(1<x>F_{\\alpha_{2}}<x>K_{2})}+q^{2}-2+q^{-2} {(F\n4<x>[ K_{2} ; 1 ]<x>[ K_{2} ; 1 ])}+-q+q^{-1} {(F_{\\alpha_{2}}<x>[ K_{2} ; 1 ]<x>K_{2})}+-q+q^{-1} {(F_{\\alpha_{2}}<x>K\n2<x>[ K_{2} ; 1 ])}+1 {(F_{\\alpha_{2}}<x>K_{2}<x>K_{2})}']

and instead of F_{\\alpha_{1}} at few places it outputs F\n1 - probably some unfortunate combination of escape sequences, dunno... (needless to say, the output at the sage prompt is fine).

I'm inclined to mark these as # known bug and deal with them later...

Last edited 17 months ago by dimpase (previous) (diff)

comment:41 Changed 17 months ago by embray

In the case of the "grape" package, ISTM the only actual "compiled" part is not technically part of the package at all, but rather it builds its own copy of nauty. It ought to be able to use the nauty that's already included in Sage.

I probably won't stress about it too much in the short time that we have, but this should probably be fixed later.

comment:42 Changed 17 months ago by dimpase

I agree about nauty/grape - see #26923

comment:43 in reply to: ↑ 40 ; follow-up: Changed 17 months ago by embray

Replying to dimpase:

I'm getting an unpleasant surprise from the doctest framework here - it mangles long lines in this file, e.g. here: and instead of F_{\\alpha_{1}} at few places it outputs F\n1 - probably some unfortunate combination of escape sequences, dunno... (needless to say, the output at the sage prompt is fine).

I'm not so sure about that. I don't think the doctest framework is doing that, but I'll look into it later.

comment:44 in reply to: ↑ 43 Changed 17 months ago by dimpase

Replying to embray:

Replying to dimpase:

I'm getting an unpleasant surprise from the doctest framework here - it mangles long lines in this file, e.g. here: and instead of F_{\\alpha_{1}} at few places it outputs F\n1 - probably some unfortunate combination of escape sequences, dunno... (needless to say, the output at the sage prompt is fine).

I'm not so sure about that. I don't think the doctest framework is doing that, but I'll look into it later.

Might be a "noisy line" thing - but why only while doctesting? Anyhow I have opened #26924 to deal with it.

comment:45 Changed 17 months ago by embray

I'm running the tests for some of these packages. A bunch of tests fail for guava: I'm not sure whether or not this is expected.

comment:46 follow-up: Changed 17 months ago by embray

A new failure I'm getting is:

File "src/sage/graphs/strongly_regular_db.pyx", line 2445, in sage.graphs.strongly_regular_db.SRG_416_100_36_20
Failed example:
    g = SRG_416_100_36_20()                # optional - gap_packages # long time
Expected nothing
Got:
    #I  io package is not available. Check that the name is correct
    #I  and it is present in one of the GAP root directories (see '??RootPaths')
    #I  io package is not available. Check that the name is correct
    #I  and it is present in one of the GAP root directories (see '??RootPaths')
**********************************************************************
1 item had failures:
   1 of   4 in sage.graphs.strongly_regular_db.SRG_416_100_36_20
    [334 tests, 1 failure, 61.16 s]

So perhaps one of these packages at least tries to load the IO package if available, though doesn't strictly need it. In that case we should at least be silencing this message.

comment:47 follow-ups: Changed 17 months ago by embray

Another one that's a bit troubling is:

sage -t --long src/sage/tests/gap_packages.py
**********************************************************************
File "src/sage/tests/gap_packages.py", line 8, in sage.tests.gap_packages
Failed example:
    test_packages(pkgs, only_failures=True)    # optional - gap_packages
Expected:
      Status   Package   GAP Output
    +--------+---------+------------+
Got:
    #I  polymake command not found. Please set POLYMAKE_COMMAND by hand
    #I  HAP warning: Set POLYMAKE_PATH manually if needed.
    #I  HAP warning: Set BROWSER_PATH manually if needed.
    #I  HAP warning: Set ASY_PATH manually if needed.
    #I  You may wish to install the xgap package
    #I  and enjoy the graphic capabilities of SONATA.
    #I  qpa-version package is not available. Check that the name is correct
    #I  and it is present in one of the GAP root directories (see '??RootPaths')
      Status    Package       GAP Output
    +---------+-------------+------------+
      Failure   polenta       fail
      Failure   qpa-version   fail
      Failure   resclasses    fail
**********************************************************************
1 item had failures:
   1 of   5 in sage.tests.gap_packages
    [10 tests, 1 failure, 3.98 s]

I get these warnings when loading the relevant GAP packages in a vanilla GAP source tree, so I suppose they're normal and/or expected, or at least not indicative of any specific bug on our part. But we probably need to at least temporarily silence them for the purposes of the code and tests they affect.

The one exception being the error about "qpa-version". The name of the package is just "qpa", but its source directory is unusually named "qpa-version-<x.y>", so there's some code that's making an invalid assumption about GAP package names :sigh:

Last edited 17 months ago by embray (previous) (diff)

comment:48 in reply to: ↑ 46 Changed 17 months ago by dimpase

Replying to embray:

A new failure I'm getting is:

File "src/sage/graphs/strongly_regular_db.pyx", line 2445, in sage.graphs.strongly_regular_db.SRG_416_100_36_20
Failed example:
    g = SRG_416_100_36_20()                # optional - gap_packages # long time
Expected nothing
Got:
    #I  io package is not available. Check that the name is correct
    #I  and it is present in one of the GAP root directories (see '??RootPaths')
    #I  io package is not available. Check that the name is correct
    #I  and it is present in one of the GAP root directories (see '??RootPaths')
**********************************************************************
1 item had failures:
   1 of   4 in sage.graphs.strongly_regular_db.SRG_416_100_36_20
    [334 tests, 1 failure, 61.16 s]

So perhaps one of these packages at least tries to load the IO package if available, though doesn't strictly need it. In that case we should at least be silencing this message.

Yes, atlasrep does this. However, you might have a stale libgap workspace. I saw this warning, and then went away after I wiped the workspaces out.

comment:49 in reply to: ↑ 47 Changed 17 months ago by dimpase

Replying to embray:

Another one that's a bit troubling is:

sage -t --long src/sage/tests/gap_packages.py
**********************************************************************
File "src/sage/tests/gap_packages.py", line 8, in sage.tests.gap_packages
Failed example:
    test_packages(pkgs, only_failures=True)    # optional - gap_packages
Expected:
      Status   Package   GAP Output
    +--------+---------+------------+
Got:
    #I  polymake command not found. Please set POLYMAKE_COMMAND by hand
    #I  HAP warning: Set POLYMAKE_PATH manually if needed.
    #I  HAP warning: Set BROWSER_PATH manually if needed.
    #I  HAP warning: Set ASY_PATH manually if needed.
    #I  You may wish to install the xgap package
    #I  and enjoy the graphic capabilities of SONATA.
    #I  qpa-version package is not available. Check that the name is correct
    #I  and it is present in one of the GAP root directories (see '??RootPaths')
      Status    Package       GAP Output
    +---------+-------------+------------+
      Failure   polenta       fail
      Failure   qpa-version   fail
      Failure   resclasses    fail
**********************************************************************
1 item had failures:
   1 of   5 in sage.tests.gap_packages
    [10 tests, 1 failure, 3.98 s]

I get these warnings when loading the relevant GAP packages in a vanilla GAP source tree, so I suppose they're normal and/or expected, or at least not indicative of any specific bug on our part. But we probably need to at least temporarily silence them for the purposes of the code and tests they affect.

The one exception being the error about "qpa-version". The name of the package is just "qpa", but its source directory is unusually named "qpa-version-<x.y>", so there's some code that's making an invalid assumption about GAP package names :sigh:

Yes, that's where I am now too. I'll need to step away for an hour, I'll push what I have in a second.

comment:50 Changed 17 months ago by git

  • Commit changed from 01d85a7b1ebe5b95cb8a84d273dafd13c62f4a65 to 78db91a51010662e85dda67649495eaed4193025

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

f31a8cdbackport https://github.com/gap-packages/cohomolo/pull/19
78db91aknown-buggification - see Trac #26924

comment:51 Changed 17 months ago by dimpase

OK, over to you now.

comment:52 Changed 17 months ago by embray

I think this change

  • src/sage/algebras/quantum_groups/quantum_group_gap.py

    diff --git a/src/sage/algebras/quantum_groups/quantum_group_gap.py b/src/sage/algebras/quantum_groups/quantum_group_gap.py
    index 4ce98f4..0e4549b 100644
    a b class QuantumGroupModule(Parent, UniqueRepresentation): 
    15661564            sage: Q = QuantumGroup(['A',2])           # optional - gap_packages
    15671565            sage: V = Q.highest_weight_module([1,1])  # optional - gap_packages
    15681566            sage: V.gap()                             # optional - gap_packages
    1569             <8-dimensional left-module over QuantumUEA( <root system of type A2>,
    1570             Qpar = q )>
     1567            <8-dimensional left-module over QuantumUEA( <root system of type A
     1568            2>, Qpar = q )>
    15711569        """
    15721570        return self._libgap

This one feels like a bug in the GAP and/or the QuaGroup? package to me, but I confirmed that this is in fact the output provided by GAP and not a bug on our end, so for the time being there's nothing to be done (other than possibly report it upstream)

This is one of the problems with GAP objects not having a real string representation that isn't tied to how exactly they're printed to the screen :|

comment:53 Changed 17 months ago by embray

The problem here, accounting for all these test failures including the test above that was modified, is that the QuaGroups package seems (I have not looked for the exact code) to insert linebreaks into the representations of its types in different places depending on the terminal width. Not exactly sure if that's a GAP thing in general or something specific to this package, but this is the first place it's come up. In any case we apparently aren't dealing well with those unnecessary line breaks.

comment:54 Changed 17 months ago by dimpase

linebreaks are inserted by GAP, it was always the case.

comment:55 Changed 17 months ago by embray

I see now, the output splitting is indeed done by default by GAP. Again, this is no good, especially since it can depend on terminal width. I haven't found yet exactly where GAP determines this, but I need to figure out a way to force it to just not do this at all, if possible. That might be something to do as part of #22626 or as a follow-up.

comment:56 Changed 17 months ago by git

  • Commit changed from 78db91a51010662e85dda67649495eaed4193025 to fc197922d0c70666dfaa48efccbedfdeb7be1070

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

fc19792install deps of polenta, qpa, resclasses, fix test

comment:57 in reply to: ↑ 47 Changed 17 months ago by dimpase

Replying to embray:

Another one that's a bit troubling is:

sage -t --long src/sage/tests/gap_packages.py
[.....]
     +---------+-------------+------------+
      Failure   polenta       fail
      Failure   qpa-version   fail
      Failure   resclasses    fail
**********************************************************************
1 item had failures:
   1 of   5 in sage.tests.gap_packages
    [10 tests, 1 failure, 3.98 s]

I get these warnings when loading the relevant GAP packages in a vanilla GAP source tree, so I suppose they're normal and/or expected, or at least not indicative of any specific bug on our part. But we probably need to at least temporarily silence them for the purposes of the code and tests they affect.

The one exception being the error about "qpa-version". The name of the package is just "qpa", but its source directory is unusually named "qpa-version-<x.y>", so there's some code that's making an invalid assumption about GAP package names :sigh:

the latest commit fixes these failures, which are two-fold: missing dependencies and weird -version-part which one should strip.

I should still figure out how to suppress these #I ... lines...

comment:58 Changed 17 months ago by git

  • Commit changed from fc197922d0c70666dfaa48efccbedfdeb7be1070 to 92517b495dc21eb739f9c8fcb39840674bc690f0

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

92517b4suppress "#I.." warnings

comment:59 follow-up: Changed 17 months ago by git

  • Commit changed from 92517b495dc21eb739f9c8fcb39840674bc690f0 to 08ffb3396308b4643cfcbed1c56c0272c39f6a17

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

08ffb33more doctest fixes

comment:60 Changed 17 months ago by dimpase

By the way, I tried installing IO package in the same way as this ticket builds packages needing compilation, but this fails due to a "missing" header (not sure what goes wrong):

[gap_packages-4.10.0] configure: creating ./config.status
[gap_packages-4.10.0] config.status: creating Makefile
[gap_packages-4.10.0] config.status: creating src/pkgconfig.h
[gap_packages-4.10.0] config.status: executing depfiles commands
[gap_packages-4.10.0] config.status: executing libtool commands
[gap_packages-4.10.0] make[2]: Entering directory '/mnt/opt/Sage/sage-dev/local/share/gap/pkg/io-4.5.4'
[gap_packages-4.10.0] /bin/sh ./libtool  --tag=CC   --mode=compile gcc -DHAVE_CONFIG_H -I. -I./src  -I/mnt/opt/Sage/sage-dev/local/var/tmp/sage/build/gap-4.10.0/src/gen -I/mnt/opt/Sage/sage-dev/local/var/tmp/sage/build/gap-4.10.0/src/src -I/mnt/opt/Sage/sage-dev/local/var/tmp/sage/build/gap-4.10.0/src -DHAVE_CONFIG_H -I/mnt/opt/Sage/sage-dev/local/include        -O2 -g   -g -O2 -MT src/io_la-io.lo -MD -MP -MF src/.deps/io_la-io.Tpo -c -o src/io_la-io.lo `test -f 'src/io.c' || echo './'`src/io.c
[gap_packages-4.10.0] libtool: compile:  gcc -DHAVE_CONFIG_H -I. -I./src -I/mnt/opt/Sage/sage-dev/local/var/tmp/sage/build/gap-4.10.0/src/gen -I/mnt/opt/Sage/sage-dev/local/var/tmp/sage/build/gap-4.10.0/src/src -I/mnt/opt/Sage/sage-dev/local/var/tmp/sage/build/gap-4.10.0/src -DHAVE_CONFIG_H -I/mnt/opt/Sage/sage-dev/local/include -O2 -g -g -O2 -MT src/io_la-io.lo -MD -MP -MF src/.deps/io_la-io.Tpo -c src/io.c  -fPIC -DPIC -o src/.libs/io_la-io.o
[gap_packages-4.10.0] src/io.c:14:10: fatal error: src/compiled.h: No such file or directory
[gap_packages-4.10.0]  #include "src/compiled.h"          /* GAP headers                */
[gap_packages-4.10.0]           ^~~~~~~~~~~~~~~~

looks like the corresponding -I arguments all have an extra /src suffix, something that prevents this header to be found.

comment:61 follow-up: Changed 17 months ago by dimpase

..."thanks" to Gatwick drone operators, I can do this work now, and not sit on a plane off to Singapore...

comment:62 Changed 17 months ago by dimpase

GAP's IO is on #26930

comment:63 Changed 17 months ago by git

  • Commit changed from 08ffb3396308b4643cfcbed1c56c0272c39f6a17 to c2e30ae0aff04ce2c88b9e6e9126caac305e85eb

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

c2e30aesilence one more #I message from GAP

comment:64 Changed 17 months ago by dimpase

  • Status changed from needs_work to needs_review

OK, seems to be ready now

comment:65 Changed 17 months ago by embray

  • src/sage/graphs/strongly_regular_db.pyx

    diff --git a/src/sage/graphs/strongly_regular_db.pyx b/src/sage/graphs/strongly_regular_db.pyx
    index 8df9bf5..06a0780 100644
    a b def SRG_416_100_36_20(): 
    24422442    EXAMPLES::
    24432443
    24442444        sage: from sage.graphs.strongly_regular_db import SRG_416_100_36_20
     2445        sage: libgap.eval("SetInfoLevel(InfoWarning,0)") # silence #I warnings
    24452446        sage: g = SRG_416_100_36_20()                # optional - gap_packages # long time
     2447        sage: libgap.eval("SetInfoLevel(InfoWarning,1)") # silence #I warnings
    24462448        sage: g.is_strongly_regular(parameters=True) # optional - gap_packages # long time
    24472449        (416, 100, 36, 20)
    24482450    """

I don't like that this is done in the doctest; it distracts from the actual documentation. I think this should go in the function itself. We've already seen the warning in question, decided it is not important to us, and can go ahead and silence it.

Last edited 17 months ago by embray (previous) (diff)

comment:66 in reply to: ↑ 59 ; follow-ups: Changed 17 months ago by embray

Replying to git:

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

08ffb33more doctest fixes

This change doesn't seem to work for me. Those tests passed before this commit, and fail after. I tried clearing my existing workspaces but it still fails. That's troubling--it may still be some badly managed random state somewhere. Unless I'm missing something else.

comment:67 in reply to: ↑ 61 ; follow-up: Changed 17 months ago by embray

Replying to dimpase:

..."thanks" to Gatwick drone operators, I can do this work now, and not sit on a plane off to Singapore...

Huh?? What happened? :(

comment:68 in reply to: ↑ 67 Changed 17 months ago by dimpase

Replying to embray:

Replying to dimpase:

..."thanks" to Gatwick drone operators, I can do this work now, and not sit on a plane off to Singapore...

Huh?? What happened? :(

Lucky you, you don't read news. :-) Gatwick was shut down for ~36 hours due to some not yet caught folks flying a largish drone over it several times, 700+ flights cancelled, 100s diverted. We were able to book an even cheaper flight, via Berlin, and get a full refund. They are open since this morning, and in fact the airline rebooked us for tonight, but we were in the full panic mode and got the other flight already...

comment:69 in reply to: ↑ 66 Changed 17 months ago by dimpase

Replying to embray:

Replying to git:

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

08ffb33more doctest fixes

This change doesn't seem to work for me. Those tests passed before this commit, and fail after. I tried clearing my existing workspaces but it still fails. That's troubling--it may still be some badly managed random state somewhere. Unless I'm missing something else.

they pass for me, both with and without --long, on Linux and on OSX.

I'd try rm -rf local/share/gap local/include/gap followed by ./sage -f gap gap_packages followed up by make build, and then testing...

(and wiping out ~/.sage/gap/ too)

Last edited 17 months ago by dimpase (previous) (diff)

comment:70 follow-up: Changed 17 months ago by git

  • Commit changed from c2e30ae0aff04ce2c88b9e6e9126caac305e85eb to 1e5f679c41a7b27088ed330f986df91d03099a71

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

1e5f679move silencing into the function body

comment:71 in reply to: ↑ 70 ; follow-up: Changed 17 months ago by dimpase

Replying to git:

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

1e5f679move silencing into the function body

this addresses comment 65.

comment:72 in reply to: ↑ 66 Changed 17 months ago by embray

Replying to embray:

Replying to git:

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

08ffb33more doctest fixes

This change doesn't seem to work for me. Those tests passed before this commit, and fail after. I tried clearing my existing workspaces but it still fails. That's troubling--it may still be some badly managed random state somewhere. Unless I'm missing something else.

Nevermind, that was my fault. Nevertheless, this still won't work: The outcomes of these tests depend on whether or not gap_packages is installed, so if it isn't then these will give different results. I'm not sure what to do: Maybe these tests should be rewritten to be more flexible, or the code that's being tested needs to be rewritten to be less sensitive to what GAP packages are installed (if that's even possible...)

comment:73 Changed 17 months ago by dimpase

Let's just make gap_packages standard, as planned anyway. This will alleviate this problem.

comment:74 follow-up: Changed 17 months ago by embray

I don't think we should do that. It's bad enough that we're adding all the "default" packages, but that does seem defensible.

Otherwise there's no point in even having a separate spkg.

Meanwhile, I don't like that merely having some random package installed can cause actually different (even if not substantively) results, in ways that are very difficult to trace. This is a really annoying problem with GAP...

Last edited 17 months ago by embray (previous) (diff)

comment:75 Changed 17 months ago by embray

Let's see... For the test that's failing in sage.libs.gap.element it should suffice to just put ellipses after ValueError: libGAP: Error, no method found!. That's the only part that's important; the rest are details.

The other two look like they might result from different PRNG states, so it might suffice for those two to explicitly re-seed the PRNG before them; let me try.

comment:76 in reply to: ↑ 74 Changed 17 months ago by dimpase

Replying to embray:

I don't think we should do that. It's bad enough that we're adding all the "default" packages, but that does seem defensible.

Otherwise there's no point in even having a separate spkg.

for transition from several to one package it's OK, I think. We can merge them later on another ticket.

Meanwhile, I don't like that merely having some random package installed can cause actually different (even if not substantively) results, in ways that are very difficult to trace. This is a really annoying problem with GAP...

comment:77 Changed 17 months ago by embray

I'm definitely -1 on making it a standard package, especially in the short time that we have. These have always been an optional add-on, and for the time being I don't want to add any more footprint to the default installation than we already are (keep in mind, this also increases the possibility of bugs in user code for which we don't have test cases).

comment:78 Changed 17 months ago by embray

In the cases of the test we have failing currently, it appears that loading some of these packages just modifies the PRNG state, so the best solution for now is to explicitly re-seed them before the affected tests (not ideal, but it will ensure consistent results).

comment:79 Changed 17 months ago by embray

  • Branch changed from u/dimpase/GQ to public/ticket-26856
  • Commit 1e5f679c41a7b27088ed330f986df91d03099a71 deleted
  • Status changed from needs_review to needs_work

Public branch for this ticket. It is up-to-date with Dima's branch minus the last two commits that are contentious.

comment:80 Changed 17 months ago by dimpase

your branch does not merge cleanly...

comment:81 Changed 17 months ago by dimpase

How about tagging the previously untagged tests that differ if gap_packages are installed with # optional - gap_packages, and be done with it? It's not that many tests, right?

comment:82 Changed 17 months ago by embray

It might need to be rebased again already; I don't know what Volker has been doing. I just didn't push it yet.

I already have another solution for these tests that is less fragile.

Last edited 17 months ago by embray (previous) (diff)

comment:83 Changed 17 months ago by git

  • Commit set to 3b3053bdff12aeb168bc0157aaa4cb3e850839e8

Branch pushed to git repo; I updated commit sha1. Last 10 new commits:

5e2220dbackport guava patch to allow no gcc
29c82e8put each installed package on its own line for readability
d7bd012remove unnecessary cruft
8f22321I don't think this is necessary anymore either
d0b769dset GAP's notion of 'screen width' to the maximum allowed
b50f488backport https://github.com/gap-packages/cohomolo/pull/19
884bc4finstall deps of polenta, qpa, resclasses, fix test
bf2890eemphasize here that the pacakge's name is 'qpa', not 'qpa-version'
8e81b42the exact output of this test depends somewhat on which GAP packages happen to
3b3053badd a convenience method on the libgap object to reset the random seed on

comment:84 Changed 17 months ago by embray

For the remaining problematic tests I was able to fix them by manually calling (with some explanation in the prose) libgap.set_seed(0) before them (and modifying the expected output to correspond to that random seed).

It's unfortunate but possibly necessary, since the state of the PRNG can seem to depend on which packages were loaded and when, and for whatever reason those tests just happened to be very sensitive to this.

I'm re-running the full test suite now to see if any other examples like this turn up.

comment:85 Changed 17 months ago by git

  • Commit changed from 3b3053bdff12aeb168bc0157aaa4cb3e850839e8 to 78859fff771aa376e486d83e1ee4b6ff8ec9afb3

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

78859ffclean up installation of compiled packages so that they are are also

comment:86 Changed 17 months ago by embray

Oops, forgot to finish the tests for libgap.set_seed.

comment:87 Changed 17 months ago by git

  • Commit changed from 78859fff771aa376e486d83e1ee4b6ff8ec9afb3 to 728ad7faab01b0839317c3e77acef5c14e1a0ddb

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

0611aa2add a convenience method on the libgap object to reset the random seed on
728ad7fclean up installation of compiled packages so that they are are also

comment:88 Changed 17 months ago by dimpase

your branch does not have 8.5.rc1 merged, by the way. Just testing it might show wrong test faults...

comment:89 in reply to: ↑ 8 ; follow-up: Changed 17 months ago by embray

Replying to dimpase:

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 

I don't suppose you ever found a solution and/or explanation for this failure? I have some workarounds for most everything else but this.

comment:90 Changed 17 months ago by git

  • Commit changed from 728ad7faab01b0839317c3e77acef5c14e1a0ddb to 6a2d88243b82b25d8e22362c4ea2aae03ca4b72d

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

6a2d882fix some tests where GAP output was improved by increasing the GAP output line width

comment:91 Changed 17 months ago by git

  • Commit changed from 6a2d88243b82b25d8e22362c4ea2aae03ca4b72d to f44ca4cd544b5d565166f4d04c3ef7e11e2267f0

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

f44ca4cexplicitly calling libgap.set_seed(0) for these tests allows consistent results for them

comment:92 in reply to: ↑ 89 Changed 17 months ago by dimpase

Replying to embray:

Replying to dimpase:

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 

I don't suppose you ever found a solution and/or explanation for this failure? I have some workarounds for most everything else but this.

see comment:11. Have you merged the corresponding commit? (from the ticket it refers to)

that's why #26889 is a dependence of this ticket.

Last edited 17 months ago by dimpase (previous) (diff)

comment:93 follow-up: Changed 17 months ago by embray

Ah, right, I had forgotten about that. That makes sense.

The issue here again seems to have something in part to do with the random number generator state in GAP. In the case of:

sage: MG = GU(3,2).as_matrix_group()
sage: PG = MG.as_permutation_group()
sage: PG2 = MG._permutation_group_()

usually PG and PG2 will be the same, but on rare occasion they will be different, and that just happened to be the case in this test. Could that be a bug in GAP, or expected behavior? That I don't know.

That code is still a mess on the Sage end too as well, so any effort to clean it up can't hurt.

comment:94 in reply to: ↑ 93 ; follow-up: Changed 17 months ago by dimpase

Replying to embray:

Ah, right, I had forgotten about that. That makes sense.

The issue here again seems to have something in part to do with the random number generator state in GAP. In the case of:

sage: MG = GU(3,2).as_matrix_group()
sage: PG = MG.as_permutation_group()
sage: PG2 = MG._permutation_group_()

usually PG and PG2 will be the same, but on rare occasion they will be different, and that just happened to be the case in this test. Could that be a bug in GAP, or expected behavior? That I don't know.

We have two instances of GAP talking to each other, how their RNGs are syncronised is an interesting question. Are they?

comment:95 Changed 17 months ago by git

  • Commit changed from f44ca4cd544b5d565166f4d04c3ef7e11e2267f0 to ca05b247f3c373950a1f883ac5c97f8bb1917781

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

35abd34Merge branch 'develop' of trac.sagemath.org:sage into gap410pkg
db3f63emore libGAP in as_permutation(), less pexpect mess
2d77eeefollow up pyflake hints (all but one)
7dd5fa6reviewer's example and fix
ca05b24Merge branch 'u/dimpase/groups/better_as_permutation' of trac.sagemath.org:sage into gap410pkg

comment:96 Changed 17 months ago by dimpase

new gap_packages installation does not work, breaks at cohomolo GAP package:

[gap_packages-4.10.0] cp calcpres.gap ..//..//bin/x86_64-pc-linux-gnu-default64/calcpres.gap
[gap_packages-4.10.0] chmod +x ..//..//bin/x86_64-pc-linux-gnu-default64/calcpres.gap
[gap_packages-4.10.0] make[3]: Leaving directory '/home/scratch2/dimpase/sage/sage/local/var/tmp/sage/build/gap_packages-4.10.0/src/pkg/cohomolo-1.6.7/standalone/progs.d'
[gap_packages-4.10.0] make[2]: Leaving directory '/home/scratch2/dimpase/sage/sage/local/var/tmp/sage/build/gap_packages-4.10.0/src/pkg/cohomolo-1.6.7'
[gap_packages-4.10.0] bin/ -> /home/scratch2/dimpase/sage/sage/local/var/tmp/sage/build/gap_packages-4.10.0/inst/home/scratch2/dimpase/sage/sage/local/lib/gap/pkg/cohomolo-1.6.7
[gap_packages-4.10.0] make[2]: Entering directory '/home/scratch2/dimpase/sage/sage/local/var/tmp/sage/build/gap_packages-4.10.0/src/pkg/cohomolo-1.6.7'
[gap_packages-4.10.0] cd standalone/progs.d; make clean
[gap_packages-4.10.0] make[3]: Entering directory '/home/scratch2/dimpase/sage/sage/local/var/tmp/sage/build/gap_packages-4.10.0/src/pkg/cohomolo-1.6.7/standalone/progs.d'
[gap_packages-4.10.0] /bin/rm -f *.o
[gap_packages-4.10.0] make[3]: Leaving directory '/home/scratch2/dimpase/sage/sage/local/var/tmp/sage/build/gap_packages-4.10.0/src/pkg/cohomolo-1.6.7/standalone/progs.d'
[gap_packages-4.10.0] make[2]: Leaving directory '/home/scratch2/dimpase/sage/sage/local/var/tmp/sage/build/gap_packages-4.10.0/src/pkg/cohomolo-1.6.7'
[gap_packages-4.10.0] ********************************************************************************
[gap_packages-4.10.0] Error: source file/directory bin does not exist
[gap_packages-4.10.0] ********************************************************************************

(this is on Fedora 26, in case it matters)

comment:97 follow-up: Changed 17 months ago by vbraun

#22626 breaks some of the # optional - database_gap doctests. I'll ignore that failure for now but please fix asap.

comment:98 follow-up: Changed 17 months ago by git

  • Commit changed from ca05b247f3c373950a1f883ac5c97f8bb1917781 to bd849fbe2b32a6cc8d4b56b334e4293846061a6b

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

bd849fbdon't clean too early

comment:99 Changed 17 months ago by dimpase

  • Authors changed from Dima Pasechnik to Dima Pasechnik, Erik Bray
  • Status changed from needs_work to needs_review

comment:100 in reply to: ↑ 97 Changed 17 months ago by dimpase

Replying to vbraun:

#22626 breaks some of the # optional - database_gap doctests. I'll ignore that failure for now but please fix asap.

Yes, this ticket gets rid of database_gap (everything there will be a part of gap package), so this is taken care of here.

I've also fixed the build failure here, so it's ready for review, I think.

comment:101 Changed 17 months ago by dimpase

So the current scheme is that gap_packages remains optional spkg, for the time being at least. I'd rather make it standard too, and then merge into gap package, but Erik thinks it's too big a change (I don't think so---I think the change between what we have in Sage 8.4 (and 8.5) is already very big, not the least due to GAP 4.10 being quite a big change from GAP 4.8, and forcing people to adjust their code several times is worse than asking to do so once--- but OK).

Last edited 17 months ago by dimpase (previous) (diff)

comment:102 Changed 17 months ago by git

  • Commit changed from bd849fbe2b32a6cc8d4b56b334e4293846061a6b to ff7722e4edb044fcadef0c83d78d72193a4aa013

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

ff7722esuppress "#I.." warnings

comment:103 Changed 17 months ago by dimpase

this commit went AWOL from the current branch. With it in, all tests pass

comment:104 Changed 17 months ago by git

  • Commit changed from ff7722e4edb044fcadef0c83d78d72193a4aa013 to bf405752a966b932afb7a7bb34d7be388c94324b

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

bf40575silencing of #I warning from GAP in the function body

comment:105 Changed 17 months ago by dimpase

One more AWOL commit.

comment:106 follow-up: Changed 17 months ago by git

  • Commit changed from bf405752a966b932afb7a7bb34d7be388c94324b to 37e23290bad2ee4d36463647f4c058c500b81eba

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

37e2329tagged # random - answer is always mathematically OK, outputs differ

comment:107 in reply to: ↑ 71 Changed 17 months ago by embray

Replying to dimpase:

Replying to git:

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

1e5f679move silencing into the function body

this addresses comment 65.

This should probably use InfoLevel(InfoWarning) to get the previous level, not just assume to restore it to 1. Probably unlikely to matter usually but it is still a bug waiting to happen.

comment:108 in reply to: ↑ 106 Changed 17 months ago by embray

Replying to git:

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

37e2329tagged # random - answer is always mathematically OK, outputs differ

I'm going to double-check but this shouldn't be necessary. This is what #26874 was fixing.

comment:109 Changed 17 months ago by embray

Yes, merging in #26874 guarantees that test always works the same way, as the RNG state is consistent.

I'll deal with comment:107 and then do a couple final checks before setting positive_review.

comment:110 in reply to: ↑ 98 Changed 17 months ago by embray

Replying to git:

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

bd849fbdon't clean too early

This "fix" is a problem as well. The "cleanup" commands that were moved were deliberately before sdh_install * so that files that shouldn't be installed aren't installed. I'll see if I can reproduce the problem you had; I did not have the same problem on Ubuntu.

comment:111 Changed 17 months ago by embray

  • Status changed from needs_review to needs_work

comment:112 in reply to: ↑ 94 Changed 17 months ago by embray

Replying to dimpase:

Replying to embray:

Ah, right, I had forgotten about that. That makes sense.

The issue here again seems to have something in part to do with the random number generator state in GAP. In the case of:

sage: MG = GU(3,2).as_matrix_group()
sage: PG = MG.as_permutation_group()
sage: PG2 = MG._permutation_group_()

usually PG and PG2 will be the same, but on rare occasion they will be different, and that just happened to be the case in this test. Could that be a bug in GAP, or expected behavior? That I don't know.

We have two instances of GAP talking to each other, how their RNGs are syncronised is an interesting question. Are they?

Nope. I do have an experimental branch where I modified Randstate.set_seed_gap() to seed both libgap and the pexpect GAP simultaneously, but I haven't decided whether or not to use it yet.

comment:113 Changed 17 months ago by git

  • Commit changed from 37e23290bad2ee4d36463647f4c058c500b81eba to 5ad8209875bf43561ff0ff86b797cae67e1d5c02

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

f6c8734When running doctests, use the current_randstate()'s seed for Interface.rand_seed()
1dc5078Add the generic InfoWarning info class to the 'standard' GAP globals
e2c2c54suppress "#I.." warnings
5ad8209followup to #22626: adds a prerm script to the gap SPKG

comment:114 Changed 17 months ago by git

  • Commit changed from 5ad8209875bf43561ff0ff86b797cae67e1d5c02 to 84a3d2f93f74ce2d09d19e0a55ee5ae8a9c7f45f

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

2449886When running doctests, use the current_randstate()'s seed for Interface.rand_seed()
d9dbd00Add the generic InfoWarning info class to the 'standard' GAP globals
40b4108suppress "#I.." warnings
f3a938bfollowup to #22626: adds a prerm script to the gap SPKG
84a3d2fmake sure this make clean command is not run in parallel

comment:115 Changed 17 months ago by embray

Can you check if the latest commit fixes the issue from comment:96 ? I can't reproduce that so I'm not really sure what's going on there (seeing more of the log might help too, if it's not fixed...)

comment:116 follow-up: Changed 17 months ago by slelievre

  • Cc alexk arojas dimpase embray fbissey mantepse mmarco slelievre tscrim vbraun added
  • Description modified (diff)
  • Keywords GAP added
  • Milestone changed from sage-8.5 to sage-8.6

Does this also fix #23054?

Changed 17 months ago by mantepse

comment:117 Changed 17 months ago by mantepse

I also have the problem from comment:96, on ubuntu 17.10, sage 8.6.beta0. I attached the log.

comment:118 follow-up: Changed 17 months ago by embray

Thanks--I was able to reproduce the problem in a docker container. My attempted fix doesn't seem to help. Not sure what the problem is though; it's an odd one...

comment:119 in reply to: ↑ 118 Changed 17 months ago by dimpase

Replying to embray:

Thanks--I was able to reproduce the problem in a docker container. My attempted fix doesn't seem to help. Not sure what the problem is though; it's an odd one...

IMHO it should not hold up this ticket - open a follow-up ticket to fix it.

comment:120 Changed 17 months ago by git

  • Commit changed from 84a3d2f93f74ce2d09d19e0a55ee5ae8a9c7f45f to f3a938b92a922a0e78b955b90183b739c965d033

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

comment:121 Changed 17 months ago by embray

  • Dependencies changed from #22626, #26889 to #22626, #26889, #26965

comment:122 Changed 17 months ago by git

  • Commit changed from f3a938b92a922a0e78b955b90183b739c965d033 to 293e79ab78bf1574e469ddfa9ff8a93b7087c396

Branch pushed to git repo; I updated commit sha1. This was a forced push. Last 10 new commits:

3213d2cinstall deps of polenta, qpa, resclasses, fix test
2a2eb73emphasize here that the pacakge's name is 'qpa', not 'qpa-version'
d50642bthe exact output of this test depends somewhat on which GAP packages happen to
b68c547add a convenience method on the libgap object to reset the random seed on
47e4b6eclean up installation of compiled packages so that they are are also
2da5c32fix some tests where GAP output was improved by increasing the GAP output line width
508e6d8explicitly calling libgap.set_seed(0) for these tests allows consistent results for them
08f52a3Add the generic InfoWarning info class to the 'standard' GAP globals
2b65d0csuppress "#I.." warnings
293e79afollowup to #22626: adds a prerm script to the gap SPKG

comment:123 Changed 17 months ago by embray

Rebased on 8.6.beta0

comment:124 Changed 17 months ago by git

  • Commit changed from 293e79ab78bf1574e469ddfa9ff8a93b7087c396 to 9854c3a3a59fd2ab42aacfb78f38346439f280d9

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

5f49332fix sdh_install of symlinks with invalid targets
9854c3aMerge branch 'u/embray/build/sdh-install-symlinks' into public/ticket-26856

comment:125 Changed 17 months ago by embray

  • Status changed from needs_work to needs_review

Merged fix from #26965 so that this branch can be built successfully from a fresh install.

comment:126 Changed 17 months ago by dimpase

comment in the commit 293e79a is a bit off, as altlasrep has a cache of files downloaded from the net, not generated.

comment:127 follow-up: Changed 17 months ago by dimpase

I am getting timeouts in these iffy parts of libgap:

sage -t --warn-long 109.1 src/sage/libs/gap/all_documented_functions.py
    Timed out
sage -t --warn-long 109.1 src/sage/libs/gap/assigned_names.py
    Timed out

something these should be tagged # long time ?

comment:128 Changed 17 months ago by embray

Wherever the files come from I just mean that they are created by the package at runtime and are not "installed".

comment:129 in reply to: ↑ 127 Changed 17 months ago by embray

Replying to dimpase:

I am getting timeouts in these iffy parts of libgap:

sage -t --warn-long 109.1 src/sage/libs/gap/all_documented_functions.py
    Timed out
sage -t --warn-long 109.1 src/sage/libs/gap/assigned_names.py
    Timed out

something these should be tagged # long time ?

I think they are. I originally meant, as part of #22626 to deprecate these modules and disable their tests, but I forgot to add that to the work items.

While we're at it I'll go ahead and mark the tests to be skipped, and will handle the deprecation later, perhaps as part of #26959.

comment:130 Changed 17 months ago by dimpase

OK, tests are running, I'll have a look in the morning (I'm on UTC+8 now, sorry :-))

comment:131 Changed 17 months ago by embray

I did a fresh test run earlier today and everything looked good, but it will be good to get a second opinion. I'm about to push one more change to disable skip those problematic tests.

comment:132 Changed 17 months ago by git

  • Commit changed from 9854c3a3a59fd2ab42aacfb78f38346439f280d9 to 8bf1044efda750d8a767d88aa6a2afda69edadbe

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

8bf1044disable doctesting of these modules

comment:133 Changed 17 months ago by mantepse

seems to work here! (same timeouts, because I didn't have the final commit yet)

Thank you!

comment:134 Changed 17 months ago by dimpase

  • Status changed from needs_review to positive_review

OK, great, all works.


New commits:

8bf1044disable doctesting of these modules

comment:135 Changed 17 months ago by dimpase

  • Reviewers set to Erik Bray, Dima Pasechnik

comment:136 in reply to: ↑ 116 Changed 17 months ago by dimpase

Replying to slelievre:

Does this also fix #23054?

yes it does, thanks for pointing this out.

comment:137 Changed 17 months ago by vbraun

  • Branch changed from public/ticket-26856 to 8bf1044efda750d8a767d88aa6a2afda69edadbe
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.