Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix IsomorphismPermGroupOrFailFpGroup to honor its second argument, which limits the coset table size that gets used before it gives up #5900

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 18 additions & 5 deletions lib/grpfp.gi
Original file line number Diff line number Diff line change
Expand Up @@ -3899,15 +3899,20 @@
#M Size( <G> ) . . . . . . . . . . . . . size of a finitely presented group
##
BindGlobal("SIZE_FP_FROM_CYCLIC_INDEX",
function( G )
function( G, max... ) # max = maximal coset table length required
local fgens, # generators of the free group
rels, # relators of <G>
H, # subgroup of <G>
gen, # generator of cyclic subgroup
max, # maximal coset table length required
e,
T; # coset table of <G> by <H>

if Length(max) = 0 then
max := infinity;
else
max := max[1];
fi;

fgens := FreeGeneratorsOfFpGroup( G );
rels := RelatorsOfFpGroup( G );

Expand All @@ -3926,8 +3931,13 @@
fi;
# the group could be quite big -- try to find a cyclic subgroup of
# finite index.
gen:=FinIndexCyclicSubgroupGenerator(G,infinity);
max:=gen[2];
gen:=FinIndexCyclicSubgroupGenerator(G,max);
if gen = fail then
return fail;

Check warning on line 3936 in lib/grpfp.gi

View check run for this annotation

Codecov / codecov/patch

lib/grpfp.gi#L3936

Added line #L3936 was not covered by tests
fi;
if max = infinity then
max:=gen[2];
fi;
gen:=gen[1];

H := Subgroup(G,[gen]);
Expand Down Expand Up @@ -4049,7 +4059,10 @@

H:=[]; # indicate pseudo-size 0
if not HasSize(G) then
sz:=SIZE_FP_FROM_CYCLIC_INDEX(G);
sz:=SIZE_FP_FROM_CYCLIC_INDEX(G, max);
if sz = fail then
return fail;

Check warning on line 4064 in lib/grpfp.gi

View check run for this annotation

Codecov / codecov/patch

lib/grpfp.gi#L4064

Added line #L4064 was not covered by tests
fi;
SetSize(G,sz);
fi;
if Size(G)=infinity then
Expand Down
11 changes: 11 additions & 0 deletions tst/testbugfix/2024-07-29-fpgroup-enum.tst
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
# Sometimes GAP was able to compute the size of an fp group but then
# for any further action failed to compute a permutation representation.
# See https://github.com/gap-system/gap/issues/5764 for the report,
# and https://github.com/gap-system/gap/pull/5770 for the fix.
gap> f := FreeGroup("a","b","c");;
gap> g := f / [ f.1*f.1*f.1,f.1*f.2*f.3*f.1*f.3^-1*f.2*f.3*f.3,f.1*f.3*f.2^-1 ];
<fp group on the generators [ a, b, c ]>
gap> Size(g);
84
gap> IdGroup(g);
[ 84, 1 ]
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
# IsomorphismPermGroupOrFailFpGroup ignored its second argument
# which is supposed to limit the number of cosets that get defined
# before it gives up
gap> F:=FreeGroup(2);;G:=F/[F.1^2, F.2^2];;
gap> IsomorphismPermGroupOrFailFpGroup(G, 100);
fail
Loading