Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#26889 closed defect (fixed)

use libGAP in MatrixGroup.as_permutation_group()

Reported by: dimpase Owned by:
Priority: major Milestone: sage-8.6
Component: group theory Keywords:
Cc: Merged in:
Authors: Dima Pasechnik Reviewers: Sebastian Oehms
Report Upstream: N/A Work issues:
Branch: 7dd5fa6 (Commits, GitHub, GitLab) Commit:
Dependencies: Stopgaps:

Status badges

Description

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

Change History (11)

comment:1 Changed 3 years ago by dimpase

  • Branch set to u/dimpase/groups/better_as_permutation
  • Commit set to db3f63ef80152cafa36dce38fe49e97bf0df3f6e
  • Status changed from new to needs_review
  • Type changed from enhancement to defect

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:

db3f63emore libGAP in as_permutation(), less pexpect mess

comment:2 Changed 3 years ago by soehms

  • Reviewers set to Sebastian Oehms

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:

  • src/sage/groups/matrix_gps/finitely_generated.py

    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 b class FinitelyGeneratedMatrixGroup_gap(MatrixGroup_gap): 
    613613        iso=self._libgap_().IsomorphismPermGroup()
    614614        if algorithm == "smaller":
    615615            iso=iso.Image().SmallerDegreePermutationRepresentation()
    616         PG = PermutationGroup(map(gap, iso.Image().GeneratorsOfGroup()), \
     616        PG = PermutationGroup(iso.Image().GeneratorsOfGroup().sage(), \
    617617                       canonicalize=False) # applying gap() - as PermutationGroup is not libGAP
    618618
    619619        def permutation_group_map(element):
    620             return iso.ImageElm(element.gap())._gap_()
     620            return PG(iso.ImageElm(element.gap()).sage())
    621621
    622622        from sage.categories.homset import Hom
    623623        self._permutation_group_morphism = Hom(self, PG)(permutation_group_map)
    624624

comment:3 Changed 3 years ago by git

  • Commit changed from db3f63ef80152cafa36dce38fe49e97bf0df3f6e to 7dd5fa67e041e5b6d3f4e5046a484333a7e8d9d7

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

2d77eeefollow up pyflake hints (all but one)
7dd5fa6reviewer's example and fix

comment:4 Changed 3 years ago by dimpase

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

comment:5 follow-up: Changed 3 years ago by soehms

  • Status changed from needs_review to positive_review

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?

comment:6 in reply to: ↑ 5 Changed 3 years ago by dimpase

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.

comment:7 Changed 3 years ago by vbraun

  • Branch changed from u/dimpase/groups/better_as_permutation to 7dd5fa67e041e5b6d3f4e5046a484333a7e8d9d7
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:8 Changed 3 years ago by embray

  • Milestone changed from sage-8.5 to sage-8.6

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

comment:9 Changed 3 years ago by chapoton

  • Commit 7dd5fa67e041e5b6d3f4e5046a484333a7e8d9d7 deleted

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

comment:10 Changed 3 years ago by embray

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.

comment:11 Changed 3 years ago by chapoton

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
Note: See TracTickets for help on using tickets.