Document and enforce that "nice monomorphisms" must have IsInjective set (otherwise an infinite recursion can happen)

doc/ref/grphomom.xml

@@ -338,6 +338,14 @@ normally this can only be achieved by using the result of a call to
 <Ref Func="ActionHomomorphism" Label="for a group, an action domain, etc."/>
 or <Ref Func="GroupHomomorphismByFunction" Label="by function (and inverse function) between two domains"/>
 (see also section&nbsp;<Ref Sect="Efficiency of Homomorphisms"/>).
+<P/>
+It may happen that one knows a monomorphism <C>map</C> that is suitable as a
+nice monomorphism of a given group <C>G</C>,
+for example if <C>G</C> is a matrix group and <C>map</C> describes the
+faithful action of <C>G</C> on a small set of vectors.
+In such a case, one can prescribe this monomorphism via
+<C>SetNiceMonomorphism( G, map )</C>,
+provided that <C>map</C> stores that it is injective.
 
 <#Include Label="IsHandledByNiceMonomorphism">
 <#Include Label="NiceMonomorphism">

doc/tut/group.xml

@@ -1211,6 +1211,12 @@ Reconstructing the
 automorphism group may of course result in the  loss of other information
 ⪆ had already gathered, besides the (not-so-)nice monomorphism.
 <P/>
+Prescribing a known map as the nice monomorphism of a group is possible
+only if this map knows that it is injective.
+In the above example, this is the case because the map is created as the
+composition of two maps (a nice monomorphism and an isomorphism)
+which know that they are injective.
+<P/>
 <E>Summary.</E>  In this section we have  seen  how calculations in groups
 can be carried  out in isomorphic  images in  nicer groups. We  have seen
 that &GAP;  pursues this technique  automatically for certain classes of

lib/grpffmat.gi

@@ -150,11 +150,10 @@ InstallGlobalFunction( NicomorphismFFMatGroupOnFullSpace, function( grp )
     # vector arrangement
     SetBaseOfGroup( xset, One( grp ));
     nice := ActionHomomorphism( xset,"surjective" );
+    SetIsInjective( nice, true );
     if not HasNiceMonomorphism(grp) then
       SetNiceMonomorphism(grp,nice);
     fi;
-    SetIsInjective( nice, true );
-    SetFilterObj(nice,IsNiceMonomorphism);
     # because we act on the full space we are canonical.
     SetIsCanonicalNiceMonomorphism(nice,true);
     if Size(V)>10^5 then 

lib/grpnice.gi

@@ -16,11 +16,22 @@
 ##
 #M  SetNiceMonomorphism(<G>,<hom>)
 ##
-##  This setter method is special because we want to tell every nice
-##  monomorphism that it is one.
+##  We install a special setter method because we have to make sure that only
+##  injective maps get stored as nice monomorphisms.
+##  More precisely, we the stored map must know that it is injective,
+##  otherwise computing its kernel may run into an infinite recursion.
+##  (The maps computed by 'NiceMonomorphism' have the 'IsInjective' flag,
+##  the test affects only those maps that shall be set by hand as
+##  nice monomorphisms.)
+##
+##  Besides this, we want to tell every nice monomorphism that it is one.
+##
 InstallMethod(SetNiceMonomorphism,"set `IsNiceomorphism' property",true,
   [IsGroup,IsGroupGeneralMapping],SUM_FLAGS+10, #override system setter
 function(G,hom)
+  if not ( HasIsInjective( hom ) and IsInjective( hom ) ) then
+    Error( "'NiceMonomorphism' values must have the 'IsInjective' flag" );
+  fi;
   SetFilterObj(hom,IsNiceMonomorphism);
   TryNextMethod();
 end);
@@ -103,6 +114,9 @@ InstallMethod( GroupByNiceMonomorphism,
 function( nice, grp )
     local   fam,  pre;
 
+    if not IsInjective( nice ) then
+      Error( "<nice> is not injective" );
+    fi;
     fam := FamilyObj( Source(nice) );
     pre := Objectify(NewType(fam,IsGroup and IsAttributeStoringRep), rec());
     SetIsHandledByNiceMonomorphism( pre, true );
@@ -1043,12 +1057,14 @@ InstallMethod( NiceMonomorphism, "SeedFaithfulAction supersedes", true,
         [ IsGroup and IsHandledByNiceMonomorphism and
 	  HasSeedFaithfulAction], 1000,
 function(G)
-local b;
+  local b, hom;
   b:=SeedFaithfulAction(G);
   if b=fail then
     TryNextMethod();
   fi;
-  return MultiActionsHomomorphism(G,b.points,b.ops);
+  hom:= MultiActionsHomomorphism(G,b.points,b.ops);
+  SetIsInjective( hom, true );
+  return hom;
 end);
 
 

tst/testinstall/mapping.tst

@@ -1,5 +1,5 @@
 #@local A,B,C,M,anticomp,com,comp,conj,d,g,g2,i,i2,inv,j,map,map1,map2
-#@local mapBijective,nice,res,t,t1,t2,tuples,hom,aut,dp
+#@local mapBijective,nice,res,t,t1,t2,tuples,vecs,hom,aut,dp
 gap> START_TEST("mapping.tst");
 
 # Init
@@ -419,9 +419,26 @@ true
 gap> IsInjective(res);        
 true
 
+# set NiceMonomorphism by hand (as suggested in the Tutorial)
+gap> g:= Group( [ [ [ 0, 1 ], [ 1, 0 ] ] ] );;
+gap> IsHandledByNiceMonomorphism( g );
+true
+gap> vecs:= Orbit( g, [ 1, 0 ], OnRight );;
+gap> hom:= ActionHomomorphism( g, vecs, OnRight );;
+gap> HasNiceMonomorphism( g );
+false
+gap> SetNiceMonomorphism( g, hom );
+Error, 'NiceMonomorphism' values must have the 'IsInjective' flag
+gap> IsInjective( hom );
+true
+gap> SetNiceMonomorphism( g, hom );
+gap> HasNiceMonomorphism( g );
+true
+
 # printing of identity mapping string in direct product element (PR #3753) 
 gap> String(IdentityMapping(SymmetricGroup(3)));
 "IdentityMapping( SymmetricGroup( [ 1 .. 3 ] ) )"
+gap> g := Group((1,2),(3,4));;
 gap> hom := GroupHomomorphismByImages(g,g,[(1,2),(3,4)],[(3,4),(1,2)]);
 [ (1,2), (3,4) ] -> [ (3,4), (1,2) ]
 gap> aut := Group(hom);;
@@ -433,4 +450,4 @@ gap> GeneratorsOfGroup(dp);
       [ (1,2), (3,4) ] -> [ (3,4), (1,2) ] ] ) ]
 
 #
-gap> STOP_TEST( "mapping.tst", 1);
+gap> STOP_TEST( "mapping.tst" );