Opened 18 months ago
Closed 17 months ago
#26856 closed enhancement (fixed)
update gap_packages and database_gap to GAP 4.10
Reported by:  dimpase  Owned by:  

Priority:  critical  Milestone:  sage8.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 )
Now everything needed is already in GAP's tarball.
There is also no more copyrightinduced 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)
Change History (138)
comment:1 Changed 18 months ago by
 Branch set to u/dimpase/GQ
 Commit set to 1ed2d00b452155e803c79040460edcd55c120320
comment:2 Changed 18 months ago by
 Dependencies set to #22626
comment:3 Changed 18 months ago by
Is there a particular reason to merge database_gap
into gap_packages
?
comment:4 followup: ↓ 5 Changed 18 months ago by
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 issuesstuff in database_gap is not GPLcompatible before GAP 4.9 or 4.10
 nonworking with libgap GAP kernel extensions
the 1st one, if we decide not to repackage original GAP tarball (and I am very much for not repackaging), 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
comment:6 Changed 18 months ago by
 Commit changed from 1ed2d00b452155e803c79040460edcd55c120320 to 5d3fce1000f67cfea8be984df8959933c403e83c
Branch pushed to git repo; I updated commit sha1. New commits:
5d3fce1  fixed noisy "#I MakeReadWriteGlobal" things

comment:7 Changed 18 months ago by
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 followups: ↓ 9 ↓ 89 Changed 18 months ago by
 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/sagetracmirror/local/lib/python2.7/sitepackages/sage/doctest/forker.py", line 671, in _run self.compile_and_execute(example, compiler, test.globs) File "/home/dimpase/Sage/sagetracmirror/local/lib/python2.7/sitepackages/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/sagetracmirror/local/lib/python2.7/sitepackages/sage/doctest/forker.py", line 671, in _run self.compile_and_execute(example, compiler, test.globs) File "/home/dimpase/Sage/sagetracmirror/local/lib/python2.7/sitepackages/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...)
comment:9 in reply to: ↑ 8 Changed 18 months ago by
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 followup: ↓ 12 Changed 18 months ago by
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 sideeffects in Sage alone, or in Sage interacting with the doctest frameworkin the latter case this means it's not possible to reproduce on "standalone" Sage.
comment:11 Changed 18 months ago by
 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:
60d7eb6  Allow Gap.__getattr__ to return GAP objects other than just functions

0cf2047  updated GAP_Enter patch that does not require gapstate.h in libgapapi.h

2de776d  Merge branch 'u/embray/spkgs/gap410' of trac.sagemath.org:sage into HEAD

eefc0fc  fix warning from cython

9292d47  add sig_on/off() around GAP_Initialize, just in case unhandled segfaults or the like occur

132a7c3  copy the current environment before passing it to GAP

29d4c84  Merge branch 'u/embray/spkgs/gap410' of trac.sagemath.org:sage into HEAD

d7c85cd  When running doctests, use the current_randstate()'s seed for Interface.rand_seed()

e6a7397  Merge branch 'u/embray/doctests/interfacesrandseed' of trac.sagemath.org:sage into HEAD

ef2230a  more libGAP in as_permutation(), less pexpect mess

comment:12 in reply to: ↑ 10 Changed 18 months ago by
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
 Commit changed from ef2230a3c98cb6d8b500e5ba6b925199c30a5647 to 9266fed2e972f870d0c4ba891993bbfbb13f2100
Branch pushed to git repo; I updated commit sha1. New commits:
9266fed  adjusting to the new bigger GAP database, better messages

comment:14 Changed 18 months ago by
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
OK, I'll make our default GAP, without gap_packages
installed, to be the same as vanilla GAP 4.10, which is, packagewise, as follows:
$ ./gap ┌───────┐ GAP 4.10.0 of 01Nov2018 │ GAP │ https://www.gapsystem.org └───────┘ Architecture: x86_64pclinuxgnudefault64 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.
comment:16 Changed 18 months ago by
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
 Commit changed from 9266fed2e972f870d0c4ba891993bbfbb13f2100 to 01665fa57aa56458b2d43765fd17b88944186f23
Branch pushed to git repo; I updated commit sha1. New commits:
01665fa  rm #optional  database_gap tags, doc updates, few GAP>libGAP changes

comment:18 Changed 18 months ago by
 Commit changed from 01665fa57aa56458b2d43765fd17b88944186f23 to 90f2fea00aa8aa088c619ac128faf2e5ec5cecbd
comment:19 Changed 18 months ago by
done getting rid of database_gap
Now on to gap_packages...
comment:20 Changed 18 months ago by
 Dependencies changed from #22626 to #22626, #26889
comment:21 Changed 18 months ago by
opened #26901 to deal with changes in the only (apart from gap_packages) package of Sage dependent on database_gap.
comment:22 Changed 18 months ago by
 Commit changed from 90f2fea00aa8aa088c619ac128faf2e5ec5cecbd to 9407fba12c12b60c77eb47d11046964be2dd2c74
Branch pushed to git repo; I updated commit sha1. New commits:
9407fba  reviewer's example and fix

comment:23 Changed 17 months ago by
 Commit changed from 9407fba12c12b60c77eb47d11046964be2dd2c74 to ca7f6ff6bf514e3b640ab39d8f51760eacc75a88
Branch pushed to git repo; I updated commit sha1. New commits:
68a50fc  Merge branch 'u/embray/doctests/interfacesrandseed' of trac.sagemath.org:sage into g410

38e687b  install only those packages that are required by GAP to run

8b0d0ad  Implement GAPElement_List.__bool__ to work like a Python list

6a5d34a  fix the test for available operations on an object

72df280  clean up gap_root() a bit more; remove obsolete warning message

4e6eb3e  Merge branch 'u/embray/spkgs/gap410' of trac.sagemath.org:sage into g410

ca7f6ff  Merge branch 'u/dimpase/GQ' of trac.sagemath.org:sage into g410

comment:24 Changed 17 months ago by
 Commit changed from ca7f6ff6bf514e3b640ab39d8f51760eacc75a88 to b63db23cde4ed4800a93caec12edec29d4c24aab
Branch pushed to git repo; I updated commit sha1. New commits:
b63db23  allow GAP default packages and install them

comment:25 Changed 17 months ago by
 Commit changed from b63db23cde4ed4800a93caec12edec29d4c24aab to d305273fcdd2c247a0c50b8432e2939bc9c6a0f6
Branch pushed to git repo; I updated commit sha1. New commits:
d305273  install atlasrep, as it is a dependency of tomlib

comment:26 Changed 17 months ago by
 Commit changed from d305273fcdd2c247a0c50b8432e2939bc9c6a0f6 to 1b7109983cb030067c6a7e471da076edf8fe6f64
comment:27 Changed 17 months ago by
 Commit changed from 1b7109983cb030067c6a7e471da076edf8fe6f64 to d879657c584c866152fe795888ee7d02c8668f12
comment:28 Changed 17 months ago by
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
 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
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
 Commit changed from d879657c584c866152fe795888ee7d02c8668f12 to 785b9dedc352d33a74f8f5d54f6c91c436b225d8
Branch pushed to git repo; I updated commit sha1. New commits:
5e61d7b  pass A option to GAP to disable autoloading of default packages for both GAP interfaces

18f2f43  instead of disabling autoloading of default packages, just install all default packages as a standard part of the GAP SPKG

da909ef  more fixes to the GAP SPKG installation

d1c98f3  also include the atlasrep package, which is a dependency of tomlib

785b9de  Merge branch 'u/embray/spkgs/gap410' of trac.sagemath.org:sage into gap410pkg

comment:32 Changed 17 months ago by
 Commit changed from 785b9dedc352d33a74f8f5d54f6c91c436b225d8 to 67ca32109f47388f820d95f4171f01bc050e32c8
Branch pushed to git repo; I updated commit sha1. New commits:
67ca321  fixed gap_packages, also package compiling/installing

comment:33 Changed 17 months ago by
 Commit changed from 67ca32109f47388f820d95f4171f01bc050e32c8 to a7ef9e015984e48785b80d95ff422bcd3f6ae232
Branch pushed to git repo; I updated commit sha1. New commits:
a7ef9e0  backport guava patch to allow no gcc

comment:34 Changed 17 months ago by
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 followup: ↓ 37 Changed 17 months ago by
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
 Commit changed from a7ef9e015984e48785b80d95ff422bcd3f6ae232 to 01d85a7b1ebe5b95cb8a84d273dafd13c62f4a65
comment:37 in reply to: ↑ 35 Changed 17 months ago by
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
I need to adjust formatting in a dozen of tests, after that should be ready for review.
comment:39 Changed 17 months ago by
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 followup: ↓ 43 Changed 17 months ago by
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...
comment:41 Changed 17 months ago by
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
I agree about nauty/grape  see #26923
comment:43 in reply to: ↑ 40 ; followup: ↓ 44 Changed 17 months ago by
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 outputsF\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
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 outputsF\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
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 followup: ↓ 48 Changed 17 months ago by
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 followups: ↓ 49 ↓ 57 Changed 17 months ago by
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 qpaversion 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 qpaversion 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 "qpaversion". The name of the package is just "qpa", but its source directory is unusually named "qpaversion<x.y>", so there's some code that's making an invalid assumption about GAP package names :sigh:
comment:48 in reply to: ↑ 46 Changed 17 months ago by
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
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 qpaversion 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 qpaversion 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 "qpaversion". The name of the package is just "qpa", but its source directory is unusually named "qpaversion<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
 Commit changed from 01d85a7b1ebe5b95cb8a84d273dafd13c62f4a65 to 78db91a51010662e85dda67649495eaed4193025
comment:51 Changed 17 months ago by
OK, over to you now.
comment:52 Changed 17 months ago by
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): 1566 1564 sage: Q = QuantumGroup(['A',2]) # optional  gap_packages 1567 1565 sage: V = Q.highest_weight_module([1,1]) # optional  gap_packages 1568 1566 sage: V.gap() # optional  gap_packages 1569 <8dimensional leftmodule over QuantumUEA( <root system of type A 2>,1570 Qpar = q )>1567 <8dimensional leftmodule over QuantumUEA( <root system of type A 1568 2>, Qpar = q )> 1571 1569 """ 1572 1570 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
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
linebreaks are inserted by GAP, it was always the case.
comment:55 Changed 17 months ago by
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 followup.
comment:56 Changed 17 months ago by
 Commit changed from 78db91a51010662e85dda67649495eaed4193025 to fc197922d0c70666dfaa48efccbedfdeb7be1070
Branch pushed to git repo; I updated commit sha1. New commits:
fc19792  install deps of polenta, qpa, resclasses, fix test

comment:57 in reply to: ↑ 47 Changed 17 months ago by
Replying to embray:
Another one that's a bit troubling is:
sage t long src/sage/tests/gap_packages.py [.....] ++++ Failure polenta fail Failure qpaversion 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 "qpaversion". The name of the package is just "qpa", but its source directory is unusually named "qpaversion<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 twofold:
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
 Commit changed from fc197922d0c70666dfaa48efccbedfdeb7be1070 to 92517b495dc21eb739f9c8fcb39840674bc690f0
Branch pushed to git repo; I updated commit sha1. New commits:
92517b4  suppress "#I.." warnings

comment:59 followup: ↓ 66 Changed 17 months ago by
 Commit changed from 92517b495dc21eb739f9c8fcb39840674bc690f0 to 08ffb3396308b4643cfcbed1c56c0272c39f6a17
Branch pushed to git repo; I updated commit sha1. New commits:
08ffb33  more doctest fixes

comment:60 Changed 17 months ago by
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_packages4.10.0] configure: creating ./config.status [gap_packages4.10.0] config.status: creating Makefile [gap_packages4.10.0] config.status: creating src/pkgconfig.h [gap_packages4.10.0] config.status: executing depfiles commands [gap_packages4.10.0] config.status: executing libtool commands [gap_packages4.10.0] make[2]: Entering directory '/mnt/opt/Sage/sagedev/local/share/gap/pkg/io4.5.4' [gap_packages4.10.0] /bin/sh ./libtool tag=CC mode=compile gcc DHAVE_CONFIG_H I. I./src I/mnt/opt/Sage/sagedev/local/var/tmp/sage/build/gap4.10.0/src/gen I/mnt/opt/Sage/sagedev/local/var/tmp/sage/build/gap4.10.0/src/src I/mnt/opt/Sage/sagedev/local/var/tmp/sage/build/gap4.10.0/src DHAVE_CONFIG_H I/mnt/opt/Sage/sagedev/local/include O2 g g O2 MT src/io_laio.lo MD MP MF src/.deps/io_laio.Tpo c o src/io_laio.lo `test f 'src/io.c'  echo './'`src/io.c [gap_packages4.10.0] libtool: compile: gcc DHAVE_CONFIG_H I. I./src I/mnt/opt/Sage/sagedev/local/var/tmp/sage/build/gap4.10.0/src/gen I/mnt/opt/Sage/sagedev/local/var/tmp/sage/build/gap4.10.0/src/src I/mnt/opt/Sage/sagedev/local/var/tmp/sage/build/gap4.10.0/src DHAVE_CONFIG_H I/mnt/opt/Sage/sagedev/local/include O2 g g O2 MT src/io_laio.lo MD MP MF src/.deps/io_laio.Tpo c src/io.c fPIC DPIC o src/.libs/io_laio.o [gap_packages4.10.0] src/io.c:14:10: fatal error: src/compiled.h: No such file or directory [gap_packages4.10.0] #include "src/compiled.h" /* GAP headers */ [gap_packages4.10.0] ^~~~~~~~~~~~~~~~
looks like the corresponding I
arguments all have an extra /src
suffix, something that prevents this header to be found.
comment:61 followup: ↓ 67 Changed 17 months ago by
..."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
GAP's IO is on #26930
comment:63 Changed 17 months ago by
 Commit changed from 08ffb3396308b4643cfcbed1c56c0272c39f6a17 to c2e30ae0aff04ce2c88b9e6e9126caac305e85eb
Branch pushed to git repo; I updated commit sha1. New commits:
c2e30ae  silence one more #I message from GAP

comment:64 Changed 17 months ago by
 Status changed from needs_work to needs_review
OK, seems to be ready now
comment:65 Changed 17 months ago by

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(): 2442 2442 EXAMPLES:: 2443 2443 2444 2444 sage: from sage.graphs.strongly_regular_db import SRG_416_100_36_20 2445 sage: libgap.eval("SetInfoLevel(InfoWarning,0)") # silence #I warnings 2445 2446 sage: g = SRG_416_100_36_20() # optional  gap_packages # long time 2447 sage: libgap.eval("SetInfoLevel(InfoWarning,1)") # silence #I warnings 2446 2448 sage: g.is_strongly_regular(parameters=True) # optional  gap_packages # long time 2447 2449 (416, 100, 36, 20) 2448 2450 """
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.
comment:66 in reply to: ↑ 59 ; followups: ↓ 69 ↓ 72 Changed 17 months ago by
Replying to git:
Branch pushed to git repo; I updated commit sha1. New commits:
08ffb33 more 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 troublingit may still be some badly managed random state somewhere. Unless I'm missing something else.
comment:67 in reply to: ↑ 61 ; followup: ↓ 68 Changed 17 months ago by
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
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
Replying to embray:
Replying to git:
Branch pushed to git repo; I updated commit sha1. New commits:
08ffb33 more 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 troublingit 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)
comment:70 followup: ↓ 71 Changed 17 months ago by
 Commit changed from c2e30ae0aff04ce2c88b9e6e9126caac305e85eb to 1e5f679c41a7b27088ed330f986df91d03099a71
Branch pushed to git repo; I updated commit sha1. New commits:
1e5f679  move silencing into the function body

comment:71 in reply to: ↑ 70 ; followup: ↓ 107 Changed 17 months ago by
comment:72 in reply to: ↑ 66 Changed 17 months ago by
Replying to embray:
Replying to git:
Branch pushed to git repo; I updated commit sha1. New commits:
08ffb33 more 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 troublingit 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
Let's just make gap_packages standard, as planned anyway. This will alleviate this problem.
comment:74 followup: ↓ 76 Changed 17 months ago by
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...
comment:75 Changed 17 months ago by
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 reseed the PRNG before them; let me try.
comment:76 in reply to: ↑ 74 Changed 17 months ago by
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
I'm definitely 1 on making it a standard package, especially in the short time that we have. These have always been an optional addon, 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
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 reseed them before the affected tests (not ideal, but it will ensure consistent results).
comment:79 Changed 17 months ago by
 Branch changed from u/dimpase/GQ to public/ticket26856
 Commit 1e5f679c41a7b27088ed330f986df91d03099a71 deleted
 Status changed from needs_review to needs_work
Public branch for this ticket. It is uptodate with Dima's branch minus the last two commits that are contentious.
comment:80 Changed 17 months ago by
your branch does not merge cleanly...
comment:81 Changed 17 months ago by
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
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.
comment:83 Changed 17 months ago by
 Commit set to 3b3053bdff12aeb168bc0157aaa4cb3e850839e8
Branch pushed to git repo; I updated commit sha1. Last 10 new commits:
5e2220d  backport guava patch to allow no gcc

29c82e8  put each installed package on its own line for readability

d7bd012  remove unnecessary cruft

8f22321  I don't think this is necessary anymore either

d0b769d  set GAP's notion of 'screen width' to the maximum allowed

b50f488  backport https://github.com/gappackages/cohomolo/pull/19

884bc4f  install deps of polenta, qpa, resclasses, fix test

bf2890e  emphasize here that the pacakge's name is 'qpa', not 'qpaversion'

8e81b42  the exact output of this test depends somewhat on which GAP packages happen to

3b3053b  add a convenience method on the libgap object to reset the random seed on

comment:84 Changed 17 months ago by
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 rerunning the full test suite now to see if any other examples like this turn up.
comment:85 Changed 17 months ago by
 Commit changed from 3b3053bdff12aeb168bc0157aaa4cb3e850839e8 to 78859fff771aa376e486d83e1ee4b6ff8ec9afb3
Branch pushed to git repo; I updated commit sha1. New commits:
78859ff  clean up installation of compiled packages so that they are are also

comment:86 Changed 17 months ago by
Oops, forgot to finish the tests for libgap.set_seed.
comment:87 Changed 17 months ago by
 Commit changed from 78859fff771aa376e486d83e1ee4b6ff8ec9afb3 to 728ad7faab01b0839317c3e77acef5c14e1a0ddb
comment:88 Changed 17 months ago by
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 ; followup: ↓ 92 Changed 17 months ago by
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
 Commit changed from 728ad7faab01b0839317c3e77acef5c14e1a0ddb to 6a2d88243b82b25d8e22362c4ea2aae03ca4b72d
Branch pushed to git repo; I updated commit sha1. New commits:
6a2d882  fix some tests where GAP output was improved by increasing the GAP output line width

comment:91 Changed 17 months ago by
 Commit changed from 6a2d88243b82b25d8e22362c4ea2aae03ca4b72d to f44ca4cd544b5d565166f4d04c3ef7e11e2267f0
Branch pushed to git repo; I updated commit sha1. New commits:
f44ca4c  explicitly calling libgap.set_seed(0) for these tests allows consistent results for them

comment:92 in reply to: ↑ 89 Changed 17 months ago by
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.pyI 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.
comment:93 followup: ↓ 94 Changed 17 months ago by
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 ; followup: ↓ 112 Changed 17 months ago by
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
andPG2
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
 Commit changed from f44ca4cd544b5d565166f4d04c3ef7e11e2267f0 to ca05b247f3c373950a1f883ac5c97f8bb1917781
Branch pushed to git repo; I updated commit sha1. New commits:
35abd34  Merge branch 'develop' of trac.sagemath.org:sage into gap410pkg

db3f63e  more libGAP in as_permutation(), less pexpect mess

2d77eee  follow up pyflake hints (all but one)

7dd5fa6  reviewer's example and fix

ca05b24  Merge branch 'u/dimpase/groups/better_as_permutation' of trac.sagemath.org:sage into gap410pkg

comment:96 Changed 17 months ago by
new gap_packages installation does not work, breaks at cohomolo GAP package:
[gap_packages4.10.0] cp calcpres.gap ..//..//bin/x86_64pclinuxgnudefault64/calcpres.gap [gap_packages4.10.0] chmod +x ..//..//bin/x86_64pclinuxgnudefault64/calcpres.gap [gap_packages4.10.0] make[3]: Leaving directory '/home/scratch2/dimpase/sage/sage/local/var/tmp/sage/build/gap_packages4.10.0/src/pkg/cohomolo1.6.7/standalone/progs.d' [gap_packages4.10.0] make[2]: Leaving directory '/home/scratch2/dimpase/sage/sage/local/var/tmp/sage/build/gap_packages4.10.0/src/pkg/cohomolo1.6.7' [gap_packages4.10.0] bin/ > /home/scratch2/dimpase/sage/sage/local/var/tmp/sage/build/gap_packages4.10.0/inst/home/scratch2/dimpase/sage/sage/local/lib/gap/pkg/cohomolo1.6.7 [gap_packages4.10.0] make[2]: Entering directory '/home/scratch2/dimpase/sage/sage/local/var/tmp/sage/build/gap_packages4.10.0/src/pkg/cohomolo1.6.7' [gap_packages4.10.0] cd standalone/progs.d; make clean [gap_packages4.10.0] make[3]: Entering directory '/home/scratch2/dimpase/sage/sage/local/var/tmp/sage/build/gap_packages4.10.0/src/pkg/cohomolo1.6.7/standalone/progs.d' [gap_packages4.10.0] /bin/rm f *.o [gap_packages4.10.0] make[3]: Leaving directory '/home/scratch2/dimpase/sage/sage/local/var/tmp/sage/build/gap_packages4.10.0/src/pkg/cohomolo1.6.7/standalone/progs.d' [gap_packages4.10.0] make[2]: Leaving directory '/home/scratch2/dimpase/sage/sage/local/var/tmp/sage/build/gap_packages4.10.0/src/pkg/cohomolo1.6.7' [gap_packages4.10.0] ******************************************************************************** [gap_packages4.10.0] Error: source file/directory bin does not exist [gap_packages4.10.0] ********************************************************************************
(this is on Fedora 26, in case it matters)
comment:97 followup: ↓ 100 Changed 17 months ago by
#22626 breaks some of the # optional  database_gap
doctests. I'll ignore that failure for now but please fix asap.
comment:98 followup: ↓ 110 Changed 17 months ago by
 Commit changed from ca05b247f3c373950a1f883ac5c97f8bb1917781 to bd849fbe2b32a6cc8d4b56b334e4293846061a6b
Branch pushed to git repo; I updated commit sha1. New commits:
bd849fb  don't clean too early

comment:99 Changed 17 months ago by
 Status changed from needs_work to needs_review
comment:100 in reply to: ↑ 97 Changed 17 months ago by
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
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 soI 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).
comment:102 Changed 17 months ago by
 Commit changed from bd849fbe2b32a6cc8d4b56b334e4293846061a6b to ff7722e4edb044fcadef0c83d78d72193a4aa013
Branch pushed to git repo; I updated commit sha1. New commits:
ff7722e  suppress "#I.." warnings

comment:103 Changed 17 months ago by
this commit went AWOL from the current branch. With it in, all tests pass
comment:104 Changed 17 months ago by
 Commit changed from ff7722e4edb044fcadef0c83d78d72193a4aa013 to bf405752a966b932afb7a7bb34d7be388c94324b
Branch pushed to git repo; I updated commit sha1. New commits:
bf40575  silencing of #I warning from GAP in the function body

comment:105 Changed 17 months ago by
One more AWOL commit.
comment:106 followup: ↓ 108 Changed 17 months ago by
 Commit changed from bf405752a966b932afb7a7bb34d7be388c94324b to 37e23290bad2ee4d36463647f4c058c500b81eba
Branch pushed to git repo; I updated commit sha1. New commits:
37e2329  tagged # random  answer is always mathematically OK, outputs differ

comment:107 in reply to: ↑ 71 Changed 17 months ago by
Replying to dimpase:
Replying to git:
Branch pushed to git repo; I updated commit sha1. New commits:
1e5f679 move 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
comment:109 Changed 17 months ago by
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
Replying to git:
Branch pushed to git repo; I updated commit sha1. New commits:
bd849fb don'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
 Status changed from needs_review to needs_work
comment:112 in reply to: ↑ 94 Changed 17 months ago by
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
andPG2
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
 Commit changed from 37e23290bad2ee4d36463647f4c058c500b81eba to 5ad8209875bf43561ff0ff86b797cae67e1d5c02
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
f6c8734  When running doctests, use the current_randstate()'s seed for Interface.rand_seed()

1dc5078  Add the generic InfoWarning info class to the 'standard' GAP globals

e2c2c54  suppress "#I.." warnings

5ad8209  followup to #22626: adds a prerm script to the gap SPKG

comment:114 Changed 17 months ago by
 Commit changed from 5ad8209875bf43561ff0ff86b797cae67e1d5c02 to 84a3d2f93f74ce2d09d19e0a55ee5ae8a9c7f45f
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
2449886  When running doctests, use the current_randstate()'s seed for Interface.rand_seed()

d9dbd00  Add the generic InfoWarning info class to the 'standard' GAP globals

40b4108  suppress "#I.." warnings

f3a938b  followup to #22626: adds a prerm script to the gap SPKG

84a3d2f  make sure this make clean command is not run in parallel

comment:115 Changed 17 months ago by
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 followup: ↓ 136 Changed 17 months ago by
 Cc alexk arojas dimpase embray fbissey mantepse mmarco slelievre tscrim vbraun added
 Description modified (diff)
 Keywords GAP added
 Milestone changed from sage8.5 to sage8.6
Does this also fix #23054?
Changed 17 months ago by
comment:117 Changed 17 months ago by
I also have the problem from comment:96, on ubuntu 17.10, sage 8.6.beta0. I attached the log.
comment:118 followup: ↓ 119 Changed 17 months ago by
ThanksI 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
Replying to embray:
ThanksI 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 followup ticket to fix it.
comment:120 Changed 17 months ago by
 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
 Dependencies changed from #22626, #26889 to #22626, #26889, #26965
comment:122 Changed 17 months ago by
 Commit changed from f3a938b92a922a0e78b955b90183b739c965d033 to 293e79ab78bf1574e469ddfa9ff8a93b7087c396
Branch pushed to git repo; I updated commit sha1. This was a forced push. Last 10 new commits:
3213d2c  install deps of polenta, qpa, resclasses, fix test

2a2eb73  emphasize here that the pacakge's name is 'qpa', not 'qpaversion'

d50642b  the exact output of this test depends somewhat on which GAP packages happen to

b68c547  add a convenience method on the libgap object to reset the random seed on

47e4b6e  clean up installation of compiled packages so that they are are also

2da5c32  fix some tests where GAP output was improved by increasing the GAP output line width

508e6d8  explicitly calling libgap.set_seed(0) for these tests allows consistent results for them

08f52a3  Add the generic InfoWarning info class to the 'standard' GAP globals

2b65d0c  suppress "#I.." warnings

293e79a  followup to #22626: adds a prerm script to the gap SPKG

comment:123 Changed 17 months ago by
Rebased on 8.6.beta0
comment:124 Changed 17 months ago by
 Commit changed from 293e79ab78bf1574e469ddfa9ff8a93b7087c396 to 9854c3a3a59fd2ab42aacfb78f38346439f280d9
comment:125 Changed 17 months ago by
 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
comment in the commit 293e79a is a bit off, as altlasrep has a cache of files downloaded from the net, not generated.
comment:127 followup: ↓ 129 Changed 17 months ago by
I am getting timeouts in these iffy parts of libgap:
sage t warnlong 109.1 src/sage/libs/gap/all_documented_functions.py Timed out sage t warnlong 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
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
Replying to dimpase:
I am getting timeouts in these iffy parts of libgap:
sage t warnlong 109.1 src/sage/libs/gap/all_documented_functions.py Timed out sage t warnlong 109.1 src/sage/libs/gap/assigned_names.py Timed outsomething 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
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
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
 Commit changed from 9854c3a3a59fd2ab42aacfb78f38346439f280d9 to 8bf1044efda750d8a767d88aa6a2afda69edadbe
Branch pushed to git repo; I updated commit sha1. New commits:
8bf1044  disable doctesting of these modules

comment:133 Changed 17 months ago by
seems to work here! (same timeouts, because I didn't have the final commit yet)
Thank you!
comment:134 Changed 17 months ago by
 Status changed from needs_review to positive_review
comment:135 Changed 17 months ago by
 Reviewers set to Erik Bray, Dima Pasechnik
comment:136 in reply to: ↑ 116 Changed 17 months ago by
comment:137 Changed 17 months ago by
 Branch changed from public/ticket26856 to 8bf1044efda750d8a767d88aa6a2afda69edadbe
 Resolution set to fixed
 Status changed from positive_review to closed
Last 10 new commits:
this file is no longer needed since I'm patching GAP with this functionality instead
speed up this code up a little bit
use ViewObj instead of ViewString
use more GAP_Enter() and GAP_Leave() where it seems appropriate
fix some tests whose output changed since
fix needed due to changes in record keys
minor test fixes and cleanup
use changesignal to restore SIGCHLD and SIGINT handling
remove this debug function for now
fix doctests changed due to GAP formatting etc changes