fix: handle bigints properly in more cases (#3345)

* fix: Handle bigints properly in more cases * Allow a bigint to appear in a max/min of nonhomogeneous arguments * If randomInt is called with a bigint or a pair of bigints, return a bigint. * Preserve uniformity of results if randomInt is called with a very large range. * Extend log, log2, and log10 to bigints * Add tests for all of the above issues. * Bonus: fix one JSDoc comment issue. If every PR fixes one, we will soon get through the 150+ * fix: Refactors per comments and 1 bonus doc test (isInteger) * chore: fix lint --------- Co-authored-by: Jos de Jong <[email protected]>
josdejong · Jan 24, 2025 · 8510b85 · 8510b85
1 parent b5d635e
commit 8510b85
Show file tree

Hide file tree

Showing 20 changed files with 224 additions and 47 deletions.
diff --git a/src/function/arithmetic/log.js b/src/function/arithmetic/log.js
@@ -1,8 +1,10 @@
 import { factory } from '../../utils/factory.js'
+import { promoteLogarithm } from '../../utils/bigint.js'
 import { logNumber } from '../../plain/number/index.js'
 
 const name = 'log'
 const dependencies = ['config', 'typed', 'typeOf', 'divideScalar', 'Complex']
+const nlg16 = Math.log(16)
 
 export const createLog = /* #__PURE__ */ factory(name, dependencies, ({ typed, typeOf, config, divideScalar, Complex }) => {
   /**
@@ -40,26 +42,34 @@ export const createLog = /* #__PURE__ */ factory(name, dependencies, ({ typed, t
    * @return {number | BigNumber | Fraction | Complex}
    *            Returns the logarithm of `x`
    */
+  function complexLog (c) {
+    return c.log()
+  }
+
+  function complexLogNumber (x) {
+    return complexLog(new Complex(x, 0))
+  }
+
   return typed(name, {
     number: function (x) {
       if (x >= 0 || config.predictable) {
         return logNumber(x)
       } else {
         // negative value -> complex value computation
-        return new Complex(x, 0).log()
+        return complexLogNumber(x)
       }
     },
 
-    Complex: function (x) {
-      return x.log()
-    },
+    bigint: promoteLogarithm(nlg16, logNumber, config, complexLogNumber),
+
+    Complex: complexLog,
 
     BigNumber: function (x) {
       if (!x.isNegative() || config.predictable) {
         return x.ln()
       } else {
         // downgrade to number, return Complex valued result
-        return new Complex(x.toNumber(), 0).log()
+        return complexLogNumber(x.toNumber())
       }
     },
 

diff --git a/src/function/arithmetic/log10.js b/src/function/arithmetic/log10.js
@@ -1,9 +1,11 @@
-import { factory } from '../../utils/factory.js'
-import { deepMap } from '../../utils/collection.js'
 import { log10Number } from '../../plain/number/index.js'
+import { promoteLogarithm } from '../../utils/bigint.js'
+import { deepMap } from '../../utils/collection.js'
+import { factory } from '../../utils/factory.js'
 
 const name = 'log10'
 const dependencies = ['typed', 'config', 'Complex']
+const log16 = log10Number(16)
 
 export const createLog10 = /* #__PURE__ */ factory(name, dependencies, ({ typed, config, Complex }) => {
   /**
@@ -31,26 +33,34 @@ export const createLog10 = /* #__PURE__ */ factory(name, dependencies, ({ typed,
    * @return {number | BigNumber | Complex | Array | Matrix}
    *            Returns the 10-base logarithm of `x`
    */
+
+  function complexLog (c) {
+    return c.log().div(Math.LN10)
+  }
+
+  function complexLogNumber (x) {
+    return complexLog(new Complex(x, 0))
+  }
   return typed(name, {
     number: function (x) {
       if (x >= 0 || config.predictable) {
         return log10Number(x)
       } else {
         // negative value -> complex value computation
-        return new Complex(x, 0).log().div(Math.LN10)
+        return complexLogNumber(x)
       }
     },
 
-    Complex: function (x) {
-      return new Complex(x).log().div(Math.LN10)
-    },
+    bigint: promoteLogarithm(log16, log10Number, config, complexLogNumber),
+
+    Complex: complexLog,
 
     BigNumber: function (x) {
       if (!x.isNegative() || config.predictable) {
         return x.log()
       } else {
         // downgrade to number, return Complex valued result
-        return new Complex(x.toNumber(), 0).log().div(Math.LN10)
+        return complexLogNumber(x.toNumber())
       }
     },
 

diff --git a/src/function/arithmetic/log2.js b/src/function/arithmetic/log2.js
@@ -1,6 +1,7 @@
-import { factory } from '../../utils/factory.js'
-import { deepMap } from '../../utils/collection.js'
 import { log2Number } from '../../plain/number/index.js'
+import { promoteLogarithm } from '../../utils/bigint.js'
+import { deepMap } from '../../utils/collection.js'
+import { factory } from '../../utils/factory.js'
 
 const name = 'log2'
 const dependencies = ['typed', 'config', 'Complex']
@@ -31,24 +32,30 @@ export const createLog2 = /* #__PURE__ */ factory(name, dependencies, ({ typed,
    * @return {number | BigNumber | Complex | Array | Matrix}
    *            Returns the 2-base logarithm of `x`
    */
+  function complexLog2Number (x) {
+    return _log2Complex(new Complex(x, 0))
+  }
+
   return typed(name, {
     number: function (x) {
       if (x >= 0 || config.predictable) {
         return log2Number(x)
       } else {
         // negative value -> complex value computation
-        return _log2Complex(new Complex(x, 0))
+        return complexLog2Number(x)
       }
     },
 
+    bigint: promoteLogarithm(4, log2Number, config, complexLog2Number),
+
     Complex: _log2Complex,
 
     BigNumber: function (x) {
       if (!x.isNegative() || config.predictable) {
         return x.log(2)
       } else {
         // downgrade to number, return Complex valued result
-        return _log2Complex(new Complex(x.toNumber(), 0))
+        return complexLog2Number(x.toNumber())
       }
     },
 

diff --git a/src/function/probability/randomInt.js b/src/function/probability/randomInt.js
@@ -4,9 +4,11 @@ import { createRng } from './util/seededRNG.js'
 import { isMatrix } from '../../utils/is.js'
 
 const name = 'randomInt'
-const dependencies = ['typed', 'config', '?on']
+const dependencies = ['typed', 'config', 'log2', '?on']
 
-export const createRandomInt = /* #__PURE__ */ factory(name, dependencies, ({ typed, config, on }) => {
+const simpleCutoff = 2n ** 30n
+
+export const createRandomInt = /* #__PURE__ */ factory(name, dependencies, ({ typed, config, log2, on }) => {
   // seeded pseudo random number generator
   let rng = createRng(config.randomSeed)
 
@@ -24,7 +26,7 @@ export const createRandomInt = /* #__PURE__ */ factory(name, dependencies, ({ ty
    *
    * Syntax:
    *
-   *     math.randomInt()                // generate a random integer between 0 and 1
+   *     math.randomInt()                // generate either 0 or 1, randomly
    *     math.randomInt(max)             // generate a random integer between 0 and max
    *     math.randomInt(min, max)        // generate a random integer between min and max
    *     math.randomInt(size)            // generate a matrix with random integer between 0 and 1
@@ -48,9 +50,11 @@ export const createRandomInt = /* #__PURE__ */ factory(name, dependencies, ({ ty
    * @return {number | Array | Matrix} A random integer value
    */
   return typed(name, {
-    '': () => _randomInt(0, 1),
+    '': () => _randomInt(0, 2),
     number: (max) => _randomInt(0, max),
     'number, number': (min, max) => _randomInt(min, max),
+    bigint: (max) => _randomBigint(0n, max),
+    'bigint, bigint': _randomBigint,
     'Array | Matrix': (size) => _randomIntMatrix(size, 0, 1),
     'Array | Matrix, number': (size, max) => _randomIntMatrix(size, 0, max),
     'Array | Matrix, number, number': (size, min, max) => _randomIntMatrix(size, min, max)
@@ -64,4 +68,23 @@ export const createRandomInt = /* #__PURE__ */ factory(name, dependencies, ({ ty
   function _randomInt (min, max) {
     return Math.floor(min + rng() * (max - min))
   }
+
+  function _randomBigint (min, max) {
+    const width = max - min // number of choices
+    if (width <= simpleCutoff) { // do it with number type
+      return min + BigInt(_randomInt(0, Number(width)))
+    }
+    // Too big to choose accurately that way. Instead, choose the correct
+    // number of random bits to cover the width, and repeat until the
+    // resulting number falls within the width
+    const bits = log2(width)
+    let picked = width
+    while (picked >= width) {
+      picked = 0n
+      for (let i = 0; i < bits; ++i) {
+        picked = 2n * picked + ((rng() < 0.5) ? 0n : 1n)
+      }
+    }
+    return min + picked
+  }
 })
diff --git a/src/function/relational/larger.js b/src/function/relational/larger.js
@@ -11,13 +11,14 @@ const name = 'larger'
 const dependencies = [
   'typed',
   'config',
+  'bignumber',
   'matrix',
   'DenseMatrix',
   'concat',
   'SparseMatrix'
 ]
 
-export const createLarger = /* #__PURE__ */ factory(name, dependencies, ({ typed, config, matrix, DenseMatrix, concat, SparseMatrix }) => {
+export const createLarger = /* #__PURE__ */ factory(name, dependencies, ({ typed, config, bignumber, matrix, DenseMatrix, concat, SparseMatrix }) => {
   const matAlgo03xDSf = createMatAlgo03xDSf({ typed })
   const matAlgo07xSSf = createMatAlgo07xSSf({ typed, SparseMatrix })
   const matAlgo12xSfs = createMatAlgo12xSfs({ typed, DenseMatrix })
@@ -55,20 +56,30 @@ export const createLarger = /* #__PURE__ */ factory(name, dependencies, ({ typed
    * @param  {number | BigNumber | bigint | Fraction | boolean | Unit | string | Array | Matrix} y Second value to compare
    * @return {boolean | Array | Matrix} Returns true when the x is larger than y, else returns false
    */
+  function bignumLarger (x, y) {
+    return x.gt(y) && !bigNearlyEqual(x, y, config.relTol, config.absTol)
+  }
+
   return typed(
     name,
     createLargerNumber({ typed, config }),
     {
       'boolean, boolean': (x, y) => x > y,
 
-      'BigNumber, BigNumber': function (x, y) {
-        return x.gt(y) && !bigNearlyEqual(x, y, config.relTol, config.absTol)
-      },
+      'BigNumber, BigNumber': bignumLarger,
 
       'bigint, bigint': (x, y) => x > y,
 
       'Fraction, Fraction': (x, y) => (x.compare(y) === 1),
 
+      'Fraction, BigNumber': function (x, y) {
+        return bignumLarger(bignumber(x), y)
+      },
+
+      'BigNumber, Fraction': function (x, y) {
+        return bignumLarger(x, bignumber(y))
+      },
+
       'Complex, Complex': function () {
         throw new TypeError('No ordering relation is defined for complex numbers')
       }

diff --git a/src/function/relational/smaller.js b/src/function/relational/smaller.js
@@ -11,13 +11,14 @@ const name = 'smaller'
 const dependencies = [
   'typed',
   'config',
+  'bignumber',
   'matrix',
   'DenseMatrix',
   'concat',
   'SparseMatrix'
 ]
 
-export const createSmaller = /* #__PURE__ */ factory(name, dependencies, ({ typed, config, matrix, DenseMatrix, concat, SparseMatrix }) => {
+export const createSmaller = /* #__PURE__ */ factory(name, dependencies, ({ typed, config, bignumber, matrix, DenseMatrix, concat, SparseMatrix }) => {
   const matAlgo03xDSf = createMatAlgo03xDSf({ typed })
   const matAlgo07xSSf = createMatAlgo07xSSf({ typed, SparseMatrix })
   const matAlgo12xSfs = createMatAlgo12xSfs({ typed, DenseMatrix })
@@ -55,20 +56,30 @@ export const createSmaller = /* #__PURE__ */ factory(name, dependencies, ({ type
    * @param  {number | BigNumber | bigint | Fraction | boolean | Unit | string | Array | Matrix} y Second value to compare
    * @return {boolean | Array | Matrix} Returns true when the x is smaller than y, else returns false
    */
+  function bignumSmaller (x, y) {
+    return x.lt(y) && !bigNearlyEqual(x, y, config.relTol, config.absTol)
+  }
+
   return typed(
     name,
     createSmallerNumber({ typed, config }),
     {
       'boolean, boolean': (x, y) => x < y,
 
-      'BigNumber, BigNumber': function (x, y) {
-        return x.lt(y) && !bigNearlyEqual(x, y, config.relTol, config.absTol)
-      },
+      'BigNumber, BigNumber': bignumSmaller,
 
       'bigint, bigint': (x, y) => x < y,
 
       'Fraction, Fraction': (x, y) => (x.compare(y) === -1),
 
+      'Fraction, BigNumber': function (x, y) {
+        return bignumSmaller(bignumber(x), y)
+      },
+
+      'BigNumber, Fraction': function (x, y) {
+        return bignumSmaller(x, bignumber(y))
+      },
+
       'Complex, Complex': function (x, y) {
         throw new TypeError('No ordering relation is defined for complex numbers')
       }

diff --git a/src/function/statistics/max.js b/src/function/statistics/max.js
@@ -83,7 +83,7 @@ export const createMax = /* #__PURE__ */ factory(name, dependencies, ({ typed, c
 
     deepForEach(array, function (value) {
       try {
-        if (isNaN(value) && typeof value === 'number') {
+        if (typeof value === 'number' && isNaN(value)) {
           res = NaN
         } else if (res === undefined || larger(value, res)) {
           res = value

diff --git a/src/function/statistics/min.js b/src/function/statistics/min.js
@@ -83,7 +83,7 @@ export const createMin = /* #__PURE__ */ factory(name, dependencies, ({ typed, c
 
     deepForEach(array, function (value) {
       try {
-        if (isNaN(value) && typeof value === 'number') {
+        if (typeof value === 'number' && isNaN(value)) {
           min = NaN
         } else if (min === undefined || smaller(value, min)) {
           min = value

diff --git a/src/function/string/print.js b/src/function/string/print.js
@@ -24,7 +24,7 @@ export const createPrint = /* #__PURE__ */ factory(name, dependencies, ({ typed
    *     // the following outputs: 'The value of pi is 3.141592654'
    *     math.print('The value of pi is $pi', {pi: math.pi}, 10)
    *
-   *     // the following outputs: 'hello Mary! The date is 2013-03-23'
+   *     // the following outputs: 'Hello Mary! The date is 2013-03-23'
    *     math.print('Hello $user.name! The date is $date', {
    *       user: {
    *         name: 'Mary',

diff --git a/src/function/utils/isInteger.js b/src/function/utils/isInteger.js
@@ -25,7 +25,7 @@ export const createIsInteger = /* #__PURE__ */ factory(name, dependencies, ({ ty
    *    math.isInteger(math.fraction(4))      // returns true
    *    math.isInteger('3')                   // returns true
    *    math.isInteger([3, 0.5, -2])          // returns [true, false, true]
-   *    math.isInteger(math.complex('2-4i'))  // throws an error
+   *    math.isInteger(math.complex('2-4i'))  // throws TypeError
    *
    * See also:
    *

diff --git a/src/utils/bigint.js b/src/utils/bigint.js
@@ -0,0 +1,27 @@
+/**
+ * Build a bigint logarithm function from a number logarithm,
+ * still returning a number. The idea is that 15 hexadecimal digits
+ * (60 bits) saturates the mantissa of the log, and each additional hex
+ * digit effectively just adds the log of 16 to the resulting value. So
+ * convert the most significant 15 hex digits to a number and take its
+ * log, and then add the log of 16 for each additional hex digit that
+ * was in the bigint.
+ * For negative numbers (complex logarithms), following the bignum
+ * implementation, it just downgrades to number and uses the complex result.
+ * @param {number} log16  the log of 16
+ * @param {(number) -> number} numberLog  the logarithm function for numbers
+ * @param {ConfigurationObject} config  the mathjs configuration
+ * @param {(number) -> Complex} cplx  the associated Complex log
+ * @returns {(bigint) -> number}   the corresponding logarithm for bigints
+ */
+export function promoteLogarithm (log16, numberLog, config, cplx) {
+  return function (b) {
+    if (b > 0 || config.predictable) {
+      if (b <= 0) return NaN
+      const s = b.toString(16)
+      const s15 = s.substring(0, 15)
+      return log16 * (s.length - s15.length) + numberLog(Number('0x' + s15))
+    }
+    return cplx(b.toNumber())
+  }
+}
diff --git a/test/node-tests/doc.test.js b/test/node-tests/doc.test.js
@@ -91,7 +91,7 @@ function extractValue (spec) {
 
 const knownProblems = new Set([
   'isZero', 'isPositive', 'isNumeric', 'isNegative', 'isNaN',
-  'isInteger', 'hasNumericValue', 'clone', 'print', 'hex', 'format', 'to', 'sin',
+  'hasNumericValue', 'clone', 'hex', 'format', 'to', 'sin',
   'cos', 'atan2', 'atan', 'asin', 'asec', 'acsc', 'acoth', 'acot', 'max',
   'setUnion', 'unequal', 'equal', 'deepEqual', 'compareNatural', 'randomInt',
   'random', 'pickRandom', 'kldivergence', 'xor', 'or', 'not', 'and', 'distance',
@@ -145,6 +145,14 @@ function checkExpectation (want, got) {
     }
     return approxEqual(got, want, 1e-9)
   }
+  if (
+    typeof want === 'string' &&
+    typeof got === 'string' &&
+    want.endsWith('Error') &&
+    got.startsWith(want)
+  ) {
+    return true // we obtained the expected error type
+  }
   if (typeof want !== 'undefined') {
     return approxDeepEqual(got, want)
   } else {