#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: |
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 follow-up: ↓ 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 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
- 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 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
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
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