WIP

gap/base/recognition.gi

@@ -394,9 +394,9 @@ NUM_MANDARINS := 100;
 InstallGlobalFunction( RecogniseGeneric,
   function(H, methoddb, depthString, knowledge, mandarins)
     # Assume all the generators have no memory!
-    local N,depth,done,i,l,ll,gensNmeth,allmethods,
+    local oldRandstore, N,depth,done,i,l,ll,gensNmeth,allmethods,
           proj1,proj2,ri,rifac,riker,s,x,y,z,succ,counter,
-          kernelMandarins,factorMandarins,tmp;
+          kernelMandarins,factorMandarins;
 
     depth := Length(depthString);
 
@@ -410,13 +410,13 @@ InstallGlobalFunction( RecogniseGeneric,
 
     if mandarins = fail then
         Assert(0, depth = 0);
-        # Use SetPseudoRandomStamp or RandomElm
-        # FIXME: double-check whether mandarins are not reused. Since
-        # PseudoRandom may use RandomElm as the pseudorandomfunc it could
-        # happen that the mandarins are reused.
-        # I think we have to undo the hack to PseudoRandom or, even worse,
-        # remove the mandarins from ri!.randr manually
+        # HACK: We don't want the mandarins to be reused by any computation.
+        # Since PseudoRandom is hacked, if we don't set ri!.randstore := false
+        # then the mandarins would be reused by RandomElm and RandomElmOrd.
+        oldRandstore := ri!.randstore;
+        ri!.randstore := false;
         mandarins := List([1..NUM_MANDARINS], i -> PseudoRandom(H));
+        ri!.randstore := oldRandstore;
     fi;
 
 
@@ -471,16 +471,19 @@ InstallGlobalFunction( RecogniseGeneric,
         fi;
 
         # check mandarins now
-        tmp := NiceGens(ri);
         for x in mandarins do
             s := SLPforElement(ri, x);
             if s = fail then
                 # TODO: should not really happen
-                Error("Mandarins detected bad leaf");
+                # because we verify leaves immediately?
+                ErrorNoReturn("Mandarins detected bad leaf");
             fi;
             # TODO: validate the SLP, too?
             z := ResultOfStraightLineProgram(s, tmp);
-            Assert(0, ri!.isequal(x, z));
+            if not ri!.isequal(x, z) then
+                # TODO: deal with this properly
+                ErrorNoReturn("Mandarins detected bad leaf");
+            fi;
         od;
 
         # these two were set correctly by FindHomomorphism
@@ -498,17 +501,29 @@ InstallGlobalFunction( RecogniseGeneric,
     # In that case we know that ri now knows: homom plus additional data.
 
     # map the mandarins
+    # TODO Can we also put the ValidateHomomInput call with
+    # GeneratorsOfGroup(H) here? If it fails we'll need to handle it exactly as
+    # we handle a botched mandarin computation.
+    for x in mandarins do
+        if not ValidateHomomInput(ri, x) then
+            # TODO: deal with this properly, see CompTree's description of what
+            # it does in a "crisis"
+            ErrorNoReturn("Mandarins detected bad factor homom");
+        fi;
+    od;
     factorMandarins := [];
     for x in mandarins do
+        # TODO: magma has separate "test mandarins" and "map mandarins"
+        # functions. Does that improve performance notably?
         y := ImageElm(Homom(ri), x);
         if y = fail then
-            # TODO: deal with this properly
-            Error("Mandarins detected bad factor homom");
+            # TODO: deal with this properly, see above
+            ErrorNoReturn("Mandarins detected bad factor homom");
         fi;
         Add(factorMandarins, y);
         #s := SLPforElement(ri, x);
     od;
-    # sort the factorMandarins and remove duplicates and trivials
+    # TODO: sort the factorMandarins and remove duplicates and trivials
 
     # Try to recognise the factor a few times, then give up:
     counter := 0;
@@ -603,8 +618,7 @@ InstallGlobalFunction( RecogniseGeneric,
             Add( kernelMandarins, x / z );
         fi;
     od;
-    # sort the kernelMandarins and remove duplicates and trivials
-#    kernelMandarins := rifac!.kernelMandarins
+    # TODO: sort the kernelMandarins and remove duplicates and trivials
 
     if Length(gensN(ri)) = 0 then
         # We found out that N is the trivial group!
@@ -620,7 +634,7 @@ InstallGlobalFunction( RecogniseGeneric,
                 # we need to 
                 # WARNING: do NOT add the mandarin here, even though it may seem tempting.
                 # But (a) they don't have memory, and (b) we need them to stay "clean",
-                # if we start using them for computations, this destroys out assumption
+                # if we start using them for computations, this destroys our assumption
                 # about their independency
                 succ := FindKernelFastNormalClosure(ri,5,5);
                 Info(InfoRecog,2,"Have now ",Length(gensN(ri)),