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 PQuotient error for large groups. #5816

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from
Draft
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
29 changes: 17 additions & 12 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 All @@ -57,16 +57,21 @@ DeclareGlobalFunction( "AbelianPQuotient" );
## In case <A>F</A> does not have a largest factor <A>p</A>-group,
## the algorithm will not terminate.
## <P/>
## By default the algorithm computes only with factor groups of order at
## 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.
## Note that the order of intermediate <M>p</M>-groups may be larger than
## the final order of a <M>p</M>-quotient.
## By default the value of the paramter <A>logord</A> is set to <M>256</M>.
## If the parameter <A>logord</A> is specified, then the parameter <A>c</A> must also be
## given.
## The algorithm only computes with factor groups of order
## at most <M>p^{<A>logord</A>}</M>.
## If the order of a factor group exceeds <M>p^{<A>logord</A>}</M>,
## 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, which are constructed by the algorithm
## during the computation, may be larger than the final order of a <M>p</M>-quotient
## in the series. Thus a <M>p</M>-quotient is not necessarily computed
## even if its order does not exceed <M>p^{<A>logord</A>}</M>.
## However, for the first quotient we can guarantee that it is found,
## if the order does not exceed <M>p^{<A>logord</A>}</M>.
## <P/>
## The parameter <A>ctype</A> determines the type of collector that is used
## for computations within the factor <A>p</A>-group.
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 @@

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

## Their images are the first d generators.
if Length(gens) < d then
return false;
fi;
qs!.images{ generators } := gens{[1..d]};

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

return true;

end );

#############################################################################
Expand All @@ -1376,7 +1383,8 @@
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 @@
LengthOfDescendingSeries(qs)+1, " quotient" );

t := Runtime();
AbelianPQuotient( qs );
isAbelianPQuotientSucessful := AbelianPQuotient( qs );
if not isAbelianPQuotientSucessful then
if noninteractive then
return fail;
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;

Check warning on line 1475 in lib/pquot.gi

View check run for this annotation

Codecov / codecov/patch

lib/pquot.gi#L1475

Added line #L1475 was not covered by tests
fi;
fi;

Info( InfoQuotientSystem, 1, " rank of this layer: ",
RanksOfDescendingSeries(qs)[LengthOfDescendingSeries(qs)],
Expand Down
10 changes: 10 additions & 0 deletions tst/testbugfix/2024-10-15-PQuotient.tst
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
# 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
Loading