Skip to content

Commit

Permalink
Fix PQuotient error for large groups.
Browse files Browse the repository at this point in the history
- Return fail in AbelianPQuotient if not enough generators for current
parameters (logord in PQuotient, or #gens in collector of QuotientSystem
qs)
- Return fail or error in PQuotient for large groups, depending on option
noninteractive
- Update documentation of PQuotient
- Add test for bugfix
  • Loading branch information
FriedrichRober committed Oct 15, 2024
1 parent 520a378 commit 2087635
Show file tree
Hide file tree
Showing 3 changed files with 47 additions and 9 deletions.
13 changes: 7 additions & 6 deletions lib/pquot.gd
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,11 @@ DeclareGlobalFunction( "AbelianPQuotient" );

#############################################################################
##
#F PQuotient(<F>, <p>[, <c>][, <logord>][, <ctype>]) . . pq of an fp group
#F PQuotient(<F>, <p>[, <c>][, <logord>][, <ctype>] : noninteractive) . . pq of an fp group
##
## <#GAPDoc Label="PQuotient">
## <ManSection>
## <Func Name="PQuotient" Arg='F, p[, c][, logord][, ctype]'/>
## <Func Name="PQuotient" Arg='F, p[, c][, logord][, ctype] : noninteractive'/>
##
## <Description>
## computes a factor <A>p</A>-group of a finitely presented group <A>F</A>
Expand Down Expand Up @@ -61,10 +61,11 @@ DeclareGlobalFunction( "AbelianPQuotient" );
## most <M>p^{256}</M>. If the parameter <A>logord</A> is present, it will
## compute with factor groups of order at most <M>p^{<A>logord</A>}</M>.
## If this parameter is specified, then the parameter <A>c</A> must also be
## given. The present
## implementation produces an error message if the order of a
## <M>p</M>-quotient exceeds <M>p^{256}</M> or <M>p^{<A>logord</A>}</M>,
## respectively.
## given. If the order of a <M>p</M>-quotient exceeds <M>p^{256}</M>
## or <M>p^{<A>logord</A>}</M>,
## respectively, the behaviour of the algorithm depends on the option
## <A>noninteractive</A>: if it is present, the current implementation
## produces an error message; otherwise it returns <C>fail</C>.
## Note that the order of intermediate <M>p</M>-groups may be larger than
## the final order of a <M>p</M>-quotient.
## <P/>
Expand Down
27 changes: 24 additions & 3 deletions lib/pquot.gi
Original file line number Diff line number Diff line change
Expand Up @@ -1310,7 +1310,9 @@ end );

#############################################################################
##
#F AbelianPQuotient . . . . . . . . . . . initialize an abelian p-quotient
#F AbelianPQuotient . . . . . . try to initialize an abelian p-quotient
## . . . . . . return true if we are sucessful
## . . . . . . return false otherwise
##
InstallGlobalFunction( AbelianPQuotient,
function( qs )
Expand Down Expand Up @@ -1342,6 +1344,9 @@ function( qs )
generators := DifferenceLists( [1..n], trailers );

## Their images are the first d generators.
if Length(gens) < d then
return false;

Check warning on line 1348 in lib/pquot.gi

View check run for this annotation

Codecov / codecov/patch

lib/pquot.gi#L1348

Added line #L1348 was not covered by tests
fi;
qs!.images{ generators } := gens{[1..d]};

## Fix their definitions.
Expand All @@ -1367,6 +1372,8 @@ function( qs )
qs!.collector![SCP_WEIGHTS]{[1..qs!.numberOfGenerators]} :=
[1..qs!.numberOfGenerators] * 0 + 1;

return true;

end );

#############################################################################
Expand All @@ -1376,7 +1383,8 @@ end );
InstallGlobalFunction( PQuotient,
function( arg )

local G, p, cl, ngens, collector, qs, t,noninteractive;
local G, p, cl, ngens, collector, qs, t,noninteractive,
isAbelianPQuotientSucessful;


## First we parse the arguments to this function
Expand Down Expand Up @@ -1453,7 +1461,20 @@ function( arg )
LengthOfDescendingSeries(qs)+1, " quotient" );

t := Runtime();
AbelianPQuotient( qs );
isAbelianPQuotientSucessful := AbelianPQuotient( qs );
if not isAbelianPQuotientSucessful then
if noninteractive then
return fail;

Check warning on line 1467 in lib/pquot.gi

View check run for this annotation

Codecov / codecov/patch

lib/pquot.gi#L1467

Added line #L1467 was not covered by tests
else
Error( "Collector not large enough ",
"to define generators for abelian p-quotient.\n",
"To return the current quotient (of class ",
LengthOfDescendingSeries(qs), ") type `return;' ",
"and `quit;' otherwise.\n" );

Check warning on line 1473 in lib/pquot.gi

View check run for this annotation

Codecov / codecov/patch

lib/pquot.gi#L1469-L1473

Added lines #L1469 - L1473 were not covered by tests

return qs;
fi;

Check warning on line 1476 in lib/pquot.gi

View check run for this annotation

Codecov / codecov/patch

lib/pquot.gi#L1475-L1476

Added lines #L1475 - L1476 were not covered by tests
fi;

Info( InfoQuotientSystem, 1, " rank of this layer: ",
RanksOfDescendingSeries(qs)[LengthOfDescendingSeries(qs)],
Expand Down
16 changes: 16 additions & 0 deletions tst/testbugfix/2024-10-15-PQuotient.tst
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
# see <https://github.com/gap-system/gap/issues/5809>
gap> F := FreeGroup(["a", "b"]);;
gap> a := F.1;;
gap> b := F.1;;
gap> p := 5;;
gap> G := F / [a^p, b^p, Comm(a,b)];;
gap> PQuotient(G, p, 1, 2 : noninteractive) <> fail;
true
gap> PQuotient(G, p, 1, 1 : noninteractive) = fail;
true

#
gap> PQuotient( FreeGroup(2), 5, 10, 520 : noninteractive ) <> fail;
true
gap> gPQuotient( FreeGroup(2), 5, 10, 519 : noninteractive ) = fail;
true

0 comments on commit 2087635

Please sign in to comment.