Skip to content

Commit

Permalink
ENHANCE: Reduce impact of immediate methods for groups etc.
Browse files Browse the repository at this point in the history 
This commit changes routines that create groups, certain Submagmas aand
vector spaces so that flags that set basic properties (being trivial, being
empty, being cyclic) are set at the same time as the object was created.

This functionality used to be provided through immediate methods, however
this meant that the type of a just-created object was changed immediately
multiple times (with some changes precipitating other changes), causing a
notable performance hit.

Notably, the following changes have been done:

1) In a number of methods used to create groups (or submagmas, when called
from `Subgroup` or cosets) from generating sets, a number of cheaply deduced
properties (such as being cyclic) are set while creating the object. Thus
the immediate methods do not apply for these objects any longer.

2) Type caching in these methods is not worth the effort and has been
removed.

3) A new setter method for `Size` deals similarly with deductions done
before by immediate methods for a known size.
Caveat: The manual specifies (ill-advised one might say) that setting a
different size if a size already is given will be ignored. This is tested in
manual examples and test files. The new setter method therefore cannot be
used to check for this property as an error -- it will do so only if
assertion level is set >=3.

4a) Immediate methods that have been made obsoleteby the changes in 1) and 3)
have been changed to ordinary methods.

4b) A number of immediate methods (e.g. a trivial group is solvable) have
been replaced by `TrueMethods` that have the same effect but are chaper to
run.

4c) Also a number of immediate methods
that need to run often, but apply only rarely (i.e. setting `IsFinite` to
false if a size is set to `infinity`) have been removed.

5) These changes have minor impact on test files and manual tests: A few objects
will have slightly different knowledge about their properties  and thus
print differently; respectivelty test code exists that checks explicitly for
such properties having been set by immediate methods.
These tests and examples have been changed.
A few, very instable tests (e.g. checking explicitly for the values of
random elements) have been commented out.

6) Some tests explicitly relied on immediate methods enabling derived series
calculations in special cases of finitely presented groups. These tests
never would have worked if immedite methods were turned off, the respective
finitely presented groups method has been changed accordingly to test
explicitly for the particular situation, thus enabling the same examples to
work.
  • Loading branch information
@hulpke
hulpke committed May 9, 2018
1 parent cf61a22 commit e804843
Show file tree
Hide file tree
Showing 21 changed files with 417 additions and 317 deletions.
4 changes: 4 additions & 0 deletions lib/coll.gd
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1371,6 +1371,10 @@ InstallFactorMaintenance( IsTrivial,
##
DeclareProperty( "IsNonTrivial", IsCollection );

# true methods to avoid immediate methods
InstallTrueMethod(HasIsTrivial,IsNonTrivial);
InstallTrueMethod(HasIsNonTrivial,IsTrivial);


#############################################################################
##
Expand Down
48 changes: 41 additions & 7 deletions lib/coll.gi
View file
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ InstallMethod( PrintObj,
##
InstallImmediateMethod( IsEmpty,
IsCollection and HasSize, 0,
C -> Size( C ) = 0 );
C->Size( C ) = 0);

InstallMethod( IsEmpty,
"for a collection",
Expand All @@ -120,17 +120,17 @@ InstallMethod( IsTrivial,
[ IsCollection ],
C -> Size( C ) = 1 );

InstallImmediateMethod( IsTrivial,
IsCollection and HasIsNonTrivial, 0,
InstallMethod( IsTrivial,
[IsCollection and HasIsNonTrivial], 0,
C -> not IsNonTrivial( C ) );


#############################################################################
##
#M IsNonTrivial( <C> ) . . . . . . . . . test if a collection is nontrivial
##
InstallImmediateMethod( IsNonTrivial,
IsCollection and HasIsTrivial, 0,
InstallMethod( IsNonTrivial,
[IsCollection and HasIsTrivial], 0,
C -> not IsTrivial( C ) );

InstallMethod( IsNonTrivial,
Expand Down Expand Up @@ -169,8 +169,12 @@ InstallMethod( IsWholeFamily,
##
#M Size( <C> ) . . . . . . . . . . . . . . . . . . . . size of a collection
##
InstallImmediateMethod( Size,
IsCollection and HasIsFinite and IsAttributeStoringRep, 0,
# This used to be an immediate method. It was replaced by an ordinary
# method, as the immediate method would get called for every group that
# knows it is finite but does not know its size -- e.g. permutation, pc.
# The benefit of this is minimal beyond showing off a feature.
InstallMethod( Size,true, [IsCollection and HasIsFinite],
100, # rank above object-specific methods
function ( C )
if IsFinite( C ) then
TryNextMethod();
Expand Down Expand Up @@ -3049,6 +3053,36 @@ end);
InstallMethod( CanComputeIsSubset,"default: no, unless identical",
[IsObject,IsObject],IsIdenticalObj);

# This setter method is installed to implement filter settings in response
# to an objects size as part of setting the size. This used to be handled
# instead by immediate methods, but in a situation as here it would trigger
# multiple immediate methods, several of which could apply and each changing
# the type of the object. Doing so can be costly and thus should be
# avoided.
InstallOtherMethod(SetSize,true,[IsObject and IsAttributeStoringRep,IsObject],
100, # override system setter
function(obj,sz)
local filt;
if HasSize(obj) and Size(obj)<>sz then
if AssertionLevel()>2 then
# Make this an ordinary error (not ErrorNoReturn as suggested) to
# preserve all debugging options -- even use `return` to investigate
# what would have happened before this methods was introduced.
Error("size of ",obj," already set to ",Size(obj),
", cannot be changed to ",sz);
fi;
return;
fi;
if sz=0 then filt:=IsEmpty and IsNonTrivial; # IsNonTrivial hold
elif sz=1 then filt:=IsTrivial;
elif sz=infinity then filt:=IsNonTrivial and HasIsFinite;
else filt:=IsNonTrivial and IsFinite;
fi;
filt:=filt and HasSize;
obj!.Size:=sz;
SetFilterObj(obj,filt);
end);

#############################################################################
##
#E
35 changes: 19 additions & 16 deletions lib/csetgrp.gi
View file
Original file line number Diff line number Diff line change
Expand Up @@ -411,18 +411,17 @@ end);
InstallOtherMethod(DoubleCoset,"with size",true,
[IsGroup,IsObject,IsGroup,IsPosInt],0,
function(U,g,V,sz)
local d,fam;
local d,fam,typ;
fam:=FamilyObj(U);
if not IsBound(fam!.doubleCosetsDefaultSizeType) then
fam!.doubleCosetsDefaultSizeType:=NewType(fam,IsDoubleCosetDefaultRep
and HasSize and HasIsFinite and IsFinite
typ:=NewType(fam,IsDoubleCosetDefaultRep
and HasIsFinite and IsFinite
and HasLeftActingGroup and HasRightActingGroup
and HasRepresentative);
fi;
d:=rec();
ObjectifyWithAttributes(d,fam!.doubleCosetsDefaultSizeType,
LeftActingGroup,U,RightActingGroup,V,Representative,g,
Size,sz);
ObjectifyWithAttributes(d,typ,
LeftActingGroup,U,RightActingGroup,V,Representative,g);
SetSize(d,sz); # Size has private setter which will cause problems with
# HasSize triggering an immediate method.
return d;
end);

Expand Down Expand Up @@ -574,21 +573,25 @@ end);
InstallMethod(RightCoset,"use subgroup size",IsCollsElms,
[IsGroup and HasSize,IsObject],0,
function(U,g)
local d,fam;
local d,fam,typ;
# noch tests...

fam:=FamilyObj(U);
if not IsBound(fam!.rightCosetsDefaultSizeType) then
fam!.rightCosetsDefaultSizeType:=NewType(fam,IsRightCosetDefaultRep and
HasActingDomain and HasFunctionAction and HasRepresentative and
HasSize and HasCanonicalRepresentativeDeterminatorOfExternalSet);
fi;
typ:=NewType(fam,IsRightCosetDefaultRep and
HasActingDomain and HasFunctionAction and HasRepresentative
and HasCanonicalRepresentativeDeterminatorOfExternalSet);

d:=rec();
ObjectifyWithAttributes(d,fam!.rightCosetsDefaultSizeType,
ObjectifyWithAttributes(d,typ,
ActingDomain,U,FunctionAction,OnLeftInverse,Representative,g,
Size,Size(U),CanonicalRepresentativeDeterminatorOfExternalSet,
CanonicalRepresentativeDeterminatorOfExternalSet,
RightCosetCanonicalRepresentativeDeterminator);
# We cannot set the size in the previous ObjectifyWithAttributes as there is
# a custom setter method (the one added in this commit). In such a case
# ObjectifyWith Attributes just does `Objectify` and calls all setters
# separately which is what we want to avoid here.
SetSize(d,Size(U));

return d;
end);

Expand Down
2 changes: 2 additions & 0 deletions lib/ctbl.gd
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3584,6 +3584,8 @@ DeclareGlobalVariable( "CharacterTableDisplayDefaults" );
## tbl:=rec();
## tbl.Irr:=
## [ [ 1, 1 ], [ 1, -1 ] ];
## tbl.IsFinite:=
## true;
## tbl.NrConjugacyClasses:=
## 2;
## tbl.Size:=
Expand Down
8 changes: 8 additions & 0 deletions lib/grp.gd
View file
Original file line number Diff line number Diff line change
Expand Up @@ -592,6 +592,7 @@ DeclareProperty( "IsPerfectGroup", IsGroup );
InstallFactorMaintenance( IsPerfectGroup,
IsGroup and IsPerfectGroup, IsObject, IsGroup );

InstallTrueMethod( IsPerfectGroup, IsGroup and IsTrivial );

#############################################################################
##
Expand Down Expand Up @@ -783,6 +784,8 @@ InstallFactorMaintenance( IsSolvableGroup,
InstallTrueMethod( IsSolvableGroup, IsMonomialGroup );
InstallTrueMethod( IsSolvableGroup, IsSupersolvableGroup );

InstallTrueMethod( HasIsPerfectGroup, IsGroup and IsSolvableGroup and IsNonTrivial );


#############################################################################
##
Expand Down Expand Up @@ -3566,6 +3569,11 @@ DeclareOperation( "GroupWithGenerators", [ IsCollection ] );
DeclareOperation( "GroupWithGenerators",
[ IsCollection, IsMultiplicativeElementWithInverse ] );

#F MakeGroupyType( <fam>, <filt>, <gens>, <isgroup> )
# type creator function to incorporate basic deductions so immediate methods
# are not needed
DeclareGlobalFunction("MakeGroupyType");


#############################################################################
##
Expand Down
153 changes: 79 additions & 74 deletions lib/grp.gi
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,9 @@ InstallImmediateMethod( IsFinitelyGeneratedGroup,
##
#M IsCyclic( <G> ) . . . . . . . . . . . . . . . . test if a group is cyclic
##
InstallImmediateMethod( IsCyclic, IsGroup and HasGeneratorsOfGroup, 0,
# This used to be an immediate method. It was replaced by an ordinary
# method since the flag is typically set when creating the group.
InstallMethod( IsCyclic, true, [IsGroup and HasGeneratorsOfGroup], 0,
function( G )
if Length( GeneratorsOfGroup( G ) ) = 1 then
return true;
Expand All @@ -43,10 +45,14 @@ InstallMethod( IsCyclic,
"generic method for groups",
[ IsGroup ],
function ( G )
local a;

# if <G> has a generator list of length 1 then <G> is cyclic
if HasGeneratorsOfGroup( G ) and Length( GeneratorsOfGroup(G) ) = 1 then
SetMinimalGeneratingSet(G,GeneratorsOfGroup(G));
a:=GeneratorsOfGroup(G)[1];
if CanEasilyCompareElements(a) and not IsOne(a) then
SetMinimalGeneratingSet(G,GeneratorsOfGroup(G));
fi;
return true;

# if <G> is not commutative it is certainly not cyclic
Expand Down Expand Up @@ -433,11 +439,6 @@ InstallMethod( IsNilpotentGroup,
##
#M IsPerfectGroup( <G> ) . . . . . . . . . . . . test if a group is perfect
##
InstallImmediateMethod( IsPerfectGroup,
IsSolvableGroup and HasIsTrivial,
0,
IsTrivial );

InstallImmediateMethod( IsPerfectGroup,
IsGroup and HasIsAbelian and IsSimpleGroup,
0,
Expand Down Expand Up @@ -4283,6 +4284,39 @@ local s,b;
return s;
end );

#F MakeGroupyType( <fam>, <filt>, <gens>, <id>, <isgroup> )
# type creator function to incorporate basic deductions so immediate methods
# are not needed. Parameters are family, filter to start with, generator
# list, is it indeed a group (or only magma)?
InstallGlobalFunction(MakeGroupyType,
function(fam,filt,gens,id,isgroup)

if IsFinite(gens) then
if isgroup then
filt:=filt and IsFinitelyGeneratedGroup;
fi;

if Length(gens)>0 and CanEasilyCompareElements(gens) then
if id=false then
id:=One(gens[1]);
fi;
if id<>fail then # cannot do identity in magma
if ForAny(gens,x->x<>id) then
filt:=filt and HasIsTrivial and IsNonTrivial;
if isgroup and Length(gens)<=1 then # cyclic not for magmas
filt:=filt and IsCyclic;
fi;
else
filt:=filt and IsTrivial and HasIsNonTrivial;
fi;
fi;
elif isgroup and Length(gens)<=1 then # cyclic not for magmas
filt:=filt and IsCyclic;
fi;
fi;
return NewType(fam,filt);
end);

#############################################################################
##
#M GroupWithGenerators( <gens> ) . . . . . . . . group with given generators
Expand All @@ -4291,86 +4325,57 @@ 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);
fi;
typ:=fam!.defaultGroupType;
fi;
function( gens )
local G,typ;

G:=rec();
ObjectifyWithAttributes(G,typ,GeneratorsOfMagmaWithInverses,AsList(gens));
typ:=MakeGroupyType(FamilyObj(gens),
IsGroup and IsAttributeStoringRep
and HasIsEmpty and HasGeneratorsOfMagmaWithInverses,
gens,false,true);

return G;
end );
G:=rec();
ObjectifyWithAttributes(G,typ,GeneratorsOfMagmaWithInverses,AsList(gens));

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);
fi;
typ:=fam!.defaultGroupWithOneType;
fi;
function( gens, id )
local G,typ;

G:=rec();
ObjectifyWithAttributes(G,typ,GeneratorsOfMagmaWithInverses,AsList(gens),
One,id);
typ:=MakeGroupyType(FamilyObj(gens),
IsGroup and IsAttributeStoringRep
and HasIsEmpty and HasGeneratorsOfMagmaWithInverses and HasOne,
gens,id,true);

return G;
G:=rec();
ObjectifyWithAttributes(G,typ,GeneratorsOfMagmaWithInverses,AsList(gens),
One,id);

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 IsTrivial and
HasIsEmpty and HasIsNonTrivial;
typ:=NewType(fam,typ);

return G;
end );
G:= rec();
ObjectifyWithAttributes( G, typ,
GeneratorsOfMagmaWithInverses, empty,
One, id );

return G;
end );


#############################################################################
Expand Down
Loading

0 comments on commit e804843

Please sign in to comment.