Fix QuotientMod docs, and the integer implementation

hpcgap/lib/integer.gi

@@ -1668,15 +1668,24 @@ InstallMethod( QuotientMod,
  true,
  [ IsIntegers, IsInt, IsInt, IsInt ], 0,
  function ( Integers, r, s, m )
- if s > m then 
- s := s mod m;
- fi;
+ local g;
  if m = 1 then
  return 0;
- elif GcdInt( s, m ) <> 1 then
- return fail;
  else
- return r/s mod m;
+ r := r mod m;
+ if r = 0 then return 0; fi; # as required by QuotientMod documentation
+ s := s mod m;
+ if s = 0 then return fail; fi;
+
+ g := GcdInt( r, s );
+ r := r / g;
+ s := s / g;
+ g := GcdInt( g, m );
+ m := m / g;
+ if GcdInt( s, m ) <> 1 then
+ return fail;
+ fi;
+ return r * INVMODINT(s, m) mod m;
  fi;
  end );
 

lib/integer.gi

@@ -1641,15 +1641,24 @@ InstallMethod( QuotientMod,
  true,
  [ IsIntegers, IsInt, IsInt, IsInt ], 0,
  function ( Integers, r, s, m )
- if s > m then 
- s := s mod m;
- fi;
+ local g;
  if m = 1 then
  return 0;
- elif GcdInt( s, m ) <> 1 then
- return fail;
  else
- return r/s mod m;
+ r := r mod m;
+ if r = 0 then return 0; fi; # as required by QuotientMod documentation
+ s := s mod m;
+ if s = 0 then return fail; fi;
+
+ g := GcdInt( r, s );
+ r := r / g;
+ s := s / g;
+ g := GcdInt( g, m );
+ m := m / g;
+ if GcdInt( s, m ) <> 1 then
+ return fail;
+ fi;
+ return r * INVMODINT(s, m) mod m;
  fi;
  end );
 

lib/ring.gd

@@ -1186,24 +1186,22 @@ DeclareOperation( "QuotientRemainder",
 ## <Oper Name="QuotientMod" Arg='[R, ]r, s, m'/>
 ##
 ## <Description>
-## <Ref Oper="QuotientMod"/> returns the quotient of the ring
+## <Ref Oper="QuotientMod"/> returns a quotient of the ring
 ## elements <A>r</A> and <A>s</A> modulo the ring element <A>m</A>
 ## in the ring <A>R</A>, if given,
 ## and otherwise in their default ring, see
 ## <Ref Func="DefaultRing" Label="for ring elements"/>.
 ## <P/>
 ## <A>R</A> must be a Euclidean ring (see <Ref Func="IsEuclideanRing"/>)
 ## so that <Ref Func="EuclideanRemainder"/> can be applied.
-## If the modular quotient does not exist (i.e. when <A>s</A> and <A>m</A>
-## are not coprime), <K>fail</K> is returned.
+## If no modular quotient exists, <K>fail</K> is returned.
 ## <P/>
-## The quotient <M>q</M> of <A>r</A> and <A>s</A> modulo <A>m</A> is
-## an element of <A>R</A>
-## such that <M>q <A>s</A> = <A>r</A></M> modulo <M>m</M>, i.e.,
-## such that <M>q <A>s</A> - <A>r</A></M> is divisible by <A>m</A> in
-## <A>R</A> and that <M>q</M> is either zero (if <A>r</A> is divisible by
-## <A>m</A>) or the Euclidean degree of <M>q</M> is strictly smaller than
-## the Euclidean degree of <A>m</A>.
+## A quotient <M>q</M> of <A>r</A> and <A>s</A> modulo <A>m</A> is an
+## element of <A>R</A> such that <M>q <A>s</A> = <A>r</A></M> modulo
+## <M>m</M>, i.e., such that <M>q <A>s</A> - <A>r</A></M> is divisible by
+## <A>m</A> in <A>R</A> and that <M>q</M> is either zero (if <A>r</A> is
+## divisible by <A>m</A>) or the Euclidean degree of <M>q</M> is strictly
+## smaller than the Euclidean degree of <A>m</A>.
 ## <Example><![CDATA[
 ## gap> QuotientMod( 7, 2, 3 );
 ## 2

tst/testinstall/intarith.tst

@@ -478,6 +478,38 @@ Error, SignRat: argument must be a rational or integer (not a boolean or fail)
 gap> SIGN_INT(fail);
 Error, SignInt: argument must be an integer (not a boolean or fail)
 
+#
+# QuotientMod
+#
+
+# test "... q is either zero (if r is divisible by m) ... "
+gap> ForAll([1..12], m -> ForAll(m*[-m..m], r -> QuotientMod(0,r,m)=0));
+true
+
+# test that 0 divides nothing other than multiples of m
+gap> ForAll([1..10], m -> ForAll([1..m-1], r -> QuotientMod(r,0,m)=fail));
+true
+
+# brute force test a bunch of inputs
+gap> naivQM:={r,s,m}->First([0..m-1], q -> ((q*s-r) mod m) = 0);;
+gap> f := m -> SetX([-2*m..2*m], [-2*m..2*m], {r,s} -> QuotientMod(r,s,m) = naivQM(r,s,m));;
+gap> Set([1..12], f);
+[ [ true ] ]
+
+# test that m = 0 is forbidden
+gap> QuotientMod(5, 4, 0);
+Error, Integer operations: <divisor> must be nonzero
+
+# some old test cases, to show case that issue #149 is resolved
+gap> QuotientMod(2, 4, 6);
+2
+gap> QuotientMod(4, 2, 6);
+2
+gap> QuotientMod(-2, 2, 6);
+2
+gap> QuotientMod(4, 8, 6);
+2
+
 #
 # HexStringInt and IntHexString
 #

tst/testinstall/zmodnz.tst

@@ -438,6 +438,35 @@ ZmodnZObj( 4, 10 )
 gap> Random(GlobalRandomSource, R);
 ZmodnZObj( 9, 10 )
 
+# test some additional quotients
+gap> a:=ZmodnZObj(2, 6);; b:=ZmodnZObj(4, 6);;
+gap> b/a;
+ZmodnZObj( 2, 6 )
+gap> a/b;
+ZmodnZObj( 2, 6 )
+
+#
+gap> x := ZmodnZObj(2,10);
+ZmodnZObj( 2, 10 )
+gap> y := ZmodnZObj(0,10);
+ZmodnZObj( 0, 10 )
+gap> x/y;
+fail
+gap> y/x;
+ZmodnZObj( 0, 10 )
+gap> 0/x;
+ZmodnZObj( 0, 10 )
+gap> x/0;
+fail
+gap> x/3;
+ZmodnZObj( 4, 10 )
+gap> 3/x;
+fail
+gap> y/3;
+ZmodnZObj( 0, 10 )
+gap> 3/y;
+fail
+
 #
 gap> STOP_TEST( "zmodnz.tst", 1);