From d7df5117b8147786da623f713c5484c0719e56c5 Mon Sep 17 00:00:00 2001 From: Max Horn Date: Tue, 5 Dec 2017 16:44:44 +0100 Subject: [PATCH] Fix QuotientMod docs, and the integer implementation 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 --- hpcgap/lib/integer.gi | 16 ++++++++------ lib/integer.gi | 16 ++++++++------ lib/ring.gd | 18 +++++++--------- tst/testbugfix/2013-02-20-t00292.tst | 10 --------- tst/testinstall/intarith.tst | 32 ++++++++++++++++++++++++++++ tst/testinstall/zmodnz.tst | 7 ++++++ 6 files changed, 67 insertions(+), 32 deletions(-) delete mode 100644 tst/testbugfix/2013-02-20-t00292.tst diff --git a/hpcgap/lib/integer.gi b/hpcgap/lib/integer.gi index 31d9e1b5a93..dfc7b2ae82e 100644 --- a/hpcgap/lib/integer.gi +++ b/hpcgap/lib/integer.gi @@ -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 ); diff --git a/lib/integer.gi b/lib/integer.gi index 44d96a43f2e..79e77bca385 100644 --- a/lib/integer.gi +++ b/lib/integer.gi @@ -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 ); diff --git a/lib/ring.gd b/lib/ring.gd index ec075bceb9f..db708fe9216 100644 --- a/lib/ring.gd +++ b/lib/ring.gd @@ -1186,7 +1186,7 @@ DeclareOperation( "QuotientRemainder", ## ## ## -## returns the quotient of the ring +## returns a quotient of the ring ## elements r and s modulo the ring element m ## in the ring R, if given, ## and otherwise in their default ring, see @@ -1194,16 +1194,14 @@ DeclareOperation( "QuotientRemainder", ##

## R must be a Euclidean ring (see ) ## so that can be applied. -## If the modular quotient does not exist (i.e. when s and m -## are not coprime), fail is returned. +## If no modular quotient exists, fail is returned. ##

-## The quotient q of r and s modulo m is -## an element of R -## such that q s = r modulo m, i.e., -## such that q s - r is divisible by m in -## R and that q is either zero (if r is divisible by -## m) or the Euclidean degree of q is strictly smaller than -## the Euclidean degree of m. +## A quotient q of r and s modulo m is an +## element of R such that q s = r modulo +## m, i.e., such that q s - r is divisible by +## m in R and that q is either zero (if r is +## divisible by m) or the Euclidean degree of q is strictly +## smaller than the Euclidean degree of m. ## QuotientMod( 7, 2, 3 ); ## 2 diff --git a/tst/testbugfix/2013-02-20-t00292.tst b/tst/testbugfix/2013-02-20-t00292.tst deleted file mode 100644 index 6df90ea046d..00000000000 --- a/tst/testbugfix/2013-02-20-t00292.tst +++ /dev/null @@ -1,10 +0,0 @@ -# 2013/02/20 (AK) -gap> QuotientMod(4, 2, 6); -fail -gap> QuotientMod(2, 4, 6); -fail -gap> a := ZmodnZObj(2, 6);; b := ZmodnZObj(4, 6);; -gap> a/b; -fail -gap> b/a; -fail diff --git a/tst/testinstall/intarith.tst b/tst/testinstall/intarith.tst index 2c2b662987e..030298d867c 100644 --- a/tst/testinstall/intarith.tst +++ b/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 +# +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: 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 # diff --git a/tst/testinstall/zmodnz.tst b/tst/testinstall/zmodnz.tst index 0ddb5296551..0769c0b3de1 100644 --- a/tst/testinstall/zmodnz.tst +++ b/tst/testinstall/zmodnz.tst @@ -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 / mod , /gcd(,) and must be coprime + # gap> STOP_TEST( "zmodnz.tst", 1);