#26889 closed defect (fixed)
use libGAP in MatrixGroup.as_permutation_group()
Reported by:  dimpase  Owned by:  

Priority:  major  Milestone:  sage8.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: 
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
 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
comment:2 Changed 3 years ago by
 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): 613 613 iso=self._libgap_().IsomorphismPermGroup() 614 614 if algorithm == "smaller": 615 615 iso=iso.Image().SmallerDegreePermutationRepresentation() 616 PG = PermutationGroup( map(gap, iso.Image().GeneratorsOfGroup()), \616 PG = PermutationGroup(iso.Image().GeneratorsOfGroup().sage(), \ 617 617 canonicalize=False) # applying gap()  as PermutationGroup is not libGAP 618 618 619 619 def permutation_group_map(element): 620 return iso.ImageElm(element.gap())._gap_()620 return PG(iso.ImageElm(element.gap()).sage()) 621 621 622 622 from sage.categories.homset import Hom 623 623 self._permutation_group_morphism = Hom(self, PG)(permutation_group_map) 624 624
comment:3 Changed 3 years ago by
 Commit changed from db3f63ef80152cafa36dce38fe49e97bf0df3f6e to 7dd5fa67e041e5b6d3f4e5046a484333a7e8d9d7
comment:4 Changed 3 years ago by
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 followup: ↓ 6 Changed 3 years ago by
 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
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, sayGroupMixinLibGAP
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
 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
 Milestone changed from sage8.5 to sage8.6
This tickets were closed as fixed after the Sage 8.5 release.
comment:9 Changed 3 years ago by
 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
There aren't any obvious Python 3isms 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
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: <builtin 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: <builtin 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: <builtin 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: <builtin 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: <builtin 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: <builtin 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: <builtin 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: <builtin 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/sitepackages/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/sitepackages/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/sitepackages/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/sitepackages/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
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:
more libGAP in as_permutation(), less pexpect mess