GeneratingMCOrbits does not work with GAP master

[olexandr-konovalov]

See e.g. https://travis-ci.org/gap-packages/MapClass/jobs/424849206

########> Diff in /home/travis/build/gap-packages/MapClass/gaproot/pkg/MapClas\
s/tst/GeneratingMCOrbits.tst:6
# Input is:
orbs:=GeneratingMCOrbits(G,0,[(1,2),(1,2),(1,2),(1,2),(1,2),(1,2),(1,2),(1,2)]\
:OutputStyle:="silent");;
# Expected output:
# But found:
Error, no method found! For debugging hints type ?Recovery from NoMethodFound
Error, no 1st choice method found for `*' on 2 arguments
########

[olexandr-konovalov]

I can not manage to reproduce this in the interactive session so far, but can reproduce when I read tst/testall.g.

[olexandr-konovalov]

@shpectorov I have traced this down to the following:

gap> G:=SymmetricGroup(4);;
gap> orbs:=GeneratingMCOrbits(G,0,[(1,2),(1,2),(1,2),(1,2),(1,2),(1,2),(1,2),(1,2)]:OutputStyle:="silent");;
Error, no method found! For debugging hints type ?Recovery from NoMethodFound
Error, no 1st choice method found for `*' on 2 arguments at /circa/scratch/gap-jenkins/workspace/GAP-pkg-update-stable-quicktest/GAPCOPT\
S/64build/GAPGMP/gmp/GAPTARGET/packages/label/kovacs/GAP-pkg-update-stable-snaps\
hot/lib/methsel2.g:250 called from
product * Product( t{[ (2 * OurG + 1) .. Length( t ) ]} )
  at /circa/scratch/gap-jenkins/workspace/GAP-pkg-update-stable-quicktest/GAPCOP\
TS/64build/GAPGMP/gmp/GAPTARGET/packages/label/kovacs/GAP-pkg-update-stable-snap\
shot/pkg/MapClass-1.4.1/lib/tuple.gi:94 called from
RandomGeneratingTuple( group, tuple, OurG, OurR 
 ) at /circa/scratch/gap-jenkins/workspace/GAP-pkg-update-stable-quicktest/GAPCO\
PTS/64build/GAPGMP/gmp/GAPTARGET/packages/label/kovacs/GAP-pkg-update-stable-sna\
pshot/pkg/MapClass-1.4.1/lib/tuple.gi:139 called from
CollectRandomGeneratingTuples( NumberOfRandomTuples, genus, OurR, 
 SubgroupRecord, group, tuple 
 ) at /circa/scratch/gap-jenkins/workspace/GAP-pkg-update-stable-quicktest/GAPCO\
PTS/64build/GAPGMP/gmp/GAPTARGET/packages/label/kovacs/GAP-pkg-update-stable-sna\
pshot/pkg/MapClass-1.4.1/lib/main.gi:215 called from
MappingClassOrbits( group, genus, tuple, partition, structureconst 
 ) at /circa/scratch/gap-jenkins/workspace/GAP-pkg-update-stable-quicktest/GAPCO\
PTS/64build/GAPGMP/gmp/GAPTARGET/packages/label/kovacs/GAP-pkg-update-stable-sna\
pshot/pkg/MapClass-1.4.1/lib/interface.gi:31 called from
<function "GeneratingMCOrbits">( <arguments> )
 called from read-eval loop at *stdin*:10
type 'quit;' to quit to outer loop

Indeed, there is no way to multiply 1 and permutation:

brk> ShowArguments();
[ 1, (1,4,2,3) ]

This comes from

MapClass/lib/tuple.gi

Lines 80 to 105 in 3af435e

	InstallGlobalFunction(RandomGeneratingTuple,function(group, tuple, OurG, OurR)
	local i, k, t, product, c;
	while true do
	t:=List([1..2*OurG], x-> Random(group));
	product:=Product(List([1..OurG], x->Comm(t[x], t[OurG+x])));
	if OurR=0 then
	if product=One(group) then
	return rec(tuple:=t, subgroupNumber:=1);
	fi;
	else
	if OurR>1 then
	for c in tuple{[1..OurR-1]} do
	Append(t, [c^Random(group)]);
	od;
	product:=product*Product(t{[2*OurG+1..Length(t)]});
	fi;
	if IsConjugate(group, product^-1, tuple[OurR]) then
	Append(t,[product^-1]);
	if Size(Subgroup(group,t))= Size(group) then
	return rec(tuple:=t, subgroupNumber:=1);
	fi;
	fi;
	fi;
	od;
	end);

because OurG is 0, the product over an empty list is 1, hence product=One(group) returns false, and the function does not return, but continues further.

Would the correct fix be to check for IsOne(product) ?

[olexandr-konovalov]

@shpectorov the fix of course does not work, because the check for product to be an identity is performed only when OurR is zero, but we have OurR=8.

I am puzzled why this error does not appear in GAP 4.9, but the answer is simple: in GAP 4.9 it is possible to multiply 1 and a permutation:

gap> 1*(1,2,3);
(1,2,3)
gap> 10*(1,2,3);
Error, no method found! For debugging hints type ?Recovery from NoMethodFound
Error, no 1st choice method found for `+' on 2 arguments at /Users/alexk/gap-4.9.3/lib/methsel2.g:250 called from
<function "HANDLE_METHOD_NOT_FOUND">( <arguments> )
 called from read-eval loop at *stdin*:2
you can 'quit;' to quit to outer loop, or
you can 'return;' to continue
brk> ShowArguments();
[ (1,2,3), (1,2,3) ]

and this is no longer possible, resulting in a safer code.

[olexandr-konovalov]

So, possible fix would be to check for IsOne(product) and if so, make it One(Group)?

@shpectorov if this sounds right, I will implement it and make a new release.

[fingolfin]

If you product value is computed via a call to Product, then arguably the correct solution is to use the optional init argument to Product, passing in One(G) (where G is the group in question). That value then is returned if the first argument to Product is an empty list.

[fingolfin]

Of course if using IsOne can be done quicker, that's a pragmatic solution as well, as long as the product is not used anywhere else (and you are sure it won't be used anywhere else in the future, when everybody forgot about this problem again... but then, since the test suite seems to trigger it, that hopefully won't be an issue)