Skip to content

Commit

Permalink
ENHANCE: Reduce impact of immediate methods for groups
Browse files Browse the repository at this point in the history
1) When creating groups, set filters such as `IsEmpty`, `IsFinite` that
always hold, as well as do some very cheap tests for being not trivial to
set filters that have trivial immediate methods associated, thereby avoiding
a trigger of these methods

2) Comment out a few immediate methods that have doubtful value (the trivial
group is perfect, a collection that does not have `IsFinite` has `Size` set
to infinity etc.)

3) When creating groups of automorphisms (or similar objects), explicitly
set `IsFinite` if we know so.

4) A side-effect of this is that group types cannot be cached anymore, as
this type might end up changed in place, inheriting properties it should
not.
  • Loading branch information
hulpke committed Apr 22, 2018
1 parent 69d48c6 commit 0ec2f7f
Show file tree
Hide file tree
Showing 8 changed files with 293 additions and 186 deletions.
19 changes: 11 additions & 8 deletions lib/coll.gi
Original file line number Diff line number Diff line change
Expand Up @@ -169,14 +169,17 @@ InstallMethod( IsWholeFamily,
##
#M Size( <C> ) . . . . . . . . . . . . . . . . . . . . size of a collection
##
InstallImmediateMethod( Size,
IsCollection and HasIsFinite and IsAttributeStoringRep, 0,
function ( C )
if IsFinite( C ) then
TryNextMethod();
fi;
return infinity;
end );
# This method get called for every group that knows it is finite but does
# not know its size -- e.g. permutation, pc
# the benefit OTOH is minimal beyond showing off a feature.
#InstallImmediateMethod( Size,
# IsCollection and HasIsFinite and IsAttributeStoringRep, 0,
# function ( C )
# if IsFinite( C ) then
# TryNextMethod();
# fi;
# return infinity;
# end );

InstallImmediateMethod( Size,
IsCollection and HasAsList and IsAttributeStoringRep, 0,
Expand Down
186 changes: 112 additions & 74 deletions lib/grp.gi
Original file line number Diff line number Diff line change
Expand Up @@ -30,14 +30,15 @@ InstallImmediateMethod( IsFinitelyGeneratedGroup,
##
#M IsCyclic( <G> ) . . . . . . . . . . . . . . . . test if a group is cyclic
##
InstallImmediateMethod( IsCyclic, IsGroup and HasGeneratorsOfGroup, 0,
function( G )
if Length( GeneratorsOfGroup( G ) ) = 1 then
return true;
else
TryNextMethod();
fi;
end );
# this is now set when creating the groups
#InstallImmediateMethod( IsCyclic, IsGroup and HasGeneratorsOfGroup, 0,
# function( G )
# if Length( GeneratorsOfGroup( G ) ) = 1 then
# return true;
# else
# TryNextMethod();
# fi;
# end );

InstallMethod( IsCyclic,
"generic method for groups",
Expand Down Expand Up @@ -433,10 +434,12 @@ InstallMethod( IsNilpotentGroup,
##
#M IsPerfectGroup( <G> ) . . . . . . . . . . . . test if a group is perfect
##
InstallImmediateMethod( IsPerfectGroup,
IsSolvableGroup and HasIsTrivial,
0,
IsTrivial );

# This is now set when the group is created
#InstallImmediateMethod( IsPerfectGroup,
# IsSolvableGroup and HasIsTrivial,
# 0,
# IsTrivial );

InstallImmediateMethod( IsPerfectGroup,
IsGroup and HasIsAbelian and IsSimpleGroup,
Expand Down Expand Up @@ -4291,86 +4294,121 @@ end );
InstallMethod( GroupWithGenerators,
"generic method for collection",
[ IsCollection ],
function( gens )
local G,fam,typ;

fam:=FamilyObj(gens);
if IsFinite(gens) then
if not IsBound(fam!.defaultFinitelyGeneratedGroupType) then
fam!.defaultFinitelyGeneratedGroupType:=
NewType(fam,IsGroup and IsAttributeStoringRep
and HasGeneratorsOfMagmaWithInverses
and IsFinitelyGeneratedGroup);
fi;
typ:=fam!.defaultFinitelyGeneratedGroupType;
else
if not IsBound(fam!.defaultGroupType) then
fam!.defaultGroupType:=
NewType(fam,IsGroup and IsAttributeStoringRep
and HasGeneratorsOfMagmaWithInverses);
function( gens )
local G,fam,typ,id,triv;

fam:=FamilyObj(gens);

triv:=fail; # might not find out

typ:=IsGroup and IsAttributeStoringRep
and HasIsEmpty and HasGeneratorsOfMagmaWithInverses;
if IsFinite(gens) then
typ:=typ and IsFinitelyGeneratedGroup;
if Length(gens)<=1 then
typ:=typ and HasIsCyclic and IsCyclic
and HasIsCommutative and IsCommutative;
fi;

if Length(gens)>0 and CanEasilyCompareElements(gens) then
id:=One(gens[1]);
if ForAny(gens,x->x<>id) then
typ:=typ and HasIsTrivial and HasIsNonTrivial;
triv:=false;
else
typ:=typ and HasIsTrivial and HasIsNonTrivial;
triv:=true;
# basic consequence
typ:=typ and HasIsPerfectGroup and IsPerfectGroup;
fi;
typ:=fam!.defaultGroupType;
fi;

G:=rec();
ObjectifyWithAttributes(G,typ,GeneratorsOfMagmaWithInverses,AsList(gens));
fi;
typ:=NewType(fam,typ);

return G;
end );
G:=rec();
if triv=fail then
ObjectifyWithAttributes(G,typ,GeneratorsOfMagmaWithInverses,AsList(gens),
IsEmpty,false);
else
ObjectifyWithAttributes(G,typ,GeneratorsOfMagmaWithInverses,AsList(gens),
IsEmpty,false,IsTrivial,triv,
IsNonTrivial,not triv);
fi;

return G;
end );

InstallMethod( GroupWithGenerators,
"generic method for collection and identity element",
IsCollsElms, [ IsCollection, IsMultiplicativeElementWithInverse ],
function( gens, id )
local G,fam,typ;

fam:=FamilyObj(gens);
if IsFinite(gens) then
if not IsBound(fam!.defaultFinitelyGeneratedGroupWithOneType) then
fam!.defaultFinitelyGeneratedGroupWithOneType:=
NewType(fam,IsGroup and IsAttributeStoringRep
and HasGeneratorsOfMagmaWithInverses
and IsFinitelyGeneratedGroup and HasOne);
fi;
typ:=fam!.defaultFinitelyGeneratedGroupWithOneType;
else
if not IsBound(fam!.defaultGroupWithOneType) then
fam!.defaultGroupWithOneType:=
NewType(fam,IsGroup and IsAttributeStoringRep
and HasGeneratorsOfMagmaWithInverses and HasOne);
function( gens, id )
local G,fam,typ,triv;

fam:=FamilyObj(gens);
triv:=fail; # might not find out

typ:=IsGroup and IsAttributeStoringRep
and HasIsEmpty and HasGeneratorsOfMagmaWithInverses and HasOne;
if IsFinite(gens) then
typ:=typ and IsFinitelyGeneratedGroup;
if Length(gens)<=1 then
typ:=typ and HasIsCyclic and IsCyclic
and HasIsCommutative and IsCommutative;
fi;

if Length(gens)>0 and CanEasilyCompareElements(gens) then
if ForAny(gens,x->x<>id) then
typ:=typ and HasIsTrivial and HasIsNonTrivial;
triv:=false;
else
typ:=typ and HasIsTrivial and HasIsNonTrivial;
triv:=true;
# basic consequence
typ:=typ and HasIsPerfectGroup and IsPerfectGroup;
fi;
typ:=fam!.defaultGroupWithOneType;
fi;

G:=rec();
fi;
typ:=NewType(fam,typ);

G:=rec();
if triv=fail then
ObjectifyWithAttributes(G,typ,GeneratorsOfMagmaWithInverses,AsList(gens),
One,id);
IsEmpty,false,One,id);
else
ObjectifyWithAttributes(G,typ,GeneratorsOfMagmaWithInverses,AsList(gens),
IsEmpty,false,One,id,
IsTrivial,triv,IsNonTrivial,not triv);
fi;

return G;
return G;
end );

InstallMethod( GroupWithGenerators,
"method for empty list and element",
[ IsList and IsEmpty, IsMultiplicativeElementWithInverse ],
function( empty, id )
local G,fam,typ;
InstallMethod( GroupWithGenerators,"method for empty list and element",
[ IsList and IsEmpty, IsMultiplicativeElementWithInverse ],
function( empty, id )
local G,fam,typ;

fam:= CollectionsFamily( FamilyObj( id ) );
if not IsBound( fam!.defaultFinitelyGeneratedGroupWithOneType ) then
fam!.defaultFinitelyGeneratedGroupWithOneType:=
NewType( fam, IsGroup and IsAttributeStoringRep
and HasGeneratorsOfMagmaWithInverses
and IsFinitelyGeneratedGroup and HasOne );
fi;
typ:= fam!.defaultFinitelyGeneratedGroupWithOneType;
fam:= CollectionsFamily( FamilyObj( id ) );

G:= rec();
ObjectifyWithAttributes( G, typ,
GeneratorsOfMagmaWithInverses, empty,
One, id );
typ:=IsGroup and IsAttributeStoringRep
and HasGeneratorsOfMagmaWithInverses and HasOne
and IsFinitelyGeneratedGroup and HasIsTrivial and HasIsNonTrivial
and HasIsFinite and HasIsCyclic
and HasIsPerfectGroup and HasIsEmpty;
typ:=NewType(fam,typ);

return G;
end );
G:= rec();
ObjectifyWithAttributes( G, typ,
GeneratorsOfMagmaWithInverses, empty,
One, id,IsEmpty,false,
IsTrivial,true,IsNonTrivial,false,
IsFinite,true,IsCyclic,true,
IsPerfectGroup,true);

return G;
end );


#############################################################################
Expand Down
10 changes: 6 additions & 4 deletions lib/grpfp.gi
Original file line number Diff line number Diff line change
Expand Up @@ -1539,7 +1539,7 @@ end );
#M FactorFreeGroupByRelators(<F>,<rels>) . factor of free group by relators
##
BindGlobal( "FactorFreeGroupByRelators", function( F, rels )
local G, fam, gens;
local G, fam, gens,typ;

# Create a new family.
fam := NewFamily( "FamilyElementsFpGroup", IsElementOfFpGroup );
Expand All @@ -1549,12 +1549,14 @@ BindGlobal( "FactorFreeGroupByRelators", function( F, rels )

fam!.freeGroup := F;
fam!.relators := Immutable( rels );
typ:=IsSubgroupFpGroup and IsWholeFamily and IsAttributeStoringRep;
if IsFinitelyGeneratedGroup(F) then
typ:=typ and IsFinitelyGeneratedGroup;
fi;

# Create the group.
G := Objectify(
NewType( CollectionsFamily( fam ),
IsSubgroupFpGroup and IsWholeFamily and IsAttributeStoringRep ),
rec() );
NewType( CollectionsFamily( fam ), typ ), rec() );

# Mark <G> to be the 'whole group' of its later subgroups.
FamilyObj( G )!.wholeGroup := G;
Expand Down
Loading

0 comments on commit 0ec2f7f

Please sign in to comment.