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

Performance additions to generic 2-cohomology and Automorphism group/Isomorphism test #4219

Merged
merged 7 commits into from
Feb 9, 2021
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
435 changes: 348 additions & 87 deletions lib/autsr.gi

Large diffs are not rendered by default.

19 changes: 16 additions & 3 deletions lib/csetgrp.gi
Original file line number Diff line number Diff line change
Expand Up @@ -330,16 +330,29 @@ local o,b,img,G1,c,m,mt,hardlimit,gens,t,k,intersize;
return fail;
fi;

# old code -- obsolete

c:=ValueOption("refineChainActionLimit");
if IsInt(c) then
hardlimit:=c;
else
hardlimit:=1000000;
fi;

if Index(G,U)>hardlimit then return fail;fi;
if Index(G,U)>hardlimit/10
and ValueOption("callinintermediategroup")<>true then
# try the `AscendingChain` mechanism
c:=AscendingChain(G,U:cheap,refineIndex:=QuoInt(IndexNC(G,U),2),
callinintermediategroup);
if Length(c)>2 then
return First(c,x->Size(x)>Size(U));
fi;
fi;

if Index(G,U)>hardlimit then
Info(InfoWarning,1,
"will have to use permutation action of degree bigger than ", hardlimit);
fi;

# old code -- obsolete

if IsPermGroup(G) and Length(GeneratorsOfGroup(G))>3 then
G1:=Group(SmallGeneratingSet(G));
Expand Down
3 changes: 1 addition & 2 deletions lib/gpfpiso.gi
Original file line number Diff line number Diff line change
Expand Up @@ -1124,7 +1124,6 @@ local iso,fp,n,dec,homs,mos,i,j,ffp,imo,m,k,gens,fm,mgens,rules,

reduce:=function(w)
local red,i,p,pool,wn;
#ow:=w;
w:=LetterRepAssocWord(w);
repeat
i:=1;
Expand Down Expand Up @@ -1288,7 +1287,7 @@ end);

# special method for pc groups, basically just writing down the pc
# presentation
InstallMethod(ConfluentMonoidPresentationForGroup,"generic",
InstallMethod(ConfluentMonoidPresentationForGroup,"pc",
[IsGroup and IsFinite and IsPcGroup],
function(G)
local pcgs,iso,fp,i,j,gens,numi,ord,fm,fam,mword,k,r,addrule,a,e,m;
Expand Down
8 changes: 7 additions & 1 deletion lib/grp.gd
Original file line number Diff line number Diff line change
Expand Up @@ -1035,20 +1035,26 @@ DeclareAttribute( "CommutatorFactorGroup", IsGroup );
#############################################################################
##
#A CompositionSeries( <G> )
#A CompositionSeriesThrough( <G>, <normals> )
##
## <#GAPDoc Label="CompositionSeries">
## <ManSection>
## <Attr Name="CompositionSeries" Arg='G'/>
## <Oper Name="CompositionSeriesThrough" Arg='G, normals'/>
##
## <Description>
## A composition series is a subnormal series which cannot be refined.
## This attribute returns <E>one</E> composition series (of potentially many
## possibilities).
## possibilities). The variant <Ref Oper="CompositionSeriesThrough"/> takes
## as second argument a list <A>normals</A> of normal subgroups of the
## group, and returns a composition series that incorporates these normal
## subgroups.
## </Description>
## </ManSection>
## <#/GAPDoc>
##
DeclareAttribute( "CompositionSeries", IsGroup );
DeclareOperation( "CompositionSeriesThrough", [IsGroup,IsList] );
#T and for module?


Expand Down
58 changes: 58 additions & 0 deletions lib/grp.gi
Original file line number Diff line number Diff line change
Expand Up @@ -939,6 +939,64 @@ InstallMethod( CompositionSeries,
"for simple group", true, [IsGroup and IsSimpleGroup], 100,
S->[S,TrivialSubgroup(S)]);

InstallMethod(CompositionSeriesThrough,"intersection/union",IsElmsColls,
[IsGroup and IsFinite,IsList],0,
function(G,normals)
local cs,i,j,pre,post,c,new,rev;
cs:=CompositionSeries(G);
# find normal subgroups not yet in
normals:=Filtered(normals,x->not x in cs);
# do we satisfy by sheer dumb luck?
if Length(normals)=0 then return cs;fi;

SortBy(normals,x->-Size(x));
# check that this is a valid series
Assert(0,ForAll([2..Length(normals)],i->IsSubset(normals[i-1],normals[i])));
Copy link
Member

Choose a reason for hiding this comment

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

Just out of curiosity, is there a specific reason to use IsSubset instead of IsSubgroup here?

Copy link
Member

Choose a reason for hiding this comment

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

Perhaps there should also be a second assertion (perhaps run only at a higher assertion level) which verifies that normals truly consists of normal subgroups; and/or add a warning to the documentation that passing non-normal subgroups may result in wrong output?


# Now move series through normals by closure/intersection
for j in normals do
# first in cs that does not contain j
pre:=PositionProperty(cs,x->not IsSubset(x,j));
# first contained in j.
post:=PositionProperty(cs,x->Size(j)>=Size(x) and IsSubset(j,x));
Copy link
Member

Choose a reason for hiding this comment

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

Minor optimization possible?

Suggested change
post:=PositionProperty(cs,x->Size(j)>=Size(x) and IsSubset(j,x));
post:=PositionProperty(cs,x->(Size(j) mod Size(x) = 0) and IsSubset(j,x));


# if j is in the series, then pre>post. pre=post impossible
if pre<post then
# so from pre to post-1 needs to be changed
new:=cs{[1..pre-1]};

rev:=[j];
i:=post-1;
repeat
if not IsSubset(rev[Length(rev)],cs[i]) then
c:=ClosureGroup(cs[i],j);
if Size(c)>Size(rev[Length(rev)]) then
# proper down step
Copy link
Member

Choose a reason for hiding this comment

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

Why is adding a larger group a "down step"?

Add(rev,c);
fi;
fi;
i:=i-1;
# at some point this must reach j, then no further step needed
until Size(c)=Size(cs[pre-1]) or i<pre;
Append(new,Filtered(Reversed(rev),x->Size(x)<Size(cs[pre-1])));

i:=pre;
repeat
if not IsSubset(cs[i],new[Length(new)]) then
c:=Intersection(cs[i],j);
if Size(c)<Size(new[Length(new)]) then
# proper down step
Add(new,c);
fi;
fi;
i:=i+1;
until Size(c)=Size(cs[post]);
fi;
cs:=Concatenation(new,cs{[post+1..Length(cs)]});
od;
return cs;
end);


#############################################################################
##
Expand Down
16 changes: 12 additions & 4 deletions lib/grplatt.gd
Original file line number Diff line number Diff line change
Expand Up @@ -449,21 +449,29 @@ DeclareGlobalFunction("LowLayerSubgroups");

#############################################################################
##
#O ContainedConjugates(<G>,<A>,<B>)
#O ContainedConjugates(<G>,<A>,<B>[,<onlyone>])
##
## <#GAPDoc Label="ContainedConjugates">
## <ManSection>
## <Oper Name="ContainedConjugates" Arg='G, A, B'/>
## <Oper Name="ContainedConjugates" Arg='G, A, B [,onlyone]'/>
##
## <Description>
## For <M>A,B \leq G</M> this operation returns representatives of the <A>A</A>
## conjugacy classes of subgroups that are conjugate to <A>B</A> under <A>G</A>.
## The function returns a list of pairs of subgroup and conjugating element.
## If the optional fourth argument <A>onlyone</A> is given as <A>true</A>,
## then only one pair (or <A>fail</A> if none exists) is returned.
Comment on lines +462 to +463
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
## If the optional fourth argument <A>onlyone</A> is given as <A>true</A>,
## then only one pair (or <A>fail</A> if none exists) is returned.
## If the optional fourth argument <A>onlyone</A> is given as <K>true</K>,
## then only one pair (or <K>fail</K> if none exists) is returned.

## <Example><![CDATA[
## gap> g:=SymmetricGroup(8);;
## gap> a:=TransitiveGroup(8,47);;b:=TransitiveGroup(8,7);;
## gap> a:=TransitiveGroup(8,47);;b:=TransitiveGroup(8,9);;
## gap> ContainedConjugates(g,a,b);
## [ [ Group([ (1,4,2,5,3,6,8,7), (1,3)(2,8) ]), (2,4,5,3)(7,8) ] ]
## [ [ Group([ (1,8)(2,3)(4,5)(6,7), (1,3)(2,8)(4,6)(5,7), (1,5)(2,6)(3,7)(4,8),
## (4,5)(6,7) ]), () ],
## [ Group([ (1,8)(2,3)(4,5)(6,7), (1,5)(2,6)(3,7)(4,8), (1,3)(2,8)(4,6)(5,7),
## (2,3)(6,7) ]), (2,4)(3,5) ] ]
## gap> ContainedConjugates(g,a,b,true);
## [ Group([ (1,8)(2,3)(4,5)(6,7), (1,3)(2,8)(4,6)(5,7), (1,5)(2,6)(3,7)(4,8),
## (4,5)(6,7) ]), () ]
## ]]></Example>
## </Description>
## </ManSection>
Expand Down
34 changes: 25 additions & 9 deletions lib/grplatt.gi
Original file line number Diff line number Diff line change
Expand Up @@ -3187,21 +3187,22 @@ local act,offset,G,lim,cond,dosub,all,m,i,j,new,old;
return all;
end);

#############################################################################
##
#F ContainedConjugates( <G>, <A>, <B> )
##
InstallMethod(ContainedConjugates,"finite groups",IsFamFamFam,[IsGroup,IsGroup,IsGroup],0,
function(G,A,B)
local l,N,dc,gens,i;
DoContainedConjugates:=function(arg)
local G,A,B,onlyone,l,N,dc,gens,i;
G:=arg[1];
A:=arg[2];
B:=arg[3];
if Length(arg)>3 then onlyone:=arg[4]; else onlyone:=false;fi;
Comment on lines +3190 to +3195
Copy link
Member

Choose a reason for hiding this comment

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

Could also do this:

Suggested change
DoContainedConjugates:=function(arg)
local G,A,B,onlyone,l,N,dc,gens,i;
G:=arg[1];
A:=arg[2];
B:=arg[3];
if Length(arg)>3 then onlyone:=arg[4]; else onlyone:=false;fi;
DoContainedConjugates:=function(G,A,B,onlyone...)
local l,N,dc,gens,i;
if Length(onlyone)>0 then onlyone:=onlyone[1]; else onlyone:=false;fi;

Copy link
Member

Choose a reason for hiding this comment

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

Or this

Suggested change
DoContainedConjugates:=function(arg)
local G,A,B,onlyone,l,N,dc,gens,i;
G:=arg[1];
A:=arg[2];
B:=arg[3];
if Length(arg)>3 then onlyone:=arg[4]; else onlyone:=false;fi;
DoContainedConjugates:=function(G,A,B,onlyone)
local l,N,dc,gens,i;

and then change one InstallMethod below to use the following function instead: {G,A,B}->DoContainedConjugates(G,A,B,false)


if not IsFinite(G) and IsFinite(A) and IsFinite(B) then
TryNextMethod();
fi;
if not IsSubset(G,A) and IsSubset(G,B) then
Error("A and B must be subgroups of G");
fi;
if Size(A) mod Size(B)<>0 then
return []; # cannot be contained by order
# cannot be contained by order
if onlyone then return fail;else return [];fi;
fi;

l:=[];
Expand All @@ -3211,15 +3212,30 @@ local l,N,dc,gens,i;
gens:=SmallGeneratingSet(B);
for i in dc do
if ForAll(gens,x->x^i[1] in A) then
if onlyone then return [B^i[1],i[1]];fi;
Add(l,[B^i[1],i[1]]);
fi;
od;
if onlyone then return fail;fi;
return l;
elif onlyone then
l:=DoConjugateInto(G,A,B,true);
if IsIdenticalObj(FamilyObj(l),FamilyObj(One(G))) then return [B^l,l];
else return fail;fi;
else
l:=DoConjugateInto(G,A,B,false);
return List(l,x->[B^x,x]);
fi;
end);
end;

#############################################################################
##
#F ContainedConjugates( <G>, <A>, <B> )
##
InstallMethod(ContainedConjugates,"finite groups",IsFamFamFam,
[IsGroup,IsGroup,IsGroup],0,DoContainedConjugates);
InstallOtherMethod(ContainedConjugates,"onlyone",IsFamFamFamX,
[IsGroup,IsGroup,IsGroup,IsBool],0,DoContainedConjugates);

#############################################################################
##
Expand Down
3 changes: 2 additions & 1 deletion lib/morpheus.gi
Original file line number Diff line number Diff line change
Expand Up @@ -2675,7 +2675,8 @@ local m;
# the group is a good part
# sizeable radical
or Size(RadicalGroup(G))^2>Size(G)
or ValueOption("forcetest")=true) then
or ValueOption("forcetest")=true) and
ValueOption("forcetest")<>"old" then
# In place until a proper implementation of Cannon/Holt isomorphism is
# done
return PatheticIsomorphism(G,H);
Expand Down
Loading