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

Speed up IsRegularPGroup #5797

Merged
merged 1 commit into from
Oct 9, 2024
Merged
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
58 changes: 37 additions & 21 deletions lib/grp.gi
Original file line number Diff line number Diff line change
Expand Up @@ -433,43 +433,44 @@ InstallMethod( IsPowerfulPGroup,
InstallMethod( IsRegularPGroup,
[ IsGroup ],
function( G )
local p, hom, reps, a, b, ap_bp, ab_p, H;
local p, hom, reps, as, a, b, ap, bp, ab, ap_bp, ab_p, g, h, H, N;

if not IsPGroup(G) then
return false;
fi;

p:=PrimePGroup(G);
if p = 2 then
# see [Hup67, Satz 10.3 a)]
# see [Hup67, Satz III.10.3 a)]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good. The previous citation was incomplete. This is correct

return IsAbelian(G);
elif p = 3 and DerivedLength(G) > 2 then
# see [Hup67, Satz 10.3 b)]
# see [Hup67, Satz III.10.3 b)]
return false;
elif Size(G) <= p^p then
# see [Hal34, Corollary 14.14], [Hall, p. 183], [Hup67, Satz 10.2 b)]
# see [Hal34, Corollary 14.14], [Hall, p. 183], [Hup67, Satz III.10.2 b)]
return true;
elif NilpotencyClassOfGroup(G) < p then
# see [Hal34, Corollary 14.13], [Hall, p. 183], [Hup67, Satz 10.2 a)]
# see [Hal34, Corollary 14.13], [Hall, p. 183], [Hup67, Satz III.10.2 a)]
return true;
elif IsCyclic(DerivedSubgroup(G)) then
# see [Hup67, Satz 10.2 c)]
# see [Hup67, Satz III.10.2 c)]
return true;
elif Exponent(G) = p then
# see [Hup67, Satz 10.2 d)]
# see [Hup67, Satz III.10.2 d)]
return true;
elif p = 3 and RankPGroup(G) = 2 then
# see [Hup67, Satz 10.3 b)]: at this point we know that the derived
# subgroup is not cyclic, hence G is not regular
return false;
elif Size(G) < p^p * Size(Agemo(G,p)) then
# see [Hal36, Theorem 2.3], [Hup67, Satz 10.13]
# see [Hal36, Theorem 2.3], [Hup67, Satz III.10.13]
return true;
elif Index(DerivedSubgroup(G),Agemo(DerivedSubgroup(G),p)) < p^(p-1) then
# see [Hal36, Theorem 2.3], [Hup67, Satz 10.13]
# see [Hal36, Theorem 2.3], [Hup67, Satz III.10.13]
return true;
fi;


# Fallback to actually check the defining criterion, i.e.:
# for all a,b in G, we must have that a^p*b^p/(a*b)^p in (<a,b>')^p

Expand All @@ -483,27 +484,42 @@ local p, hom, reps, a, b, ap_bp, ab_p, H;
reps := Filtered(reps, g -> not IsOne(g));
reps := List(reps, g -> PreImagesRepresentative(hom, g));

as := List(reps, a -> [a,a^p]);

for b in Image(hom) do
b := PreImagesRepresentative(hom, b);
for a in reps do
bp := b^p;
for a in as do
ap := a[2]; a := a[1];
# if a and b commute the regularity condition automatically holds
if a*b = b*a then continue; fi;
ab := a*b;
if ab = b*a then continue; fi;

# regularity is also automatic if a^p * b^p = (a*b)^p
ap_bp := a^p * b^p;
ab_p := (a*b)^p;
ap_bp := ap * bp;
ab_p := ab^p;
if ap_bp = ab_p then continue; fi;

# if the subgroup generated H by a and b is itself regular, we are also
# done. However we don't use recursion, here, as H may be equal to G;
# and also we have to be careful to not use too expensive code here.
# But a quick size check is certainly fine.
# done. However we don't use recursion here, as it is too expensive.
# we just check the direct definition, with a twist to avoid Agemo
g := ap_bp / ab_p;
h := Comm(a,b)^p;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good. All the caching is correct and should avoid repeated element calculations

# clearly h is in Agemo(DerivedSubgroup(Group([a,b])), p), so if g=h or
# g=h^-1 then the regularity condition is satisfied
if g = h or IsOne(g*h) then continue; fi;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good. This new check is most advantageous for small p, but should always be cheap (g=h always cheap, g*h should not be expensive). For small p, I suspect it completely avoids the subgroup check in many cases

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you very, very much for you feedback @jackschmidt and sorry for taking so long to get back to you.

Regarding this point, indeed g=h is fast and indeed for small $p$ the cost for checking IsOne(g*h) is absolutely worth it, based on extensive tests with groups of order $3^{10}$ formed as direct products of smaller $3$-groups.

H := Subgroup(G, [a,b]);
if Size(H) <= p^p then continue; fi;

# finally the full check
H := DerivedSubgroup(H);
if not (ap_bp / ab_p) in Agemo(H, p) then
N := NormalClosure(H, [h]);
# To follow the definition of regular precisely we should now check if g
# is in A:=Agemo(DerivedSubgroup(H), p). Clearly h=[a,b]^p and all its
# conjugates are contained in A, and so N is a subgroup of A. But it
# could be a *proper* subgroup. Alas, if G is regular, then also H is
# regular, and from [Hup67, Hauptsatz III.10.5.b)] we conclude A = N and
# the test g in N will succeed. If on the other hand G is not regular,
# then H may also be not regular, and then N might be too small. But
# that is fine (and even beneficial), because that just means we might
# reach the 'return false' faster.
if not g in N then
return false;
fi;
od;
Expand Down
Loading