Skip to content

Commit

Permalink
gh-36792: Use the system GAP
Browse files Browse the repository at this point in the history
    
Resurrect #29644 and add an
`spkg-configure.m4` for GAP.

Some other changes were made to make this possible / nicer:

1. `GAP_LIB_DIR` and `GAP_SHARE_DIR` are merged into `GAP_ROOT_PATHS`.
This was suggested by @tornaria and is the right approach, especially
now that `GAP_SO` has been removed. Nothing else needs those two
directories and GAP itself doesn't care about them -- it only cares
about the package search path, i.e. `GAP_ROOT_PATHS`. So changing the
variable(s) brings us in line with the way GAP works.
2. We no longer pass the `-r` flag to GAP. Particularly when using the
system GAP, we do want the user to be able to install his own packages
and run his own initialization.
3. We begin to pass `-A` to GAP. This tells GAP not to autoload the big
set of "recommended" packages at start-up, which avoids the inevitable
error messages when those packages are not installed on the system GAP.
We could check for all of them in `spkg-configure.m4`, but it's a long
list, and loading them slows down GAP initialization for no benefit --
Sage itself uses only one such package. Users can autoload them via
gaprc or gap.ini in light of (2).
4. After adding `-A` to the initialization, we try to load the
PolyCyclic package if it's installed. This is the one "recommended"
package that Sage itself uses.
5. The "recommended" packages are all moved from the gap SPKG to
gap_packages because we no longer need them, except (maybe) for
PolyCyclic. Should keep polycyclic installed by default? The tests all
pass without it, but it is used a few places in sagelib.
6. Various stale doctest fixes.
7. The expected output from a few tests has changed. Where possible,
I've made the test more robust. In one case I had to drop the printing
of a matrix, because if you dig into the source code for GAP's
`NormalSubgroups()`, it chooses a `Representative()` inconsistently, and
that eventually affects the entries of the matrix.

All tests are passing afterwards... except one, a heisenbug:

```
sage -t --warn-long 187.7 --random-
seed=96688270013898650573232209016248663009
src/sage/groups/matrix_gps/finitely_generated_gap.py
**********************************************************************
File "src/sage/groups/matrix_gps/finitely_generated_gap.py", line 123,
in sage.groups.matrix_gps.finitely_generated_gap.FinitelyGeneratedMatrix
Group_gap.as_permutation_group
Failed example:
    P == Psmaller
Expected:
    False
Got:
    True
**********************************************************************
```

If anyone knows what's going on there, I'd be grateful. The GAP docs for
`SmallerDegreePermutationRepresentation()` say,

> The methods used might involve the use of random elements and the
permutation representation (or even the degree of the representation) is
not guaranteed to be the same for different calls of
SmallerDegreePermutationRepresentation.

So maybe this isn't really a bug, but I would find it strange if that
randomness could mean that sometimes the smaller degree option just
wouldn't work at all.

Anyway, have at it, and let me know what you think.
    
URL: #36792
Reported by: Michael Orlitzky
Reviewer(s): Dima Pasechnik, François Bissey, Gonzalo Tornaría, Matthias Köppe, Michael Orlitzky, Tobias Diez
  • Loading branch information
Release Manager committed Dec 17, 2023
2 parents 9deea7b + 404f23a commit d6a2dcc
Show file tree
Hide file tree
Showing 31 changed files with 412 additions and 178 deletions.
1 change: 1 addition & 0 deletions build/pkgs/gap/distros/debian.txt
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
gap
libgap-dev
5 changes: 5 additions & 0 deletions build/pkgs/gap/distros/fedora.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
gap
gap-core
gap-devel
gap-libs
libgap
96 changes: 96 additions & 0 deletions build/pkgs/gap/spkg-configure.m4
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
SAGE_SPKG_CONFIGURE([gap], [
# Default to installing the SPKG, if the check is run at all.
sage_spkg_install_gap=yes
m4_pushdef([GAP_MINVER],["4.12.2"])
SAGE_SPKG_DEPCHECK([ncurses readline zlib], [
AC_PATH_PROG(GAP, gap)
AS_IF([test -n "${GAP}"], [
AC_MSG_CHECKING([for gap version GAP_MINVER or newer])
# GAP will later add the "user" path to the list of root paths
# so long as we don't initialize GAP with -r in Sage. But we
# don't want to include it in the hard-coded list.
GAPRUN="${GAP} -r -q --bare --nointeract -c"
_cmd='Display(GAPInfo.KernelInfo.KERNEL_VERSION);'
GAP_VERSION=$( ${GAPRUN} "${_cmd}" 2>/dev/null )
AX_COMPARE_VERSION(["${GAP_VERSION}"], [ge], [GAP_MINVER], [
AC_MSG_RESULT([yes])
AC_MSG_CHECKING([for gap root paths])
_cmd='Display(JoinStringsWithSeparator(GAPInfo.RootPaths,";"));'
SYS_GAP_ROOT_PATHS=$( ${GAPRUN} "${_cmd}" 2>/dev/null )
AC_MSG_RESULT([$SYS_GAP_ROOT_PATHS])
AS_IF([test -n "${SYS_GAP_ROOT_PATHS}"], [
AC_MSG_CHECKING([for the PrimGrp, SmallGrp, and TransGrp packages])
# Check for a very minimal set of packages without which the
# sage test suite will fail. The crazy thing below is a
# "quadrigraph" for a square bracket.
_cmd="Display(@<:@"
_cmd="${_cmd} TestPackageAvailability(\"PrimGrp\"),"
_cmd="${_cmd} TestPackageAvailability(\"SmallGrp\"),"
_cmd="${_cmd} TestPackageAvailability(\"TransGrp\")"
_cmd="${_cmd} @:>@);"
_output=$( ${GAPRUN} "${_cmd}" 2>/dev/null )
AS_IF([test $? -eq 0], [
AS_CASE([$_output],
[*fail*],[AC_MSG_RESULT([no (at least one package missing)])],[
# default case, i.e. no "fail"
AC_MSG_RESULT([yes])
AC_MSG_CHECKING([if we can link against libgap])
# That was all for the CLI. Now we check for libgap,
# too. There's a long list of headers we need in
# src/sage/libs/gap/gap_includes.pxd, but libgap-api.h
# combined with the version test above should be
# sufficient even on systems where the headers are
# packaged separately.
_old_libs=$LIBS
LIBS="${LIBS} -lgap"
AC_LANG_PUSH([C])
AC_LINK_IFELSE([
AC_LANG_PROGRAM(
[[#include <gap/libgap-api.h>]],
[[
int main(int argc, char** argv) {
GAP_Initialize(0, 0, 0, 0, 0);
return 0;
}
]])
],[
AC_MSG_RESULT([yes])
sage_spkg_install_gap=no
],[
AC_MSG_RESULT([no])
])
AC_LANG_POP
LIBS="${_old_libs}"
])
], [
# The gap command itself failed
AC_MSG_RESULT([no (package check command failed)])
])
])
],[
# Version too old
AC_MSG_RESULT([no])
])
])
])
m4_popdef([GAP_MINVER])
],[],[],[
# This is the post-check phase, where we make sage-conf
# substitutions, in this case of GAP_ROOT_PATHS. We begin with the
# two root paths used by the sage distribution. The '${prefix}' is
# a magic string that sage-conf will replace.
GAP_ROOT_PATHS='${prefix}/lib/gap;${prefix}/share/gap';
AS_IF([test "${sage_spkg_install_gap}" = "no"],[
# If we're using the system GAP, append the system root
# paths to the existing two sage paths.
GAP_ROOT_PATHS="${GAP_ROOT_PATHS};${SYS_GAP_ROOT_PATHS}"
])
AC_SUBST(GAP_ROOT_PATHS, "${GAP_ROOT_PATHS}")
])
33 changes: 15 additions & 18 deletions build/pkgs/gap/spkg-install.in
Original file line number Diff line number Diff line change
Expand Up @@ -16,46 +16,43 @@ if [ "$SAGE_DEBUG" = yes ] ; then
export CFLAGS="-O0 -g3 -DDEBUG_MASTERPOINTERS -DDEBUG_GLOBAL_BAGS -DDEBUG_FUNCTIONS_BAGS $CFLAGS"
fi

# LDFLAGS hack below needed by Semigroups package
sdh_configure $SAGE_CONFIGURE_GMP LDFLAGS="-pthread" --prefix=$SAGE_LOCAL
sdh_configure $SAGE_CONFIGURE_GMP --prefix=$SAGE_LOCAL
sdh_make

sdh_make_install
# sdh_make install-headers install-libgap
# The 'packagemanager' package expects this https://github.com/gap-packages/PackageManager/issues/105
mkdir -p "$SAGE_LOCAL/lib/gap/bin"

# Install only the minimal packages GAP needs to run
sdh_install pkg/gapdoc pkg/primgrp pkg/smallgrp pkg/transgrp "$GAP_ROOT"/pkg

# Install additional packages that are not strictly required, but that are
# typically "expected" to be loaded: These are the default packages that are
# autoloaded at GAP startup (via the PackagesToLoad UserPreference) with an
# out-of-the-box GAP installation; see
# https://github.com/sagemath/sage/issues/22626#comment:393 for discussion on this
#
# Also include atlasrep which is a dependency of tomlib
# Install additional packages that are automatically loaded in the
# default GAP configuration. The list can be found in lib/package.gi
# as part of the "PackagesToLoad" user preference. Also include
# atlasrep because it is a dependency of tomlib.
sdh_install \
pkg/alnuth \
pkg/atlasrep \
pkg/autodoc \
pkg/autpgrp \
pkg/alnuth \
pkg/crisp \
pkg/ctbllib \
pkg/factint \
pkg/fga \
pkg/irredsol \
pkg/laguna \
pkg/packagemanager \
pkg/polenta \
pkg/polycyclic \
pkg/radiroot \
pkg/resclasses \
pkg/sophus \
pkg/tomlib \
pkg/utils \
"$GAP_ROOT"/pkg

# Finally, install packagemanager for the people who reject both
# sage's and their system's package managers. We have to create
# the local bin directory first:
#
# https://github.com/gap-packages/PackageManager/issues/105
#
mkdir -p "$SAGE_LOCAL/lib/gap/bin"
sdh_install pkg/packagemanager "$GAP_ROOT"/pkg

# TODO: This seems unnecessary--we are already installing all of doc/ to
# GAP_ROOT, which is necessary for some functionality in GAP to work. Do
# we need this? Maybe doc/gap could just be a symlink to gap/doc??
Expand Down
14 changes: 14 additions & 0 deletions build/pkgs/gap_packages/SPKG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,20 @@ Description
Several "official" and "undeposited" GAP packages available from
https://www.gap-system.org/Packages/packages.html

Installing this SPKG will install the corresponding GAP packages, but
before you can use them in Sage, they still have to be loaded into
either the GAP interface or libgap::

sage: gap.eval('LoadPackage("Grape")') # optional - gap_packages
'true'
sage: libgap.LoadPackage("Grape") # optional - gap_packages
true

Those correspond to::

gap> LoadPackage("Grape");

within the GAP interface and libgap, respectively.

Upstream Contact
----------------
Expand Down
36 changes: 34 additions & 2 deletions build/pkgs/gap_packages/spkg-install.in
Original file line number Diff line number Diff line change
@@ -1,5 +1,34 @@
GAP_ROOT="$SAGE_LOCAL/lib/gap"
PKG_DIR="$GAP_ROOT/pkg"
# Ask GAP for the directory where sysinfo.gap lives. This is to
# support system GAP installations. This root-path gathering
# command is borrowed from gap's spkg-configure.m4 and modified
# to separate the paths with spaces.
GAPRUN="gap -r -q --bare --nointeract -c"
_cmd='Display(JoinStringsWithSeparator(GAPInfo.RootPaths," "));'
GAP_ROOT_PATHS=$(${GAPRUN} "${_cmd}")

# Loop though GAP_ROOT_PATHS looking for sysinfo.gap
GAP_ROOT=""
for grp in $GAP_ROOT_PATHS; do
if [ -f "${grp}/sysinfo.gap" ]; then
GAP_ROOT=$grp
echo "found GAP root $GAP_ROOT"
break
fi
done

# Try the old sage default if nothing else worked.
if [ -z "$GAP_ROOT" ]; then
GAP_ROOT="$SAGE_LOCAL/lib/gap"
echo "falling back to GAP root $GAP_ROOT"
fi

# And finally, throw an error ASAP if the build is going to fail anyway.
if [ ! -f "${GAP_ROOT}/sysinfo.gap" ]; then
sdh_die "no sysinfo.gap in your gap root"
fi

# Where to install these packages
PKG_DIR="$SAGE_LOCAL/lib/gap/pkg"

PKG_SRC_DIR="$(pwd)/src/pkg"
cd "$PKG_SRC_DIR"
Expand All @@ -12,6 +41,7 @@ cd "$PKG_SRC_DIR"

sdh_install \
aclib \
autodoc \
corelg \
crime \
cryst \
Expand All @@ -31,11 +61,13 @@ sdh_install \
polymaking \
qpa \
quagroup \
radiroot \
repsn \
singular \
sla \
sonata \
toric \
utils \
"$PKG_DIR"

install_compiled_pkg()
Expand Down
5 changes: 5 additions & 0 deletions pkgs/sage-conf/_sage_conf/_conf.py.in
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,11 @@ VERSION = "@PACKAGE_VERSION@"
SAGE_LOCAL = "@prefix@"
SAGE_ROOT = "@SAGE_ROOT@"

# The semicolon-separated list of GAP root paths. This is the list of
# locations that are searched for GAP packages. This is passed directly
# to GAP via the -l flag.
GAP_ROOT_PATHS = "@GAP_ROOT_PATHS@".replace('${prefix}', SAGE_LOCAL)

# The path to the standalone maxima executable.
MAXIMA = "@SAGE_MAXIMA@".replace('${prefix}', SAGE_LOCAL)

Expand Down
4 changes: 3 additions & 1 deletion src/bin/sage
Original file line number Diff line number Diff line change
Expand Up @@ -628,7 +628,9 @@ fi

if [ "$1" = '-gap' -o "$1" = '--gap' ]; then
shift
exec gap "$@"
# Use "-A" to avoid warnings about missing packages. The gap
# interface and libgap within sage both already do this.
exec gap -A "$@"
fi

if [ "$1" = '-gap3' -o "$1" = '--gap3' ]; then
Expand Down
4 changes: 2 additions & 2 deletions src/doc/en/constructions/groups.rst
Original file line number Diff line number Diff line change
Expand Up @@ -180,8 +180,8 @@ Here's another way, working more directly with GAP::
sage: print(gap.eval("normal := NormalSubgroups( G )"))
[ Alt( [ 1 .. 5 ] ), Group(()) ]
sage: G = gap.new("DihedralGroup( 10 )")
sage: G.NormalSubgroups()
[ Group( [ f1, f2 ] ), Group( [ f2 ] ), Group( <identity> of ... ) ]
sage: G.NormalSubgroups().SortedList()
[ Group( <identity> of ... ), Group( [ f2 ] ), Group( [ f1, f2 ] ) ]
sage: print(gap.eval("G := SymmetricGroup( 4 )"))
Sym( [ 1 .. 4 ] )
sage: print(gap.eval("normal := NormalSubgroups( G );"))
Expand Down
12 changes: 5 additions & 7 deletions src/doc/en/prep/Quickstarts/Abstract-Algebra.rst
Original file line number Diff line number Diff line change
Expand Up @@ -83,15 +83,13 @@ rather than just a list of numbers. This can be very powerful.

::

sage: for K in D.normal_subgroups():
sage: len(D.normal_subgroups())
7
sage: for K in sorted(D.normal_subgroups()):
....: print(K)
Subgroup generated by [(1,2,3,4,5,6,7,8), (1,8)(2,7)(3,6)(4,5)] of (Dihedral group of order 16 as a permutation group)
Subgroup generated by [(1,2,3,4,5,6,7,8), (1,3,5,7)(2,4,6,8), (1,5)(2,6)(3,7)(4,8)] of (Dihedral group of order 16 as a permutation group)
Subgroup generated by [(1,3,5,7)(2,4,6,8), (1,5)(2,6)(3,7)(4,8), (1,8)(2,7)(3,6)(4,5)] of (Dihedral group of order 16 as a permutation group)
Subgroup generated by [(2,8)(3,7)(4,6), (1,3,5,7)(2,4,6,8), (1,5)(2,6)(3,7)(4,8)] of (Dihedral group of order 16 as a permutation group)
Subgroup generated by [(1,3,5,7)(2,4,6,8), (1,5)(2,6)(3,7)(4,8)] of (Dihedral group of order 16 as a permutation group)
Subgroup generated by [(1,5)(2,6)(3,7)(4,8)] of (Dihedral group of order 16 as a permutation group)
Subgroup generated by [()] of (Dihedral group of order 16 as a permutation group)
...
Subgroup generated by [(1,2,3,4,5,6,7,8), (1,8)(2,7)(3,6)(4,5)] of (Dihedral group of order 16 as a permutation group)

We can access specific subgroups if we know the generators as a
permutation group.
Expand Down
13 changes: 7 additions & 6 deletions src/sage/env.py
Original file line number Diff line number Diff line change
Expand Up @@ -198,8 +198,7 @@ def var(key: str, *fallbacks: Optional[str], force: bool = False) -> Optional[st
GRAPHS_DATA_DIR = var("GRAPHS_DATA_DIR", join(SAGE_SHARE, "graphs"))
ELLCURVE_DATA_DIR = var("ELLCURVE_DATA_DIR", join(SAGE_SHARE, "ellcurves"))
POLYTOPE_DATA_DIR = var("POLYTOPE_DATA_DIR", join(SAGE_SHARE, "reflexive_polytopes"))
GAP_LIB_DIR = var("GAP_LIB_DIR", join(SAGE_LOCAL, "lib", "gap"))
GAP_SHARE_DIR = var("GAP_SHARE_DIR", join(SAGE_SHARE, "gap"))

COMBINATORIAL_DESIGN_DATA_DIR = var("COMBINATORIAL_DESIGN_DATA_DIR", join(SAGE_SHARE, "combinatorial_designs"))
CREMONA_MINI_DATA_DIR = var("CREMONA_MINI_DATA_DIR", join(SAGE_SHARE, "cremona"))
CREMONA_LARGE_DATA_DIR = var("CREMONA_LARGE_DATA_DIR", join(SAGE_SHARE, "cremona"))
Expand Down Expand Up @@ -243,11 +242,13 @@ def var(key: str, *fallbacks: Optional[str], force: bool = False) -> Optional[st
# GAP memory and args

SAGE_GAP_MEMORY = var('SAGE_GAP_MEMORY', None)
_gap_cmd = "gap -r"
if SAGE_GAP_MEMORY is not None:
_gap_cmd += " -s " + SAGE_GAP_MEMORY + " -o " + SAGE_GAP_MEMORY
SAGE_GAP_COMMAND = var('SAGE_GAP_COMMAND', _gap_cmd)
SAGE_GAP_COMMAND = var('SAGE_GAP_COMMAND', None)

# The semicolon-separated search path for GAP packages. It is passed
# directly to GAP via the -l flag.
GAP_ROOT_PATHS = var("GAP_ROOT_PATHS",
";".join([join(SAGE_LOCAL, "lib", "gap"),
join(SAGE_LOCAL, "share", "gap")]))

# post process
if DOT_SAGE is not None and ' ' in DOT_SAGE:
Expand Down
14 changes: 14 additions & 0 deletions src/sage/ext_data/gap/sage.g
Original file line number Diff line number Diff line change
Expand Up @@ -134,3 +134,17 @@ end;
#
# LogTo("/tmp/gapsage.log");
#


# Load the GAP packages that GAP itself tries to autoload in the
# default configuration (see "PackagesToLoad" in lib/package.gi). The
# combination of passing -A to gap and these LoadPackage statements
# allows us to load the usual set of packages, but only if they are
# installed. So most people will get exactly the default behavior,
# but minimal installations won't throw warnings and fail tests.
_autoloads := [ "autpgrp", "alnuth", "crisp", "ctbllib", "factint", "fga",
"irredsol", "laguna", "polenta", "polycyclic", "resclasses",
"sophus", "tomlib" ];
for p in _autoloads do
LoadPackage(p);
od;
Loading

0 comments on commit d6a2dcc

Please sign in to comment.