Skip to content

Commit

Permalink
Fix QuotientMod docs, and the integer implementation
Browse files Browse the repository at this point in the history
This partially reverts changes we made 2013. The documentation
is now correct (resp. consistent again), and several corner cases
work correctly. E.g.  QuotientMod(0,0,m) never worked correctly, even
though by the definition given in the manual, 1 is a valid result.

Fixes #149
  • Loading branch information
fingolfin committed Dec 5, 2017
1 parent 82b11d6 commit d7df511
Show file tree
Hide file tree
Showing 6 changed files with 67 additions and 32 deletions.
16 changes: 10 additions & 6 deletions hpcgap/lib/integer.gi
Original file line number Diff line number Diff line change
Expand Up @@ -1668,15 +1668,19 @@ InstallMethod( QuotientMod,
true,
[ IsIntegers, IsInt, IsInt, IsInt ], 0,
function ( Integers, r, s, m )
if s > m then
s := s mod m;
fi;
local q;
if m = 1 then
return 0;
elif GcdInt( s, m ) <> 1 then
return fail;
else
return r/s mod m;
r := r mod m;
s := s mod m;
if r = s then return 1; fi; # this covers the case s = r = 0
if s = 0 then return fail; fi;
q := r / s;
if GcdInt( DenominatorRat(q), m ) <> 1 then
return fail;
fi;
return q mod m;
fi;
end );

Expand Down
16 changes: 10 additions & 6 deletions lib/integer.gi
Original file line number Diff line number Diff line change
Expand Up @@ -1641,15 +1641,19 @@ InstallMethod( QuotientMod,
true,
[ IsIntegers, IsInt, IsInt, IsInt ], 0,
function ( Integers, r, s, m )
if s > m then
s := s mod m;
fi;
local q;
if m = 1 then
return 0;
elif GcdInt( s, m ) <> 1 then
return fail;
else
return r/s mod m;
r := r mod m;
s := s mod m;
if r = s then return 1; fi; # this covers the case s = r = 0
if s = 0 then return fail; fi;
q := r / s;
if GcdInt( DenominatorRat(q), m ) <> 1 then
return fail;
fi;
return q mod m;
fi;
end );

Expand Down
18 changes: 8 additions & 10 deletions lib/ring.gd
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
10 changes: 0 additions & 10 deletions tst/testbugfix/2013-02-20-t00292.tst

This file was deleted.

32 changes: 32 additions & 0 deletions tst/testinstall/intarith.tst
Original file line number Diff line number Diff line change
Expand Up @@ -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
#
gap> QuotientMod(0, 0, 5);
1
gap> QuotientMod(0, 1, 5);
0
gap> QuotientMod(1, 0, 5);
fail
gap> QuotientMod(1, 1, 5);
1

#
gap> QuotientMod(5, 4, 0);
Error, Integer operations: <divisor> must be nonzero
gap> QuotientMod(5, 4, 1);
0
gap> QuotientMod(5, 4, 2);
fail
gap> QuotientMod(5, 4, 3);
2

#
gap> QuotientMod(2, 4, 6);
fail
gap> QuotientMod(4, 2, 6);
2
gap> QuotientMod(-2, 2, 6);
2
gap> QuotientMod(4, 8, 6);
2

#
# HexStringInt and IntHexString
#
Expand Down
7 changes: 7 additions & 0 deletions tst/testinstall/zmodnz.tst
Original file line number Diff line number Diff line change
Expand Up @@ -438,6 +438,13 @@ 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;
Error, ModRat: for <r>/<s> mod <n>, <s>/gcd(<r>,<s>) and <n> must be coprime
#
gap> STOP_TEST( "zmodnz.tst", 1);
Expand Down

0 comments on commit d7df511

Please sign in to comment.