Allow optionally passing a random source to RandomList, and to Random for collections

hpcgap/lib/coll.gd

@@ -1661,15 +1661,17 @@ DeclareOperation( "Random", [ IS_INT, IS_INT ] );
 ##
 ##  <#GAPDoc Label="RandomList">
 ##  <ManSection>
-##  <Func Name="RandomList" Arg='list'/>
+##  <Func Name="RandomList" Arg='[rs,] list'/>
 ##
 ##  <Description>
 ##  <Index>random seed</Index>
 ##  For a dense list <A>list</A>,
 ##  <Ref Func="RandomList"/> returns a (pseudo-)random element with equal
 ##  distribution.
 ##  <P/>
-##  This function uses the <Ref Var="GlobalMersenneTwister"/> to produce the
+##  The random source <A>rs</A> is used to choose a random number.
+##  If <A>rs</A> is absent,
+##  this function uses the <Ref Var="GlobalMersenneTwister"/> to produce the
 ##  random elements (a source of high quality random numbers).
 ##  <P/>
 ##  </Description>

lib/coll.gd

@@ -1631,15 +1631,17 @@ DeclareOperation( "Random", [ IS_INT, IS_INT ] );
 ##
 ##  <#GAPDoc Label="RandomList">
 ##  <ManSection>
-##  <Func Name="RandomList" Arg='list'/>
+##  <Func Name="RandomList" Arg='[rs,] list'/>
 ##
 ##  <Description>
 ##  <Index>random seed</Index>
 ##  For a dense list <A>list</A>,
 ##  <Ref Func="RandomList"/> returns a (pseudo-)random element with equal
 ##  distribution.
 ##  <P/>
-##  This function uses the <Ref Var="GlobalMersenneTwister"/> to produce the
+##  The random source <A>rs</A> is used to choose a random number.
+##  If <A>rs</A> is absent,
+##  this function uses the <Ref Var="GlobalMersenneTwister"/> to produce the
 ##  random elements (a source of high quality random numbers).
 ##  <P/>
 ##  </Description>

lib/coll.gi

@@ -246,15 +246,28 @@ InstallMethod( RepresentativeSmallest,
 ##  an enumerator of <C> and selects a random element of this list using the
 ##  function `RandomList', which is a pseudo random number generator.
 ##
-InstallGlobalFunction( RandomList, function(list)
-  return list[Random(GlobalMersenneTwister, 1, Length(list))];
+
+# RandomList is not an operation to avoid the (often expensive) cost of
+# dispatch for lists
+InstallGlobalFunction( RandomList, function(args...)
+  local len, source, list;
+  len := Length(args);
+  if len = 1 then
+    source := GlobalMersenneTwister;
+    list := args[1];
+  elif len = 2 then
+    source := args[1];
+    list := args[2];
+  else
+    Error(" Usage: RandomList([rs], list))");
+  fi;
+
+  return list[Random(source, 1, Length(list))];
 end );
-InstallMethod( Random, "for a (finite) collection",
-    [ IsCollection and IsFinite ],
-    C -> RandomList( Enumerator( C ) ) );
 
-RedispatchOnCondition(Random,true,[IsCollection],[IsFinite],0);
 
+RedispatchOnCondition(Random,true,[IsCollection],[IsFinite],0);
+RedispatchOnCondition(Random,true,[IsRandomSource,IsCollection],[IsFinite],0);
 
 #############################################################################
 ##

lib/random.gi

@@ -311,3 +311,9 @@ end );
     InstallGlobalFunction("InstallMethodWithRandomSource", func(InstallMethod));
     InstallGlobalFunction("InstallOtherMethodWithRandomSource", func(InstallOtherMethod));
 end)();
+
+# This method must rank below Random(SomeRandomSource, IsList)
+# for any random source SomeRandomSource, to avoid an infinite loop.
+InstallMethodWithRandomSource( Random, "for a random source and a (finite) collection",
+    [ IsRandomSource, IsCollection and IsFinite ], -8,
+    {rs, C} -> RandomList(rs, Enumerator( C ) ) );

tst/testinstall/random.tst

@@ -1,6 +1,13 @@
 gap> START_TEST("random.tst");
 gap> ReadGapRoot( "tst/testrandom.g" );
 
+# Test RandomList
+gap> randomTest([1,2,3], RandomList);
+gap> randomTest([1..100], RandomList);
+gap> randomTest("abcdef", RandomList);
+gap> randomTest(BlistList([1..100],[1,3..99]), RandomList);
+gap> randomTest([1], RandomList);
+
 #
 # fields and rings
 #
@@ -21,7 +28,7 @@ gap> randomTest(GF(257^2), Random);
 gap> randomTest(GF(2^20), Random);
 
 # ZmodnZ
-gap> randomTestForSizeOneCollection(Integers mod 1, Random);
+gap> randomTest(Integers mod 1, Random);
 gap> randomTest(Integers mod 4, Random);
 gap> randomTest(Integers mod 100, Random);
 
@@ -37,14 +44,14 @@ gap> randomTest(FreeMagma(1), Random);
 gap> randomTest(FreeMagma(2), Random);
 
 #
-gap> randomTestForSizeOneCollection(FreeMonoid(0), Random);
+gap> randomTest(FreeMonoid(0), Random);
 gap> randomTest(FreeMonoid(1), Random);
 gap> randomTest(FreeMonoid(2), Random);
 
 #
 # permutation groups
 #
-gap> randomTestForSizeOneCollection(TrivialGroup(IsPermGroup), Random);
+gap> randomTest(TrivialGroup(IsPermGroup), Random);
 
 #
 gap> randomTest(SymmetricGroup(2), Random);
@@ -65,15 +72,15 @@ gap> randomTest(PrimitiveGroup(5,3)*(1,4,6), Random);
 #
 # pc groups
 #
-gap> randomTestForSizeOneCollection(TrivialGroup(IsPcGroup), Random);
+gap> randomTest(TrivialGroup(IsPcGroup), Random);
 gap> randomTest(AbelianGroup(IsPcGroup, [2]), Random);
 gap> randomTest(AbelianGroup(IsPcGroup, [2,3,4,5]), Random);
 
 #
 # fp groups
 #
-gap> randomTestForSizeOneCollection(TrivialGroup(IsFpGroup), Random);
-gap> randomTestForSizeOneCollection(FreeGroup(0), Random);
+gap> randomTest(TrivialGroup(IsFpGroup), Random);
+gap> randomTest(FreeGroup(0), Random);
 gap> randomTest(FreeGroup(1), Random);
 gap> randomTest(FreeGroup(2), Random);
 gap> randomTest(FreeGroup(infinity), Random);
@@ -82,9 +89,9 @@ gap> randomTest(DihedralGroup(IsFpGroup, 6), Random);
 #
 # matrix groups
 #
-gap> randomTestForSizeOneCollection(CyclicGroup(IsMatrixGroup, GF(2), 1), Random);
-gap> randomTestForSizeOneCollection(CyclicGroup(IsMatrixGroup, GF(9), 1), Random);
-gap> randomTestForSizeOneCollection(CyclicGroup(IsMatrixGroup, Rationals, 1), Random);
+gap> randomTest(CyclicGroup(IsMatrixGroup, GF(2), 1), Random);
+gap> randomTest(CyclicGroup(IsMatrixGroup, GF(9), 1), Random);
+gap> randomTest(CyclicGroup(IsMatrixGroup, Rationals, 1), Random);
 
 #
 gap> randomTest(CyclicGroup(IsMatrixGroup, GF(2), 3), Random);
@@ -108,8 +115,11 @@ gap> randomTest(DoubleCoset(Group(()), (1,2), Group((1,2,3)) ), Random);
 gap> randomTest(DoubleCoset(Group((1,2),(3,4)), (), Group((1,2,3)) ), Random);
 
 #
-gap> randomTestForSizeOneCollection([1], Random);
+gap> randomTest([1], Random);
 gap> randomTest([1..10], Random);
+gap> randomTest([1..2], Random);
+gap> randomTest([0, 10..1000], Random);
+gap> randomTest("cheese", Random);
 gap> randomTest([1,-6,"cheese", Group(())], Random);
 
 #

tst/testrandom.g

@@ -1,35 +1,58 @@
-randomTest := function(collection, method, checkin...)
-    local test1, test2, test3, test4, test5, test6, localgen, checkmethod;
-    if Length(checkin) = 0 then
-        checkmethod := \in;
-    else
-        checkmethod := checkin[1];
-    fi;
+# Perform a variety of tests on Random Sources and functions which create
+# random objects.
+#
+# This function is used for a variety is different tests:
+#
+# Test that 'Random(C)' and 'Random(GlobalMersenneTwister, C)' produce 
+# the same answer.
+#
+# Test that 'Random(rs,C)' only uses 'rs', and no other source of random
+#
+# Test Random and RandomList
+#
+# Where there is a global instance of a random source
+# (GlobalMersenneTwister and GlobalRandomSource), they produce the same
+# sequence of answers as a new instance of the same random source.
+
+# filter: The type of random source we are testing.
+# global_rs: A pre-existing object of type 'filter'.
+# randfunc(rs, C): A two argument function which creates random elements of C using
+#              'rs' as the source.
+# global_randfunc(C): A one argument function which is equivalent to 
+#             {x} -> rand(global_rs, x). This lets us check 'Random(C)' and
+#             'Random(GlobalMersenneTwister,C)' produce the same answer when testing
+#             GlobalMersenneTwister. For other random sources, this can just
+#             be set to {x} -> rand(global_rs,x).
+# collection: The object (usually a collection) to find random members of.
+# checkin(e, C): returns if e is in C (usually checkin is '\in').
+
+randomTestInner := function(filter, global_rs, global_randfunc, randfunc, collection, checkin)
+    local test1, test2, test3, test4, test5, test6, local_rs;
 
     # We do a single call first, to deal with calling Random causing extra attributes
     # of 'collection' to be set, changing the dispatch
-    method(collection);
+    randfunc(collection);
 
     # Firstly, we will generate a base list
-    Init(GlobalMersenneTwister, 6);
-    test1 := List([1..1000], x -> method(collection));
-    # test2 should = test1
-    Init(GlobalMersenneTwister, 6);
-    test2 := List([1..1000], x -> method(collection));
+    Init(global_rs, 6);
+    test1 := List([1..1000], x -> global_randfunc(collection));
+    # test2 should equal test1
+    Init(global_rs, 6);
+    test2 := List([1..1000], x -> global_randfunc(collection));
     # test3 should also = test1
-    Init(GlobalMersenneTwister, 6);
-    test3 := List([1..1000], x -> method(GlobalMersenneTwister, collection));
+    Init(global_rs, 6);
+    test3 := List([1..1000], x -> randfunc(global_rs, collection));
     # test4 should be different (as it came from a different seed)
-    Init(GlobalMersenneTwister, 8);
-    test4 := List([1..1000], x -> method(collection));
+    Init(global_rs, 8);
+    test4 := List([1..1000], x -> global_randfunc(collection));
     # test5 should be the same as test4, as it is made from seed 8
     # test6 should be the same as test1. Also, it checks that making test5
     # did not touch the global source at all.
-    Init(GlobalMersenneTwister, 8);
-    localgen :=  RandomSource(IsMersenneTwister, 6);
-    test5 := List([1..1000], x -> method(localgen, collection));
-    test6 := List([1..1000], x -> method(collection));
-    if ForAny(Concatenation(test1, test2, test3, test4, test5, test6), x -> not (checkmethod(x, collection)) ) then
+    Init(global_rs, 8);
+    local_rs :=  RandomSource(filter, 6);
+    test5 := List([1..1000], x -> randfunc(local_rs, collection));
+    test6 := List([1..1000], x -> global_randfunc(collection));
+    if ForAny(Concatenation(test1, test2, test3, test4, test5, test6), x -> not (checkin(x, collection)) ) then
         Print("Random member outside collection: ", collection,"\n");
     fi;
     if test1 <> test2 then
@@ -45,49 +68,81 @@ randomTest := function(collection, method, checkin...)
         Print("Alt gen broken: ", collection, "\n");
     fi;
     if test4 <> test6 then
-        Print("Random with a passed in seed affected the global generator: ", collection, "\n");
+        Print("Random with a passed in seed affected the global source: ", collection, "\n");
     fi;
 end;;
 
+
 # A special test for collections of size 1
-randomTestForSizeOneCollection := function(collection, method)
-    local i, val, localgen, intlist1, intlist2;
-    if Size(collection) <> 1 then
-        Print("randomTestForSizeOneCollection is only for collections of size 1");
-        return;
-    fi;
+# Here we can't check different seeds produce different answers
+# We do check that the random source is not used, for efficency.
+randomTestForSizeOneCollectionInner := function(filter, global_rs, global_randfunc, randfunc, collection, checkin)
+    local i, val, local_rs, intlist1, intlist2;
 
     val := Representative(collection);
 
-    Init(GlobalMersenneTwister, 6);
-    intlist1 := List([1..1000], x -> Random([1..10]));
+    Init(global_rs, 6);
+    intlist1 := List([1..10], x -> global_randfunc([1..1000]));
 
-    for i in [1..1000] do
-        if method(collection) <> val then
+    for i in [1..100] do
+        if global_randfunc(collection) <> val then
             Print("Random returned something outside collection :", collection, ":", val);
         fi;
     od;
 
-    for i in [1..1000] do
-        if method(GlobalMersenneTwister, collection) <> val then
+    for i in [1..100] do
+        if randfunc(global_rs, collection) <> val then
             Print("Random returned something outside collection :", collection, ":", val);
         fi;
     od;
 
-    localgen := RandomSource(IsMersenneTwister, 6);
+    local_rs := RandomSource(filter, 6);
 
-    Init(GlobalMersenneTwister, 6);
-    for i in [1..1000] do
-        if method(localgen, collection) <> val then
+    Init(global_rs, 6);
+    for i in [1..100] do
+        if randfunc(local_rs, collection) <> val then
             Print("Random returned something outside collection :", collection, ":", val);
         fi;
     od;
 
-    # The previous loop should not have affected GlobalMersenneTwister,
+    # The previous loop should not have affected global_rs,
     # so this should be the same as intlist1
-    intlist2 := List([1..1000], x -> Random([1..10]));
+    intlist2 := List([1..10], x -> global_randfunc([1..1000]));
 
     if intlist1 <> intlist2 then
         Print("Random read from local gen affected global gen: ", collection);
     fi;
 end;;
+
+
+randomTest := function(collection, randfunc, checkin...)
+    local sizeone, randchecker;
+    if Length(checkin) = 0 then
+        checkin := \in;
+    else
+        checkin := checkin[1];
+    fi;
+
+    # Make a best attempt to find if the collection is size 1.
+    # There are implementations of random for objects which do not support
+    # Size or IsTrivial, e.g. PadicExtensionNumberFamily
+    if IsList(collection) then
+        sizeone := (Size(collection) = 1);
+    elif IsCollection(collection) then
+        sizeone := IsTrivial(collection);
+    else
+        sizeone := false;
+    fi;
+
+    if sizeone then
+        randchecker := randomTestForSizeOneCollectionInner;
+    else
+        randchecker := randomTestInner;
+    fi;
+
+    randchecker(IsMersenneTwister,
+        GlobalMersenneTwister, x -> randfunc(x), randfunc, collection, checkin);
+    randchecker(IsGAPRandomSource,
+        GlobalRandomSource, x -> randfunc(GlobalRandomSource, x), randfunc,
+        collection, checkin);
+end;