Skip to content

Commit

Permalink
Fix: #3100 function round not handling round-off errors (#3136)
Browse files Browse the repository at this point in the history
* Fixing rounding bug from issue 3100

* Corrected syntax and converted if...else to logic using ternary operator

* 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.

* Removing dynamic epsilon and adding test for changing config.epsilon during runtime

* 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.

---------

Co-authored-by: Jos de Jong <[email protected]>
  • Loading branch information
BrianFugate and josdejong authored Feb 22, 2024
1 parent 9baf478 commit 85b65da
Show file tree
Hide file tree
Showing 2 changed files with 61 additions and 5 deletions.
39 changes: 34 additions & 5 deletions src/function/arithmetic/round.js
Original file line number Diff line number Diff line change
@@ -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'
Expand All @@ -10,18 +12,23 @@ const NO_INT = 'Number of decimals in function round must be an integer'
const name = 'round'
const dependencies = [
'typed',
'config',
'matrix',
'equalScalar',
'zeros',
'BigNumber',
'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 })

function toExponent (epsilon) {
return Math.abs(splitNumber(epsilon).exponent)
}

/**
* Round a value towards the nearest rounded value.
* For matrices, the function is evaluated element wise.
Expand Down Expand Up @@ -67,9 +74,22 @@ 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) {
// Handle round off errors by first rounding to epsilon precision
const xEpsilon = roundNumber(x, toExponent(config.epsilon))
const xSelected = nearlyEqual(x, xEpsilon, config.epsilon) ? xEpsilon : x
return roundNumber(xSelected)
},

'number, number': roundNumber,
'number, number': function (x, n) {
// Same as number: unless user specifies more decimals than epsilon
const epsilonExponent = toExponent(config.epsilon)
if (n >= epsilonExponent) { return roundNumber(x, n) }

const xEpsilon = roundNumber(x, epsilonExponent)
const xSelected = nearlyEqual(x, xEpsilon, config.epsilon) ? xEpsilon : x
return roundNumber(xSelected, n)
},

'number, BigNumber': function (x, n) {
if (!n.isInteger()) { throw new TypeError(NO_INT) }
Expand All @@ -95,13 +115,22 @@ export const createRound = /* #__PURE__ */ factory(name, dependencies, ({ typed,
},

BigNumber: function (x) {
return x.toDecimalPlaces(0)
// Handle round off errors by first rounding to epsilon precision
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) }

return x.toDecimalPlaces(n.toNumber())
// Same as BigNumber: unless user specifies more decimals than epsilon
const epsilonExponent = toExponent(config.epsilon)
if (n >= epsilonExponent) { return x.toDecimalPlaces(n.toNumber()) }

const xEpsilon = x.toDecimalPlaces(epsilonExponent)
const xSelected = bigNearlyEqual(x, xEpsilon, config.epsilon) ? xEpsilon : x
return xSelected.toDecimalPlaces(n.toNumber())
},

Fraction: function (x) {
Expand Down
27 changes: 27 additions & 0 deletions test/unit-tests/function/arithmetic/round.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,11 @@ 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
const testBigNum = bignumber('10.9999999999999999999999999999999999999999999999999999999999999998')

describe('round', function () {
it('should round a number to te given number of decimals', function () {
Expand All @@ -26,6 +31,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 () {
Expand Down Expand Up @@ -65,6 +73,10 @@ 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))
Expand Down Expand Up @@ -92,6 +104,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)
Expand Down Expand Up @@ -159,6 +173,19 @@ 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)
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), 1e-12)
assert.deepStrictEqual(math2.round(bignumber(1.49e-12), bignumber(12)), bignumber(1e-12))
})
})

describe('Array', function () {
it('should round array', function () {
assert.deepStrictEqual(round([1.7, 2.3]), [2, 2])
Expand Down

0 comments on commit 85b65da

Please sign in to comment.