#26856 closed enhancement (fixed)
Upgrade to gap_packages 4.10 and remove database_gap
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: | |
Branch: | 8bf1044 (Commits, GitHub, GitLab) | Commit: | |
Dependencies: | #22626, #26889, #26965 | Stopgaps: |
Description (last modified by )
After the upgrade to GAP 4.10 in #22626, we revisit what GAP packages are available in Sage via the GAP standard SPKG and via extra optional SPGKs.
All GAP packages formerly in our "database_gap" optional SPKG, except tomlib, now get installed with GAP as part of our "gap" standard SPKG.
This ticket adds tomlib to the "gap" standard SPKG and removes the "database_gap" optional SPKG.
(All packages previously in "database_gap" have GPL-compatible licenses and can therefore be distributed as part of the "gap" standard SPKG without license worries.)
We also update the list of extra GAP packages in gap_packages, which remains a separate and optional SPKG.
Once the present ticket is merged, GAP packages shipped with the "gap" standard SPKG and with the "gap_packages" optional SPKG are as follows.
GAP packages now shipped with "gap" (standard SageMath package)
A basic package used by most GAP packages for their documentation
- GAPDoc
Five database packages formerly in "database_gap"
- PrimGrp
- SmallGrp
- TransGrp
- TomLib
- AtlasRep
Twelve more packages
- AutPGrp
- Alnuth
- CRISP
- !CTblLib
- FactInt
- FGA
- IRREDSOL
- LAGUNA
- Polenta
- Polycyclic
- ResClasses
- Sophus
GAP packages now shipped with "gap_packages" (optional SageMath package)
Twenty-five "pure GAP" packages (not requiring compilation)
- AClib
- AutoDoc
- CoReLG
- CRIME
- Cryst
- CrystCat
- DESIGN
- GBNP
- HAP
- HAPcryst
- hecke
- LieAlgDB
- LiePRing
- LieRing
- loops
- MapClass
- polymaking
- QPA
- QuaGroup
- RadiRoot
- Repsn
- SLA
- SONATA
- Toric
- utils
Four compiled GAP packages
- cohomolo
- GRAPE
- GUAVA
- nq
No longer distributed
- HAPprime: unmaintained, partly merged in HAP, see list of "No longer redistributed with GAP or renamed" GAP packages, source code archived at GitHub
Attachments (1)
Change History (139)
comment:1 Changed 3 years ago by
- Branch set to u/dimpase/GQ
- Commit set to 1ed2d00b452155e803c79040460edcd55c120320
comment:2 Changed 3 years ago by
- Dependencies set to #22626
comment:3 Changed 3 years ago by
Is there a particular reason to merge database_gap
into gap_packages
?
comment:4 follow-up: ↓ 5 Changed 3 years 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 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 3 years ago by
comment:6 Changed 3 years 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 3 years 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 follow-ups: ↓ 9 ↓ 89 Changed 3 years 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/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...)
comment:9 in reply to: ↑ 8 Changed 3 years 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 follow-up: ↓ 12 Changed 3 years 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 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.
comment:11 Changed 3 years 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 libgap-api.h
|
2de776d | Merge branch 'u/embray/spkgs/gap-410' 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/gap-410' 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/interfaces-rand-seed' of trac.sagemath.org:sage into HEAD
|
ef2230a | more libGAP in as_permutation(), less pexpect mess
|
comment:12 in reply to: ↑ 10 Changed 3 years 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 3 years 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 3 years 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 3 years ago by
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.
comment:16 Changed 3 years 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 3 years 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 3 years ago by
- Commit changed from 01665fa57aa56458b2d43765fd17b88944186f23 to 90f2fea00aa8aa088c619ac128faf2e5ec5cecbd
comment:19 Changed 3 years ago by
done getting rid of database_gap
Now on to gap_packages...
comment:20 Changed 3 years ago by
- Dependencies changed from #22626 to #22626, #26889
comment:21 Changed 3 years 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 3 years 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 3 years 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/interfaces-rand-seed' 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/gap-410' of trac.sagemath.org:sage into g410
|
ca7f6ff | Merge branch 'u/dimpase/GQ' of trac.sagemath.org:sage into g410
|
comment:24 Changed 3 years 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 3 years 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 3 years ago by
- Commit changed from d305273fcdd2c247a0c50b8432e2939bc9c6a0f6 to 1b7109983cb030067c6a7e471da076edf8fe6f64
comment:27 Changed 3 years ago by
- Commit changed from 1b7109983cb030067c6a7e471da076edf8fe6f64 to d879657c584c866152fe795888ee7d02c8668f12
comment:28 Changed 3 years 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 3 years 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 3 years 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 3 years 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/gap-410' of trac.sagemath.org:sage into gap410pkg
|
comment:32 Changed 3 years 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 3 years 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 3 years 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 follow-up: ↓ 37 Changed 3 years 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 3 years ago by
- Commit changed from a7ef9e015984e48785b80d95ff422bcd3f6ae232 to 01d85a7b1ebe5b95cb8a84d273dafd13c62f4a65
comment:37 in reply to: ↑ 35 Changed 3 years 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 3 years ago by
I need to adjust formatting in a dozen of tests, after that should be ready for review.
comment:39 Changed 3 years 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 follow-up: ↓ 43 Changed 3 years 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 3 years 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 3 years ago by
I agree about nauty/grape - see #26923
comment:43 in reply to: ↑ 40 ; follow-up: ↓ 44 Changed 3 years 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 3 years 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 3 years 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 follow-up: ↓ 48 Changed 3 years 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 follow-ups: ↓ 49 ↓ 57 Changed 3 years 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 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.
comment:48 in reply to: ↑ 46 Changed 3 years 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 3 years 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 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 3 years ago by
- Commit changed from 01d85a7b1ebe5b95cb8a84d273dafd13c62f4a65 to 78db91a51010662e85dda67649495eaed4193025
comment:51 Changed 3 years ago by
OK, over to you now.
comment:52 Changed 3 years 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 <8-dimensional left-module over QuantumUEA( <root system of type A 2>,1570 Qpar = q )>1567 <8-dimensional left-module 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 3 years 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 3 years ago by
linebreaks are inserted by GAP, it was always the case.
comment:55 Changed 3 years 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 follow-up.
comment:56 Changed 3 years 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 3 years 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 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 3 years 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 follow-up: ↓ 66 Changed 3 years 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 3 years 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_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: ↓ 67 Changed 3 years 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 3 years ago by
GAP's IO is on #26930
comment:63 Changed 3 years 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 3 years ago by
- Status changed from needs_work to needs_review
OK, seems to be ready now
comment:65 Changed 3 years 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 ; follow-ups: ↓ 69 ↓ 72 Changed 3 years 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 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: ↓ 68 Changed 3 years 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 3 years 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 3 years 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 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)
comment:70 follow-up: ↓ 71 Changed 3 years 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 ; follow-up: ↓ 107 Changed 3 years ago by
comment:72 in reply to: ↑ 66 Changed 3 years 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 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 3 years ago by
Let's just make gap_packages standard, as planned anyway. This will alleviate this problem.
comment:74 follow-up: ↓ 76 Changed 3 years 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 3 years 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 re-seed the PRNG before them; let me try.
comment:76 in reply to: ↑ 74 Changed 3 years 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 3 years 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 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 3 years 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 re-seed them before the affected tests (not ideal, but it will ensure consistent results).
comment:79 Changed 3 years ago by
- 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 3 years ago by
your branch does not merge cleanly...
comment:81 Changed 3 years 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 3 years 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 3 years 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/gap-packages/cohomolo/pull/19
|
884bc4f | install deps of polenta, qpa, resclasses, fix test
|
bf2890e | emphasize here that the pacakge's name is 'qpa', not 'qpa-version'
|
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 3 years 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 re-running the full test suite now to see if any other examples like this turn up.
comment:85 Changed 3 years 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 3 years ago by
Oops, forgot to finish the tests for libgap.set_seed.
comment:87 Changed 3 years ago by
- Commit changed from 78859fff771aa376e486d83e1ee4b6ff8ec9afb3 to 728ad7faab01b0839317c3e77acef5c14e1a0ddb
comment:88 Changed 3 years 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 ; follow-up: ↓ 92 Changed 3 years 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 3 years 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 3 years 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 3 years 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 follow-up: ↓ 94 Changed 3 years 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 ; follow-up: ↓ 112 Changed 3 years 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 3 years 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 3 years ago by
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: ↓ 100 Changed 3 years ago by
#22626 breaks some of the # optional - database_gap
doctests. I'll ignore that failure for now but please fix asap.
comment:98 follow-up: ↓ 110 Changed 3 years 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 3 years ago by
- Status changed from needs_work to needs_review
comment:100 in reply to: ↑ 97 Changed 3 years 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 3 years 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 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).
comment:102 Changed 3 years 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 3 years ago by
this commit went AWOL from the current branch. With it in, all tests pass
comment:104 Changed 3 years 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 3 years ago by
One more AWOL commit.
comment:106 follow-up: ↓ 108 Changed 3 years 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 3 years 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 3 years ago by
comment:109 Changed 3 years 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 3 years 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 3 years ago by
- Status changed from needs_review to needs_work
comment:112 in reply to: ↑ 94 Changed 3 years 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 3 years 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 3 years 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 3 years 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 follow-up: ↓ 136 Changed 3 years ago by
- 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 3 years ago by
comment:117 Changed 3 years ago by
I also have the problem from comment:96, on ubuntu 17.10, sage 8.6.beta0. I attached the log.
comment:118 follow-up: ↓ 119 Changed 3 years ago by
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 3 years ago by
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 3 years 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 3 years ago by
- Dependencies changed from #22626, #26889 to #22626, #26889, #26965
comment:122 Changed 3 years 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 'qpa-version'
|
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 3 years ago by
Rebased on 8.6.beta0
comment:124 Changed 3 years ago by
- Commit changed from 293e79ab78bf1574e469ddfa9ff8a93b7087c396 to 9854c3a3a59fd2ab42aacfb78f38346439f280d9
comment:125 Changed 3 years 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 3 years 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 follow-up: ↓ 129 Changed 3 years ago by
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 3 years 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 3 years ago by
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 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 3 years ago by
OK, tests are running, I'll have a look in the morning (I'm on UTC+8 now, sorry :-))
comment:131 Changed 3 years 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 3 years 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 3 years ago by
seems to work here! (same timeouts, because I didn't have the final commit yet)
Thank you!
comment:134 Changed 3 years ago by
- Status changed from needs_review to positive_review
comment:135 Changed 3 years ago by
- Reviewers set to Erik Bray, Dima Pasechnik
comment:136 in reply to: ↑ 116 Changed 3 years ago by
comment:137 Changed 3 years ago by
- Branch changed from public/ticket-26856 to 8bf1044efda750d8a767d88aa6a2afda69edadbe
- Resolution set to fixed
- Status changed from positive_review to closed
comment:138 Changed 11 months ago by
- Commit 8bf1044efda750d8a767d88aa6a2afda69edadbe deleted
- Description modified (diff)
- Summary changed from update gap_packages and database_gap to GAP 4.10 to Upgrade to gap_packages 4.10 and remove database_gap
- Work issues work out where/how to build GAP packages deleted
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