use libGAP in MatrixGroup.as_permutation_group() #26889

dimpase · 2018-12-13T12:14:00Z

Currently, one uses pexpect interface there to convert a libGAP matrix group to strings, feed it to GAP's pexpect interface, etc.

This ticket will streamline this. One still will need one conversion to pexpect GAP, at the end, to feed it to PermutationGroup().

Component: group theory

Author: Dima Pasechnik

Branch: 7dd5fa6

Reviewer: Sebastian Oehms

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

The text was updated successfully, but these errors were encountered:

dimpase · 2018-12-13T16:11:54Z

comment:1

I am marking this as a defect, as without this change some tests mysteriously fail on a new libgap interface, see #22626 and #26856.

New commits:

`db3f63e`	`more libGAP in as_permutation(), less pexpect mess`

dimpase · 2018-12-13T16:11:54Z

Commit: db3f63e

dimpase · 2018-12-13T16:11:54Z

Branch: u/dimpase/groups/better_as_permutation

soehms · 2018-12-15T16:37:17Z

comment:2

The following example which worked with the former version, is causing a string buffer overflow, right now:

sage: S = Sp(6,3)
sage: S.as_permutation_group()
Traceback (most recent call last):
...
TypeError: Gap terminated unexpectedly while reading in a large line:

My suggestion to avoid this can be seen from the following diff:

diff --git a/src/sage/groups/matrix_gps/finitely_generated.py b/src/sage/groups/matrix_gps/finitely_generated.py
index 2f5241f..4052740 100644
--- a/src/sage/groups/matrix_gps/finitely_generated.py
+++ b/src/sage/groups/matrix_gps/finitely_generated.py
@@ -613,11 +613,13 @@ class FinitelyGeneratedMatrixGroup_gap(MatrixGroup_gap):
         iso=self._libgap_().IsomorphismPermGroup()
         if algorithm == "smaller":
             iso=iso.Image().SmallerDegreePermutationRepresentation()
-        PG = PermutationGroup(map(gap, iso.Image().GeneratorsOfGroup()), \
+        PG = PermutationGroup(iso.Image().GeneratorsOfGroup().sage(), \
                        canonicalize=False) # applying gap() - as PermutationGroup is not libGAP

         def permutation_group_map(element):
-            return iso.ImageElm(element.gap())._gap_()
+            return PG(iso.ImageElm(element.gap()).sage())

         from sage.categories.homset import Hom
         self._permutation_group_morphism = Hom(self, PG)(permutation_group_map)

soehms · 2018-12-15T16:37:17Z

Reviewer: Sebastian Oehms

sagetrac-git · 2018-12-16T00:20:34Z

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

`2d77eee`	`follow up pyflake hints (all but one)`
`7dd5fa6`	`reviewer's example and fix`

sagetrac-git · 2018-12-16T00:20:34Z

Changed commit from db3f63e to 7dd5fa6

dimpase · 2018-12-16T00:23:19Z

comment:4

the last 2 commits add this fix, the corresponding Sp(6,3) example, and also trims the code following pyflake plugin bot hints.

soehms · 2018-12-16T13:48:32Z

comment:5

Coming back to my last question in comment 15 of #25706: The aim of that question is the following: What does prevent us to move as_permutation_group to, say GroupMixinLibGAP replacing both implementations?

dimpase · 2018-12-16T18:29:24Z

comment:6

Replying to @soehms:

Coming back to my last question in comment 15 of #25706: The aim of that question is the following: What does prevent us to move as_permutation_group to, say GroupMixinLibGAP replacing both implementations?

I don't know. If you can get something converging towards removing pexpect gap here, it would be great.

vbraun · 2018-12-23T23:39:54Z

Changed branch from u/dimpase/groups/better_as_permutation to 7dd5fa6

embray · 2018-12-28T14:06:38Z

comment:8

This tickets were closed as fixed after the Sage 8.5 release.

fchapoton · 2018-12-30T19:38:32Z

comment:9

This is breaking some doctests under python3 in the matrix_gps folder (where all tests were passing).

fchapoton · 2018-12-30T19:38:32Z

Changed commit from 7dd5fa6 to none

embray · 2018-12-31T10:04:11Z

comment:10

There aren't any obvious Python 3-isms in this patch so if that's true that really speaks to the fragility of this code; it looks like it needs quite a bit more cleaning up in general. Please open a new ticket.

fchapoton · 2018-12-31T10:09:47Z

comment:11

traceback:

File "src/sage/groups/matrix_gps/finitely_generated.py", line 445, in sage.groups.matrix_gps.finitely_generated.FinitelyGeneratedMatrixGroup_generic.__reduce__
Failed example:
    loads(dumps(G)) == G
Expected:
    True
Got:
    RuntimeError: Syntax error: ; expected in stream:1
    Symbolic Ring;
     ^^^^^^^^^^^^
    Error, Variable: 'Complex' must have a value
    <BLANKLINE>
    The above exception was the direct cause of the following exception:
    <BLANKLINE>
    Traceback (most recent call last):
      File "sage/libs/gap/util.pyx", line 506, in sage.libs.gap.util.extract_libgap_errout (build/cythonized/sage/libs/gap/util.c:6644)
    SystemError: <built-in method replace of str object at 0x7f1666da6bb0> returned a result with an error set
    Exception ignored in: 'sage.libs.gap.util.error_handler'
    Traceback (most recent call last):
      File "sage/libs/gap/util.pyx", line 506, in sage.libs.gap.util.extract_libgap_errout (build/cythonized/sage/libs/gap/util.c:6644)
    SystemError: <built-in method replace of str object at 0x7f1666da6bb0> returned a result with an error set
    RuntimeError: Syntax error: ; expected in stream:1
    Complex Field with 53 bits of precision;
     ^^^^^^^^^^^^^^^^^^^^
    Error, Variable: 'bits' must have a value
    <BLANKLINE>
    The above exception was the direct cause of the following exception:
    <BLANKLINE>
    Traceback (most recent call last):
      File "sage/libs/gap/util.pyx", line 506, in sage.libs.gap.util.extract_libgap_errout (build/cythonized/sage/libs/gap/util.c:6644)
    SystemError: <built-in method replace of str object at 0x7f16671a8e00> returned a result with an error set
    Exception ignored in: 'sage.libs.gap.util.error_handler'
    Traceback (most recent call last):
      File "sage/libs/gap/util.pyx", line 506, in sage.libs.gap.util.extract_libgap_errout (build/cythonized/sage/libs/gap/util.c:6644)
    SystemError: <built-in method replace of str object at 0x7f16671a8e00> returned a result with an error set
    True
**********************************************************************
File "src/sage/groups/matrix_gps/finitely_generated.py", line 450, in sage.groups.matrix_gps.finitely_generated.FinitelyGeneratedMatrixGroup_generic.__reduce__
Failed example:
    R = MatrixSpace(SR, 2)
Expected nothing
Got:
    RuntimeError: Error, Variable: 'Complex' must have a value
    <BLANKLINE>
    The above exception was the direct cause of the following exception:
    <BLANKLINE>
    Traceback (most recent call last):
      File "sage/libs/gap/util.pyx", line 506, in sage.libs.gap.util.extract_libgap_errout (build/cythonized/sage/libs/gap/util.c:6644)
    SystemError: <built-in method replace of str object at 0x7f1666da6bb0> returned a result with an error set
    Exception ignored in: 'sage.libs.gap.util.error_handler'
    Traceback (most recent call last):
      File "sage/libs/gap/util.pyx", line 506, in sage.libs.gap.util.extract_libgap_errout (build/cythonized/sage/libs/gap/util.c:6644)
    SystemError: <built-in method replace of str object at 0x7f1666da6bb0> returned a result with an error set
    RuntimeError: Syntax error: ; expected in stream:1
    Complex Field with 53 bits of precision;
     ^^^^^^^^^^^^^^^^^^^^
    Error, Variable: 'bits' must have a value
    <BLANKLINE>
    The above exception was the direct cause of the following exception:
    <BLANKLINE>
    Traceback (most recent call last):
      File "sage/libs/gap/util.pyx", line 506, in sage.libs.gap.util.extract_libgap_errout (build/cythonized/sage/libs/gap/util.c:6644)
    SystemError: <built-in method replace of str object at 0x7f16671a8e00> returned a result with an error set
    Exception ignored in: 'sage.libs.gap.util.error_handler'
    Traceback (most recent call last):
      File "sage/libs/gap/util.pyx", line 506, in sage.libs.gap.util.extract_libgap_errout (build/cythonized/sage/libs/gap/util.c:6644)
    SystemError: <built-in method replace of str object at 0x7f16671a8e00> returned a result with an error set
**********************************************************************
File "src/sage/groups/matrix_gps/finitely_generated.py", line 584, in sage.groups.matrix_gps.finitely_generated.FinitelyGeneratedMatrixGroup_gap.as_permutation_group
Failed example:
    G = MatrixGroup(map(MS, GG.GeneratorsOfGroup()))
Exception raised:
    Traceback (most recent call last):
      File "/home/u1/chapoton/sage3/local/lib/python3.6/site-packages/sage/doctest/forker.py", line 671, in _run
        self.compile_and_execute(example, compiler, test.globs)
      File "/home/u1/chapoton/sage3/local/lib/python3.6/site-packages/sage/doctest/forker.py", line 1086, in compile_and_execute
        exec(compiled, globs)
      File "<doctest sage.groups.matrix_gps.finitely_generated.FinitelyGeneratedMatrixGroup_gap.as_permutation_group[21]>", line 1, in <module>
        G = MatrixGroup(map(MS, GG.GeneratorsOfGroup()))
      File "sage/misc/lazy_import.pyx", line 354, in sage.misc.lazy_import.LazyImport.__call__ (build/cythonized/sage/misc/lazy_import.c:3684)
        return self.get_object()(*args, **kwds)
      File "/home/u1/chapoton/sage3/local/lib/python3.6/site-packages/sage/groups/matrix_gps/finitely_generated.py", line 290, in MatrixGroup
        gens = normalize_square_matrices(gens)
      File "/home/u1/chapoton/sage3/local/lib/python3.6/site-packages/sage/groups/matrix_gps/finitely_generated.py", line 131, in normalize_square_matrices
        raise ValueError('list of plain numbers must have square integer length')
    ValueError: list of plain numbers must have square integer length

dimpase added this to the sage-8.5 milestone Dec 13, 2018

dimpase added c: group theory labels Dec 13, 2018

dimpase added s: needs review and removed t: enhancement labels Dec 13, 2018

soehms added s: positive review and removed s: needs review labels Dec 16, 2018

vbraun removed the s: positive review label Dec 23, 2018

vbraun closed this as completed in 6c40fa6 Dec 23, 2018

embray modified the milestones: sage-8.5, sage-8.6 Dec 28, 2018

dimpase mentioned this issue Jul 7, 2021

Upgrade to gap_packages 4.10 and remove database_gap #26856

Closed

dimpase mentioned this issue Mar 2, 2023

convert all of Sage library code to libgap #26902

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

use libGAP in MatrixGroup.as_permutation_group() #26889

use libGAP in MatrixGroup.as_permutation_group() #26889

dimpase commented Dec 13, 2018

dimpase commented Dec 13, 2018

dimpase commented Dec 13, 2018

dimpase commented Dec 13, 2018

soehms commented Dec 15, 2018

soehms commented Dec 15, 2018

sagetrac-git mannequin commented Dec 16, 2018

sagetrac-git mannequin commented Dec 16, 2018

dimpase commented Dec 16, 2018

soehms commented Dec 16, 2018

dimpase commented Dec 16, 2018

vbraun commented Dec 23, 2018

embray commented Dec 28, 2018

fchapoton commented Dec 30, 2018

fchapoton commented Dec 30, 2018

embray commented Dec 31, 2018

fchapoton commented Dec 31, 2018

use libGAP in MatrixGroup.as_permutation_group() #26889

use libGAP in MatrixGroup.as_permutation_group() #26889

Comments

dimpase commented Dec 13, 2018

dimpase commented Dec 13, 2018

dimpase commented Dec 13, 2018

dimpase commented Dec 13, 2018

soehms commented Dec 15, 2018

soehms commented Dec 15, 2018

sagetrac-git mannequin commented Dec 16, 2018

sagetrac-git mannequin commented Dec 16, 2018

dimpase commented Dec 16, 2018

soehms commented Dec 16, 2018

dimpase commented Dec 16, 2018

vbraun commented Dec 23, 2018

embray commented Dec 28, 2018

fchapoton commented Dec 30, 2018

fchapoton commented Dec 30, 2018

embray commented Dec 31, 2018

fchapoton commented Dec 31, 2018