Fix round-off errors in mod() (#3011)

* changes made to the following files:

- mod.js

- gcd.js

* updated BigNumber implementation and added validating tests

* added validating test cases

* updated test cases

* formatted code

* Made updates according to requirement

used mathjs floor in mod.js

imported mod in gcd.js

made mod work for negative divisors

wrote and updated tests to validate new behavior

* updated mod in arithmetic.js

* added tests for modNumber function

---------

Co-authored-by: Jos de Jong <[email protected]>

AUTHORS

@@ -230,6 +230,7 @@ Michael Greminger <[email protected]>
 Kiku <[email protected]>
 MaybePixem <[email protected]>
 Aly Khaled <[email protected]>
+Praise Nnamonu <[email protected]>
 BuildTools <[email protected]>
 Anik Patel <[email protected]>
 Vrushaket Chaudhari <[email protected]>

src/function/arithmetic/gcd.js

@@ -1,16 +1,20 @@
+import { isInteger } from '../../utils/number.js'
 import { factory } from '../../utils/factory.js'
+import { createMod } from './mod.js'
 import { createMatAlgo01xDSid } from '../../type/matrix/utils/matAlgo01xDSid.js'
 import { createMatAlgo04xSidSid } from '../../type/matrix/utils/matAlgo04xSidSid.js'
 import { createMatAlgo10xSids } from '../../type/matrix/utils/matAlgo10xSids.js'
 import { createMatrixAlgorithmSuite } from '../../type/matrix/utils/matrixAlgorithmSuite.js'
-import { gcdNumber } from '../../plain/number/index.js'
 import { ArgumentsError } from '../../error/ArgumentsError.js'
 
 const name = 'gcd'
 const dependencies = [
   'typed',
+  'config',
+  'round',
   'matrix',
   'equalScalar',
+  'zeros',
   'BigNumber',
   'DenseMatrix',
   'concat'
@@ -23,7 +27,8 @@ function is1d (array) {
   return !array.some(element => Array.isArray(element))
 }
 
-export const createGcd = /* #__PURE__ */ factory(name, dependencies, ({ typed, matrix, equalScalar, BigNumber, DenseMatrix, concat }) => {
+export const createGcd = /* #__PURE__ */ factory(name, dependencies, ({ typed, matrix, config, round, equalScalar, zeros, BigNumber, DenseMatrix, concat }) => {
+  const mod = createMod({ typed, config, round, matrix, equalScalar, zeros, DenseMatrix, concat })
   const matAlgo01xDSid = createMatAlgo01xDSid({ typed })
   const matAlgo04xSidSid = createMatAlgo04xSidSid({ typed, equalScalar })
   const matAlgo10xSids = createMatAlgo10xSids({ typed, DenseMatrix })
@@ -57,7 +62,7 @@ export const createGcd = /* #__PURE__ */ factory(name, dependencies, ({ typed, m
   return typed(
     name,
     {
-      'number, number': gcdNumber,
+      'number, number': _gcdNumber,
       'BigNumber, BigNumber': _gcdBigNumber,
       'Fraction, Fraction': (x, y) => x.gcd(y)
     },
@@ -89,6 +94,28 @@ export const createGcd = /* #__PURE__ */ factory(name, dependencies, ({ typed, m
     }
   )
 
+  /**
+ * Calculate gcd for numbers
+ * @param {number} a
+ * @param {number} b
+ * @returns {number} Returns the greatest common denominator of a and b
+ * @private
+ */
+  function _gcdNumber (a, b) {
+    if (!isInteger(a) || !isInteger(b)) {
+      throw new Error('Parameters in function gcd must be integer numbers')
+    }
+
+    // https://en.wikipedia.org/wiki/Euclidean_algorithm
+    let r
+    while (b !== 0) {
+      r = mod(a, b)
+      a = b
+      b = r
+    }
+    return (a < 0) ? -a : a
+  }
+
   /**
    * Calculate gcd for BigNumbers
    * @param {BigNumber} a
@@ -104,7 +131,7 @@ export const createGcd = /* #__PURE__ */ factory(name, dependencies, ({ typed, m
     // https://en.wikipedia.org/wiki/Euclidean_algorithm
     const zero = new BigNumber(0)
     while (!b.isZero()) {
-      const r = a.mod(b)
+      const r = mod(a, b)
       a = b
       b = r
     }

src/function/arithmetic/mod.js

@@ -1,22 +1,26 @@
 import { factory } from '../../utils/factory.js'
+import { createFloor } from './floor.js'
 import { createMatAlgo02xDS0 } from '../../type/matrix/utils/matAlgo02xDS0.js'
 import { createMatAlgo03xDSf } from '../../type/matrix/utils/matAlgo03xDSf.js'
 import { createMatAlgo05xSfSf } from '../../type/matrix/utils/matAlgo05xSfSf.js'
 import { createMatAlgo11xS0s } from '../../type/matrix/utils/matAlgo11xS0s.js'
 import { createMatAlgo12xSfs } from '../../type/matrix/utils/matAlgo12xSfs.js'
-import { modNumber } from '../../plain/number/index.js'
 import { createMatrixAlgorithmSuite } from '../../type/matrix/utils/matrixAlgorithmSuite.js'
 
 const name = 'mod'
 const dependencies = [
   'typed',
+  'config',
+  'round',
   'matrix',
   'equalScalar',
+  'zeros',
   'DenseMatrix',
   'concat'
 ]
 
-export const createMod = /* #__PURE__ */ factory(name, dependencies, ({ typed, matrix, equalScalar, DenseMatrix, concat }) => {
+export const createMod = /* #__PURE__ */ factory(name, dependencies, ({ typed, config, round, matrix, equalScalar, zeros, DenseMatrix, concat }) => {
+  const floor = createFloor({ typed, config, round, matrix, equalScalar, zeros, DenseMatrix })
   const matAlgo02xDS0 = createMatAlgo02xDS0({ typed, equalScalar })
   const matAlgo03xDSf = createMatAlgo03xDSf({ typed })
   const matAlgo05xSfSf = createMatAlgo05xSfSf({ typed, equalScalar })
@@ -62,13 +66,12 @@ export const createMod = /* #__PURE__ */ factory(name, dependencies, ({ typed, m
   return typed(
     name,
     {
-      'number, number': modNumber,
+      'number, number': _modNumber,
 
       'BigNumber, BigNumber': function (x, y) {
         if (y.isNeg()) {
           throw new Error('Cannot calculate mod for a negative divisor')
-        }
-        return y.isZero() ? x : x.mod(y)
+        } return y.isZero() ? x : x.mod(y)
       },
 
       'Fraction, Fraction': function (x, y) {
@@ -87,4 +90,21 @@ export const createMod = /* #__PURE__ */ factory(name, dependencies, ({ typed, m
       sS: matAlgo12xSfs
     })
   )
+
+  /**
+ * Calculate the modulus of two numbers
+ * @param {number} x
+ * @param {number} y
+ * @returns {number} res
+ * @private
+ */
+  function _modNumber (x, y) {
+    // We don't use JavaScript's % operator here as this doesn't work
+    // correctly for x < 0 and x === 0
+    // see https://en.wikipedia.org/wiki/Modulo_operation
+
+    // We use mathjs floor to handle errors associated with
+    // precision float approximation
+    return (y === 0) ? x : x - y * floor(x / y)
+  }
 })

src/plain/number/arithmetic.js

@@ -157,17 +157,10 @@ log1pNumber.signature = n1
  * @private
  */
 export function modNumber (x, y) {
-  if (y > 0) {
-    // We don't use JavaScript's % operator here as this doesn't work
-    // correctly for x < 0 and x === 0
-    // see https://en.wikipedia.org/wiki/Modulo_operation
-    return x - y * Math.floor(x / y)
-  } else if (y === 0) {
-    return x
-  } else { // y < 0
-    // TODO: implement mod for a negative divisor
-    throw new Error('Cannot calculate mod for a negative divisor')
-  }
+  // We don't use JavaScript's % operator here as this doesn't work
+  // correctly for x < 0 and x === 0
+  // see https://en.wikipedia.org/wiki/Modulo_operation
+  return (y === 0) ? x : x - y * Math.floor(x / y)
 }
 modNumber.signature = n2
 

test/node-tests/plain/number/arithmetic.test.js

@@ -0,0 +1,55 @@
+// test modNumber function in src\plain\number\arithmetic.js
+import assert from 'assert'
+import { modNumber as mod } from '../../../../lib/cjs/plain/number/aristhmetic'
+import approx from '../../../../tools/approx.js'
+
+const math = require('../../../../lib/cjs/defaultInstance').default
+const bignumber = math.bignumber
+
+describe('mod', function () {
+  it('should calculate the modulus of booleans correctly', function () {
+    assert.strictEqual(mod(true, true), 0)
+    assert.strictEqual(mod(false, true), 0)
+    assert.strictEqual(mod(true, false), 1)
+    assert.strictEqual(mod(false, false), 0)
+  })
+
+  it('should calculate the modulus of two numbers', function () {
+    assert.strictEqual(mod(1, 1), 0)
+    assert.strictEqual(mod(0, 1), 0)
+    assert.strictEqual(mod(1, 0), 1)
+    assert.strictEqual(mod(0, 0), 0)
+    assert.strictEqual(mod(7, 0), 7)
+
+    approx.equal(mod(7, 2), 1)
+    approx.equal(mod(9, 3), 0)
+    approx.equal(mod(10, 4), 2)
+    approx.equal(mod(-10, 4), 2)
+    approx.equal(mod(8.2, 3), 2.2)
+    approx.equal(mod(4, 1.5), 1)
+    approx.equal(mod(0, 3), 0)
+    approx.equal(mod(-10, 4), 2)
+    approx.equal(mod(-5, 3), 1)
+  })
+
+  it('should handle precise approximation of float approximation', function () {
+    approx.equal(mod(0.1, 0.01), 0)
+    approx.equal(mod(0.15, 0.05), 0)
+    approx.equal(mod(1.23456789, 0.00000000001), 0)
+  })
+
+  it('should calculate mod for negative divisor', function () {
+    assert.strictEqual(mod(10, -4), -2)
+  })
+
+  it('should throw an error if used with wrong number of arguments', function () {
+    assert.throws(function () { mod(1) }, /TypeError: Too few arguments/)
+    assert.throws(function () { mod(1, 2, 3) }, /TypeError: Too many arguments/)
+  })
+
+  it('should throw an error if used with wrong type of arguments', function () {
+    assert.throws(function () { mod(1, new Date()) }, /TypeError: Unexpected type of argument/)
+    assert.throws(function () { mod(1, null) }, /TypeError: Unexpected type of argument/)
+    assert.throws(function () { mod(new Date(), bignumber(2)) }, /TypeError: Unexpected type of argument/)
+  })
+})

test/unit-tests/expression/parse.test.js

@@ -1234,7 +1234,10 @@ describe('parse', function () {
     it('should parse mod %', function () {
       approx.equal(parseAndEval('8 % 3'), 2)
       approx.equal(parseAndEval('80% pi'), 1.4601836602551685)
-      assert.throws(function () { parseAndEval('3%(-100)') }, /Cannot calculate mod for a negative divisor/)
+    })
+
+    it('should parse mod % for negative divisors', function () {
+      assert.strictEqual(parseAndEval('3%(-100)'), -97)
     })
 
     it('should parse % value', function () {

test/unit-tests/function/arithmetic/mod.test.js

@@ -34,8 +34,14 @@ describe('mod', function () {
     approx.equal(mod(-5, 3), 1)
   })
 
-  it('should throw an error if the divisor is negative', function () {
-    assert.throws(function () { mod(10, -4) })
+  it('should handle precise approximation of float approximation', function () {
+    approx.equal(mod(0.1, 0.01), 0)
+    approx.equal(mod(0.15, 0.05), 0)
+    approx.equal(mod(1.23456789, 0.00000000001), 0)
+  })
+
+  it('should calculate mod for negative divisor', function () {
+    assert.strictEqual(mod(10, -4), -2)
   })
 
   it('should throw an error if used with wrong number of arguments', function () {