Improve bicoset code

- add `bicoset` to the reference manual index
- clarify documentation for `IsBiCoset` ("for example" -> "if and only if")
- improve `IsBiCoset` by replacing `r*x/r` with `x^r`
- improve multiplication of right cosets to only transfer knowledge about
  whether this is a bicoset if this was already known for the right operand,
  instead of unconditionally computing it; but also transfer if it is not
  (a bicoset times a non-bicoset is not a bicoset)
- replace `TryNextMethod` in multiplication method for right cosets with
  helpful error message, just like we do in the inversion method
- improve error message when inverting a right coset which is not a bicoset
- add a test that ensures we indeed produce an error when multiplying or
  inverting right cosets for which these operations are not defined

Author: fingolfin

lib/csetgrp.gd

@@ -329,9 +329,11 @@ DeclareCategory("IsRightCoset", IsDomain and IsExternalOrbit and
 ##  <Prop Name="IsBiCoset" Arg='C'/>
 ##
 ##  <Description>
-##  A (right) coset is considered a <E>bicoset</E> if its set of elements simultaneously forms a left
-##  coset for the same subgroup. This is the case, for example, if the coset representative normalizes
-##  the subgroup.
+##  <Index>bicoset</Index>
+##  A (right) coset <M>Ug</M> is considered a <E>bicoset</E> if its set of
+##  elements simultaneously forms a left coset for the same subgroup. This is
+##  the case if and only if the coset representative <M>g</M> normalizes the
+##  subgroup <M>U</M>.
 ##  </Description>
 ##  </ManSection>
 ##  <#/GAPDoc>

lib/csetgrp.gi

@@ -634,7 +634,7 @@ function(c)
 local s,r;
   s:=ActingDomain(c);
   r:=Representative(c);
-  return ForAll(GeneratorsOfGroup(s),x->r*x/r in s);
+  return ForAll(GeneratorsOfGroup(s),x->x^r in s);
 end);
 
 InstallMethodWithRandomSource(Random,
@@ -667,11 +667,11 @@ function(a,b)
 local c;
   if ActingDomain(a)<>ActingDomain(b) then TryNextMethod();fi;
   if not IsBiCoset(a) then # product does not require b to be bicoset
-    TryNextMethod();
+    ErrorNoReturn("right cosets can only be multiplied if the left operand is a bicoset");
   fi; 
   c:=RightCoset(ActingDomain(a), Representative(a) * Representative(b) );
-  if IsBiCoset(b) then
-    SetIsBiCoset(c,true);
+  if HasIsBiCoset(b) then
+    SetIsBiCoset(c,IsBiCoset(b));
   fi;
 
   return c;
@@ -684,7 +684,7 @@ local s,r;
   s:=ActingDomain(a);
   r:=Representative(a);
   if not IsBiCoset(a) then
-    Error("Inversion only works for cosets of normal subgroups");
+    ErrorNoReturn("only right cosets which are bicosets can be inverted");
   fi;
   r:=RightCoset(s,Inverse(r));
   SetIsBiCoset(r,true);

tst/testbugfix/2018-08-08-bicosets.tst

@@ -0,0 +1,25 @@
+# We used to allow multiplying and inverting right cosets for which this
+# was not valid. This test verifies this is not the case anymore.
+# See <https://github.com/gap-system/gap/issues/2555>
+gap> G := SymmetricGroup(3);;
+gap> U := Group( (1,2) );;
+gap> cos1 := RightCoset(U, (1,2));;
+gap> cos2 := RightCoset(U, (1,3));;
+gap> IsBiCoset(cos1);
+true
+gap> IsBiCoset(cos2);
+false
+
+#
+gap> cos1*cos1 = cos1;
+true
+gap> cos1*cos2 = cos2;
+true
+gap> cos2*cos1;
+Error, right cosets can only be multiplied if the left operand is a bicoset
+
+#
+gap> cos1^-1 = cos1;
+true
+gap> cos2^-1;
+Error, only right cosets which are bicosets can be inverted