Reduce impact of immediate methods: change some to implications, remove others, use custom SetSize method (rebased #2387)

lib/coll.gd

@@ -1415,6 +1415,11 @@ InstallFactorMaintenance( IsTrivial,
 ##
 DeclareProperty( "IsNonTrivial", IsCollection );
 
+InstallTrueMethod( IsNonTrivial, IsEmpty );
+InstallTrueMethod( HasIsTrivial, IsNonTrivial );
+InstallTrueMethod( HasIsEmpty, IsTrivial );
+InstallTrueMethod( HasIsNonTrivial, IsTrivial );
+
 
 #############################################################################
 ##
@@ -1447,6 +1452,7 @@ InstallFactorMaintenance( IsFinite,
     IsCollection and IsFinite, IsObject, IsCollection );
 
 InstallTrueMethod( IsFinite, IsTrivial );
+InstallTrueMethod( IsFinite, IsEmpty );
 
 
 #############################################################################

lib/coll.gi

@@ -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,
@@ -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();
@@ -3055,6 +3059,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;
+  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

lib/csetgrp.gi

@@ -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);
 
@@ -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);
 

lib/ctbl.gd

@@ -3584,6 +3584,8 @@ DeclareGlobalVariable( "CharacterTableDisplayDefaults" );
 ##  tbl:=rec();
 ##  tbl.Irr:=
 ##  [ [ 1, 1 ], [ 1, -1 ] ];
+##  tbl.IsFinite:=
+##  true;
 ##  tbl.NrConjugacyClasses:=
 ##  2;
 ##  tbl.Size:=

lib/grp.gd

@@ -603,6 +603,8 @@ InstallTrueMethod( IsGroup, IsPerfectGroup );
 InstallFactorMaintenance( IsPerfectGroup,
     IsGroup and IsPerfectGroup, IsObject, IsGroup );
 
+InstallTrueMethod( IsPerfectGroup, IsGroup and IsTrivial );
+
 #############################################################################
 ##
 #P  IsSporadicSimpleGroup( <G> )
@@ -797,6 +799,8 @@ InstallFactorMaintenance( IsSolvableGroup,
 InstallTrueMethod( IsSolvableGroup, IsMonomialGroup );
 InstallTrueMethod( IsSolvableGroup, IsSupersolvableGroup );
 
+InstallTrueMethod( HasIsPerfectGroup, IsGroup and IsSolvableGroup and IsNonTrivial );
+
 
 #############################################################################
 ##
@@ -3582,6 +3586,12 @@ DeclareOperation( "GroupWithGenerators",
     [ IsListOrCollection, IsMultiplicativeElementWithInverse ] );
 
 
+#F  MakeGroupyType( <fam>, <filt>, <gens>, <isgroup> )
+# type creator function to incorporate basic deductions so immediate methods
+# are not needed
+DeclareGlobalFunction("MakeGroupyType");
+
+
 #############################################################################
 ##
 #F  Group( <gen>, ... )

lib/grp.gi

@@ -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;
@@ -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
@@ -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,
@@ -4297,6 +4298,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 IsNonTrivial;
+          if isgroup and Length(gens)<=1 then # cyclic not for magmas
+            filt:=filt and IsCyclic;
+          fi;
+        else
+          filt:=filt and IsTrivial;
+        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
@@ -4305,86 +4339,56 @@ 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;
+  typ:=NewType(fam,typ);
 
-    return G;
-    end );
+  G:= rec();
+  ObjectifyWithAttributes( G, typ,
+                            GeneratorsOfMagmaWithInverses, empty,
+                            One, id );
+
+  return G;
+end );
 
 
 #############################################################################