From 890b181fb9eaa31e58d689f08712a731a5e023c0 Mon Sep 17 00:00:00 2001
From: Max Horn <max@quendi.de>
Date: Wed, 8 Aug 2018 14:00:15 +0200
Subject: [PATCH] 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
---
 lib/csetgrp.gd                         |  8 +++++---
 lib/csetgrp.gi                         | 10 +++++-----
 tst/testbugfix/2018-08-08-bicosets.tst | 25 +++++++++++++++++++++++++
 3 files changed, 35 insertions(+), 8 deletions(-)
 create mode 100644 tst/testbugfix/2018-08-08-bicosets.tst

diff --git a/lib/csetgrp.gd b/lib/csetgrp.gd
index 3dfcf8b20d..eab7e2a1ce 100644
--- a/lib/csetgrp.gd
+++ b/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>
diff --git a/lib/csetgrp.gi b/lib/csetgrp.gi
index 8e36cd2c9b..1d12e398b1 100644
--- a/lib/csetgrp.gi
+++ b/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);
diff --git a/tst/testbugfix/2018-08-08-bicosets.tst b/tst/testbugfix/2018-08-08-bicosets.tst
new file mode 100644
index 0000000000..6823e824c2
--- /dev/null
+++ b/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