Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes #3100 Bug on simple rounding #3136

Merged
merged 11 commits into from
Feb 22, 2024
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 24 additions & 5 deletions src/function/arithmetic/round.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { factory } from '../../utils/factory.js'
import { deepMap } from '../../utils/collection.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'
Expand All @@ -10,17 +11,19 @@ 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 })
const epsilonExponent = Math.abs(splitNumber(config.epsilon).exponent)
josdejong marked this conversation as resolved.
Show resolved Hide resolved

/**
* Round a value towards the nearest rounded value.
Expand Down Expand Up @@ -67,9 +70,19 @@ 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, epsilonExponent)
return roundNumber(xEpsilon)
},

'number, number': roundNumber,
'number, number': function (x, n) {
// Same as number: unless user specifies more decimals than epsilon
const dynamicEpsilonExponent = (n < 15) ? (n + 1) : n
josdejong marked this conversation as resolved.
Show resolved Hide resolved
const epsilonExponentSelected = epsilonExponent > dynamicEpsilonExponent ? epsilonExponent : dynamicEpsilonExponent
const xEpsilon = roundNumber(x, epsilonExponentSelected)
return roundNumber(xEpsilon, n)
},

'number, BigNumber': function (x, n) {
if (!n.isInteger()) { throw new TypeError(NO_INT) }
Expand All @@ -95,13 +108,19 @@ 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(epsilonExponent)
return xEpsilon.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 dynamicEpsilonExponent = (n < 64) ? (n + 1) : n
const epsilonExponentSelected = epsilonExponent > dynamicEpsilonExponent ? new BigNumber(epsilonExponent) : new BigNumber(dynamicEpsilonExponent)
josdejong marked this conversation as resolved.
Show resolved Hide resolved
const xEpsilon = x.toDecimalPlaces(epsilonExponentSelected.toNumber())
return xEpsilon.toDecimalPlaces(n.toNumber())
},

Fraction: function (x) {
Expand Down
13 changes: 13 additions & 0 deletions test/unit-tests/function/arithmetic/round.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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 () {
Expand Down Expand Up @@ -65,6 +72,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))
josdejong marked this conversation as resolved.
Show resolved Hide resolved
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 +103,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