From c53d6d42faa7e2d7a634943d4edf8038a3cfd5f8 Mon Sep 17 00:00:00 2001 From: BrianFugate Date: Wed, 31 Jan 2024 11:20:03 -0600 Subject: [PATCH 1/5] Fixing rounding bug from issue 3100 --- src/function/arithmetic/round.js | 34 ++++++++++++++++--- .../function/arithmetic/round.test.js | 4 +++ 2 files changed, 33 insertions(+), 5 deletions(-) diff --git a/src/function/arithmetic/round.js b/src/function/arithmetic/round.js index 5065dafcc9..ef14af8a6d 100644 --- a/src/function/arithmetic/round.js +++ b/src/function/arithmetic/round.js @@ -1,5 +1,7 @@ import { factory } from '../../utils/factory.js' import { deepMap } from '../../utils/collection.js' +import { nearlyEqual, splitNumber } from '../../utils/number.js' +import { nearlyEqual as bigNearlyEqual } from '../../utils/bignumber/nearlyEqual.js' import { createMatAlgo11xS0s } from '../../type/matrix/utils/matAlgo11xS0s.js' import { createMatAlgo12xSfs } from '../../type/matrix/utils/matAlgo12xSfs.js' import { createMatAlgo14xDs } from '../../type/matrix/utils/matAlgo14xDs.js' @@ -10,6 +12,7 @@ const NO_INT = 'Number of decimals in function round must be an integer' const name = 'round' const dependencies = [ 'typed', + 'config', 'matrix', 'equalScalar', 'zeros', @@ -17,10 +20,11 @@ const dependencies = [ 'DenseMatrix' ] -export const createRound = /* #__PURE__ */ factory(name, dependencies, ({ typed, matrix, equalScalar, zeros, BigNumber, DenseMatrix }) => { +export const createRound = /* #__PURE__ */ factory(name, dependencies, ({ typed, config, matrix, equalScalar, zeros, BigNumber, DenseMatrix }) => { const matAlgo11xS0s = createMatAlgo11xS0s({ typed, equalScalar }) const matAlgo12xSfs = createMatAlgo12xSfs({ typed, DenseMatrix }) const matAlgo14xDs = createMatAlgo14xDs({ typed }) + const epsilonExponent = Math.abs(splitNumber(config.epsilon).exponent) /** * Round a value towards the nearest rounded value. @@ -67,9 +71,21 @@ export const createRound = /* #__PURE__ */ factory(name, dependencies, ({ typed, * @return {number | BigNumber | Fraction | Complex | Array | Matrix} Rounded value */ return typed(name, { - number: roundNumber, + number: function (x) { + if (nearlyEqual(x, roundNumber(x, epsilonExponent), config.epsilon)) { + return roundNumber(roundNumber(x, epsilonExponent)) + } else { + return roundNumber + } + }, - 'number, number': roundNumber, + 'number, number': function (x, n) { + if (nearlyEqual(x, roundNumber(x, epsilonExponent), config.epsilon)) { + return roundNumber(roundNumber(x, epsilonExponent), n) + } else { + return roundNumber + } + }, 'number, BigNumber': function (x, n) { if (!n.isInteger()) { throw new TypeError(NO_INT) } @@ -95,13 +111,21 @@ export const createRound = /* #__PURE__ */ factory(name, dependencies, ({ typed, }, BigNumber: function (x) { - return x.toDecimalPlaces(0) + if (bigNearlyEqual(x, x.toDecimalPlaces(epsilonExponent), config.epsilon)) { + return x.toDecimalPlaces(epsilonExponent).toDecimalPlaces(0) + } else { + return x.toDecimalPlaces(0) + } }, 'BigNumber, BigNumber': function (x, n) { if (!n.isInteger()) { throw new TypeError(NO_INT) } - return x.toDecimalPlaces(n.toNumber()) + if (bigNearlyEqual(x, x.toDecimalPlaces(epsilonExponent), config.epsilon)) { + return x.toDecimalPlaces(epsilonExponent).toDecimalPlaces(n.toNumber()) + } else { + return x.toDecimalPlaces(n.toNumber()) + } }, Fraction: function (x) { diff --git a/test/unit-tests/function/arithmetic/round.test.js b/test/unit-tests/function/arithmetic/round.test.js index 624110cb80..2939413a78 100644 --- a/test/unit-tests/function/arithmetic/round.test.js +++ b/test/unit-tests/function/arithmetic/round.test.js @@ -65,6 +65,8 @@ describe('round', function () { }) it('should round bignumbers', function () { + assert.deepStrictEqual(round(bignumber(0.145 * 100)), bignumber(15)) + assert.deepStrictEqual(round(bignumber(0.145 * 100), bignumber(0)), bignumber(15)) assert.deepStrictEqual(round(bignumber(2.7)), bignumber(3)) assert.deepStrictEqual(round(bignumber(2.5)), bignumber(3)) assert.deepStrictEqual(round(bignumber(-2.5)), bignumber(-3)) @@ -92,6 +94,8 @@ describe('round', function () { }) it('should gracefully handle round-off errors', function () { + assert.strictEqual(round(0.145 * 100), 15) + assert.strictEqual(round((0.145 * 100), 0), 15) assert.strictEqual(round(3.0000000000000004), 3) assert.strictEqual(round(7.999999999999999), 8) assert.strictEqual(round(-3.0000000000000004), -3) From 89cf5598fc0526302ecfc958cd723dbd860bb1ca Mon Sep 17 00:00:00 2001 From: BrianFugate Date: Thu, 1 Feb 2024 10:11:02 -0600 Subject: [PATCH 2/5] Corrected syntax and converted if...else to logic using ternary operator --- src/function/arithmetic/round.js | 32 ++++++++++++-------------------- 1 file changed, 12 insertions(+), 20 deletions(-) diff --git a/src/function/arithmetic/round.js b/src/function/arithmetic/round.js index ef14af8a6d..c8c7465e2e 100644 --- a/src/function/arithmetic/round.js +++ b/src/function/arithmetic/round.js @@ -72,19 +72,15 @@ export const createRound = /* #__PURE__ */ factory(name, dependencies, ({ typed, */ return typed(name, { number: function (x) { - if (nearlyEqual(x, roundNumber(x, epsilonExponent), config.epsilon)) { - return roundNumber(roundNumber(x, epsilonExponent)) - } else { - return roundNumber - } + const xEpsilon = roundNumber(x, epsilonExponent) + const xSelected = nearlyEqual(x, xEpsilon, config.epsilon) ? xEpsilon : x + return roundNumber(xSelected) }, 'number, number': function (x, n) { - if (nearlyEqual(x, roundNumber(x, epsilonExponent), config.epsilon)) { - return roundNumber(roundNumber(x, epsilonExponent), n) - } else { - return roundNumber - } + const xEpsilon = roundNumber(x, epsilonExponent) + const xSelected = nearlyEqual(x, xEpsilon, config.epsilon) ? xEpsilon : x + return roundNumber(xSelected, n) }, 'number, BigNumber': function (x, n) { @@ -111,21 +107,17 @@ export const createRound = /* #__PURE__ */ factory(name, dependencies, ({ typed, }, BigNumber: function (x) { - if (bigNearlyEqual(x, x.toDecimalPlaces(epsilonExponent), config.epsilon)) { - return x.toDecimalPlaces(epsilonExponent).toDecimalPlaces(0) - } else { - return x.toDecimalPlaces(0) - } + const xEpsilon = new BigNumber(x).toDecimalPlaces(epsilonExponent) + const xSelected = bigNearlyEqual(x, xEpsilon, config.epsilon) ? xEpsilon : x + return xSelected.toDecimalPlaces(0) }, 'BigNumber, BigNumber': function (x, n) { if (!n.isInteger()) { throw new TypeError(NO_INT) } - if (bigNearlyEqual(x, x.toDecimalPlaces(epsilonExponent), config.epsilon)) { - return x.toDecimalPlaces(epsilonExponent).toDecimalPlaces(n.toNumber()) - } else { - return x.toDecimalPlaces(n.toNumber()) - } + const xEpsilon = new BigNumber(x).toDecimalPlaces(epsilonExponent) + const xSelected = bigNearlyEqual(x, xEpsilon, config.epsilon) ? xEpsilon : x + return xSelected.toDecimalPlaces(n.toNumber()) }, Fraction: function (x) { From 6dc1c8df93f372b327a5cc82c34c7a2379161c44 Mon Sep 17 00:00:00 2001 From: BrianFugate Date: Fri, 2 Feb 2024 12:04:56 -0600 Subject: [PATCH 3/5] Removing nearlyEqual comparison because a false return value was mathematically impossible by user input. Adding dynamic epsilon logic to cover cases when a user requests to round a number to a higher precision than epsilon in the config file. Also adding tests to cover dynamic epsilon cases. --- src/function/arithmetic/round.js | 27 ++++++++++--------- .../function/arithmetic/round.test.js | 9 +++++++ 2 files changed, 24 insertions(+), 12 deletions(-) diff --git a/src/function/arithmetic/round.js b/src/function/arithmetic/round.js index c8c7465e2e..da4d24ff26 100644 --- a/src/function/arithmetic/round.js +++ b/src/function/arithmetic/round.js @@ -1,7 +1,6 @@ import { factory } from '../../utils/factory.js' import { deepMap } from '../../utils/collection.js' -import { nearlyEqual, splitNumber } from '../../utils/number.js' -import { nearlyEqual as bigNearlyEqual } from '../../utils/bignumber/nearlyEqual.js' +import { splitNumber } from '../../utils/number.js' import { createMatAlgo11xS0s } from '../../type/matrix/utils/matAlgo11xS0s.js' import { createMatAlgo12xSfs } from '../../type/matrix/utils/matAlgo12xSfs.js' import { createMatAlgo14xDs } from '../../type/matrix/utils/matAlgo14xDs.js' @@ -72,15 +71,17 @@ export const createRound = /* #__PURE__ */ factory(name, dependencies, ({ typed, */ return typed(name, { number: function (x) { + // Handle round off errors by first rounding to epsilon precision const xEpsilon = roundNumber(x, epsilonExponent) - const xSelected = nearlyEqual(x, xEpsilon, config.epsilon) ? xEpsilon : x - return roundNumber(xSelected) + return roundNumber(xEpsilon) }, 'number, number': function (x, n) { - const xEpsilon = roundNumber(x, epsilonExponent) - const xSelected = nearlyEqual(x, xEpsilon, config.epsilon) ? xEpsilon : x - return roundNumber(xSelected, n) + // Same as number: unless user specifies more decimals than epsilon + const dynamicEpsilonExponent = (n < 15) ? (n + 1) : n + const epsilonExponentSelected = epsilonExponent > dynamicEpsilonExponent ? epsilonExponent : dynamicEpsilonExponent + const xEpsilon = roundNumber(x, epsilonExponentSelected) + return roundNumber(xEpsilon, n) }, 'number, BigNumber': function (x, n) { @@ -107,17 +108,19 @@ export const createRound = /* #__PURE__ */ factory(name, dependencies, ({ typed, }, BigNumber: function (x) { + // Handle round off errors by first rounding to epsilon precision const xEpsilon = new BigNumber(x).toDecimalPlaces(epsilonExponent) - const xSelected = bigNearlyEqual(x, xEpsilon, config.epsilon) ? xEpsilon : x - return xSelected.toDecimalPlaces(0) + return xEpsilon.toDecimalPlaces(0) }, 'BigNumber, BigNumber': function (x, n) { if (!n.isInteger()) { throw new TypeError(NO_INT) } - const xEpsilon = new BigNumber(x).toDecimalPlaces(epsilonExponent) - const xSelected = bigNearlyEqual(x, xEpsilon, config.epsilon) ? xEpsilon : x - return xSelected.toDecimalPlaces(n.toNumber()) + // Same as BigNumber: unless user specifies more decimals than epsilon + const dynamicEpsilonExponent = (n < 64) ? (n + 1) : n + const epsilonExponentSelected = epsilonExponent > dynamicEpsilonExponent ? new BigNumber(epsilonExponent) : new BigNumber(dynamicEpsilonExponent) + const xEpsilon = x.toDecimalPlaces(epsilonExponentSelected.toNumber()) + return xEpsilon.toDecimalPlaces(n.toNumber()) }, Fraction: function (x) { diff --git a/test/unit-tests/function/arithmetic/round.test.js b/test/unit-tests/function/arithmetic/round.test.js index 2939413a78..f1ee34290b 100644 --- a/test/unit-tests/function/arithmetic/round.test.js +++ b/test/unit-tests/function/arithmetic/round.test.js @@ -11,6 +11,10 @@ const sparse = math.sparse const round = math.round const unit = math.unit +// 1 2 3 4 5 6 +// xx.1234567890123456789012345678901234567890123456789012345678901234 +const testBigNum = bignumber('10.9999999999999999999999999999999999999999999999999999999999999998') + describe('round', function () { it('should round a number to te given number of decimals', function () { approx.equal(round(math.pi), 3) @@ -26,6 +30,9 @@ describe('round', function () { assert.strictEqual(round(-2.5), -3) assert.strictEqual(round(-2.7), -3) assert.strictEqual(round(-2.5, 0), -3) + assert.strictEqual(round(6.999999999999998, 15), 6.999999999999998) + assert.strictEqual(round(6.999999999999998, 14), 7) + assert.strictEqual(round(2.555555555555555, 13), 2.5555555555556) }) it('should round booleans (yeah, not really useful but it should be supported)', function () { @@ -67,6 +74,8 @@ describe('round', function () { it('should round bignumbers', function () { assert.deepStrictEqual(round(bignumber(0.145 * 100)), bignumber(15)) assert.deepStrictEqual(round(bignumber(0.145 * 100), bignumber(0)), bignumber(15)) + assert.deepStrictEqual(round(testBigNum, bignumber(63)), bignumber(11)) + assert.deepStrictEqual(round(testBigNum, bignumber(64)), testBigNum) assert.deepStrictEqual(round(bignumber(2.7)), bignumber(3)) assert.deepStrictEqual(round(bignumber(2.5)), bignumber(3)) assert.deepStrictEqual(round(bignumber(-2.5)), bignumber(-3)) From 4e8809dfcd1a2f59d9644d1bb22ea395c5034ba2 Mon Sep 17 00:00:00 2001 From: BrianFugate Date: Thu, 15 Feb 2024 08:52:55 -0600 Subject: [PATCH 4/5] Removing dynamic epsilon and adding test for changing config.epsilon during runtime --- src/function/arithmetic/round.js | 24 ++++++++++++------- .../function/arithmetic/round.test.js | 12 ++++++++++ 2 files changed, 27 insertions(+), 9 deletions(-) diff --git a/src/function/arithmetic/round.js b/src/function/arithmetic/round.js index da4d24ff26..1e37ee6328 100644 --- a/src/function/arithmetic/round.js +++ b/src/function/arithmetic/round.js @@ -23,7 +23,10 @@ export const createRound = /* #__PURE__ */ factory(name, dependencies, ({ typed, const matAlgo11xS0s = createMatAlgo11xS0s({ typed, equalScalar }) const matAlgo12xSfs = createMatAlgo12xSfs({ typed, DenseMatrix }) const matAlgo14xDs = createMatAlgo14xDs({ typed }) - const epsilonExponent = Math.abs(splitNumber(config.epsilon).exponent) + + function toExp (epsilon) { + return Math.abs(splitNumber(epsilon).exponent) + } /** * Round a value towards the nearest rounded value. @@ -72,15 +75,16 @@ export const createRound = /* #__PURE__ */ factory(name, dependencies, ({ typed, return typed(name, { number: function (x) { // Handle round off errors by first rounding to epsilon precision - const xEpsilon = roundNumber(x, epsilonExponent) + const xEpsilon = roundNumber(x, toExp(config.epsilon)) return roundNumber(xEpsilon) }, 'number, number': function (x, n) { // Same as number: unless user specifies more decimals than epsilon - const dynamicEpsilonExponent = (n < 15) ? (n + 1) : n - const epsilonExponentSelected = epsilonExponent > dynamicEpsilonExponent ? epsilonExponent : dynamicEpsilonExponent - const xEpsilon = roundNumber(x, epsilonExponentSelected) + const epsilonExponent = toExp(config.epsilon) + if (n >= epsilonExponent) { return roundNumber(x, n) } + + const xEpsilon = roundNumber(x, epsilonExponent) return roundNumber(xEpsilon, n) }, @@ -109,7 +113,7 @@ export const createRound = /* #__PURE__ */ factory(name, dependencies, ({ typed, BigNumber: function (x) { // Handle round off errors by first rounding to epsilon precision - const xEpsilon = new BigNumber(x).toDecimalPlaces(epsilonExponent) + const xEpsilon = new BigNumber(x).toDecimalPlaces(toExp(config.epsilon)) return xEpsilon.toDecimalPlaces(0) }, @@ -117,9 +121,11 @@ export const createRound = /* #__PURE__ */ factory(name, dependencies, ({ typed, if (!n.isInteger()) { throw new TypeError(NO_INT) } // Same as BigNumber: unless user specifies more decimals than epsilon - const dynamicEpsilonExponent = (n < 64) ? (n + 1) : n - const epsilonExponentSelected = epsilonExponent > dynamicEpsilonExponent ? new BigNumber(epsilonExponent) : new BigNumber(dynamicEpsilonExponent) - const xEpsilon = x.toDecimalPlaces(epsilonExponentSelected.toNumber()) + const epsilonExponent = toExp(config.epsilon) + + if (n >= epsilonExponent) { return x.toDecimalPlaces(n.toNumber()) } + + const xEpsilon = x.toDecimalPlaces(epsilonExponent) return xEpsilon.toDecimalPlaces(n.toNumber()) }, diff --git a/test/unit-tests/function/arithmetic/round.test.js b/test/unit-tests/function/arithmetic/round.test.js index f1ee34290b..4698f00a2d 100644 --- a/test/unit-tests/function/arithmetic/round.test.js +++ b/test/unit-tests/function/arithmetic/round.test.js @@ -10,6 +10,7 @@ const matrix = math.matrix const sparse = math.sparse const round = math.round const unit = math.unit +const math2 = math.create() // 1 2 3 4 5 6 // xx.1234567890123456789012345678901234567890123456789012345678901234 @@ -172,6 +173,17 @@ describe('round', function () { assert.deepStrictEqual(round(math.matrix([1.7, 2.3])).valueOf(), [2, 2]) }) + describe('changing config.epsilon during runtime', function () { + it('uses default config.epsilon of 1e-12', function () { + assert.strictEqual(math2.round((0.000000000001459), 12), 1e-12) + }) + + it('uses updated config.epsilon value', function () { + math2.config({ epsilon: 1e-13 }) + assert.strictEqual(math2.round((0.000000000001459), 12), 2e-12) + }) + }) + describe('Array', function () { it('should round array', function () { assert.deepStrictEqual(round([1.7, 2.3]), [2, 2]) From 68d4aa7543f8319f4bef0404bf3273f010ac82e8 Mon Sep 17 00:00:00 2001 From: BrianFugate Date: Wed, 21 Feb 2024 07:13:34 -0600 Subject: [PATCH 5/5] Reintroducing nearly equal verification for round function. Adding test case for changing epsilon at runtime. Both tests for changing epsilon at runtime also verify the false nearlyEqual scenario. --- src/function/arithmetic/round.js | 26 +++++++++++-------- .../function/arithmetic/round.test.js | 4 ++- 2 files changed, 18 insertions(+), 12 deletions(-) diff --git a/src/function/arithmetic/round.js b/src/function/arithmetic/round.js index 1e37ee6328..6bd4da261e 100644 --- a/src/function/arithmetic/round.js +++ b/src/function/arithmetic/round.js @@ -1,6 +1,7 @@ import { factory } from '../../utils/factory.js' import { deepMap } from '../../utils/collection.js' -import { splitNumber } from '../../utils/number.js' +import { nearlyEqual, splitNumber } from '../../utils/number.js' +import { nearlyEqual as bigNearlyEqual } from '../../utils/bignumber/nearlyEqual.js' import { createMatAlgo11xS0s } from '../../type/matrix/utils/matAlgo11xS0s.js' import { createMatAlgo12xSfs } from '../../type/matrix/utils/matAlgo12xSfs.js' import { createMatAlgo14xDs } from '../../type/matrix/utils/matAlgo14xDs.js' @@ -24,7 +25,7 @@ export const createRound = /* #__PURE__ */ factory(name, dependencies, ({ typed, const matAlgo12xSfs = createMatAlgo12xSfs({ typed, DenseMatrix }) const matAlgo14xDs = createMatAlgo14xDs({ typed }) - function toExp (epsilon) { + function toExponent (epsilon) { return Math.abs(splitNumber(epsilon).exponent) } @@ -75,17 +76,19 @@ export const createRound = /* #__PURE__ */ factory(name, dependencies, ({ typed, return typed(name, { number: function (x) { // Handle round off errors by first rounding to epsilon precision - const xEpsilon = roundNumber(x, toExp(config.epsilon)) - return roundNumber(xEpsilon) + const xEpsilon = roundNumber(x, toExponent(config.epsilon)) + const xSelected = nearlyEqual(x, xEpsilon, config.epsilon) ? xEpsilon : x + return roundNumber(xSelected) }, 'number, number': function (x, n) { // Same as number: unless user specifies more decimals than epsilon - const epsilonExponent = toExp(config.epsilon) + const epsilonExponent = toExponent(config.epsilon) if (n >= epsilonExponent) { return roundNumber(x, n) } const xEpsilon = roundNumber(x, epsilonExponent) - return roundNumber(xEpsilon, n) + const xSelected = nearlyEqual(x, xEpsilon, config.epsilon) ? xEpsilon : x + return roundNumber(xSelected, n) }, 'number, BigNumber': function (x, n) { @@ -113,20 +116,21 @@ export const createRound = /* #__PURE__ */ factory(name, dependencies, ({ typed, BigNumber: function (x) { // Handle round off errors by first rounding to epsilon precision - const xEpsilon = new BigNumber(x).toDecimalPlaces(toExp(config.epsilon)) - return xEpsilon.toDecimalPlaces(0) + const xEpsilon = new BigNumber(x).toDecimalPlaces(toExponent(config.epsilon)) + const xSelected = bigNearlyEqual(x, xEpsilon, config.epsilon) ? xEpsilon : x + return xSelected.toDecimalPlaces(0) }, 'BigNumber, BigNumber': function (x, n) { if (!n.isInteger()) { throw new TypeError(NO_INT) } // Same as BigNumber: unless user specifies more decimals than epsilon - const epsilonExponent = toExp(config.epsilon) - + const epsilonExponent = toExponent(config.epsilon) if (n >= epsilonExponent) { return x.toDecimalPlaces(n.toNumber()) } const xEpsilon = x.toDecimalPlaces(epsilonExponent) - return xEpsilon.toDecimalPlaces(n.toNumber()) + const xSelected = bigNearlyEqual(x, xEpsilon, config.epsilon) ? xEpsilon : x + return xSelected.toDecimalPlaces(n.toNumber()) }, Fraction: function (x) { diff --git a/test/unit-tests/function/arithmetic/round.test.js b/test/unit-tests/function/arithmetic/round.test.js index 4698f00a2d..04b841e401 100644 --- a/test/unit-tests/function/arithmetic/round.test.js +++ b/test/unit-tests/function/arithmetic/round.test.js @@ -176,11 +176,13 @@ describe('round', function () { describe('changing config.epsilon during runtime', function () { it('uses default config.epsilon of 1e-12', function () { assert.strictEqual(math2.round((0.000000000001459), 12), 1e-12) + assert.deepStrictEqual(math2.round(bignumber(1.49e-12), bignumber(12)), bignumber(1e-12)) }) it('uses updated config.epsilon value', function () { math2.config({ epsilon: 1e-13 }) - assert.strictEqual(math2.round((0.000000000001459), 12), 2e-12) + assert.strictEqual(math2.round((0.000000000001459), 12), 1e-12) + assert.deepStrictEqual(math2.round(bignumber(1.49e-12), bignumber(12)), bignumber(1e-12)) }) })