Skip to content

Commit

Permalink
fix: #3360 add bigint support to matrix indices and ranges (#3361)
Browse files Browse the repository at this point in the history
* fix: Accept bigints as matrix indices and range params, demoting to number

* feat: bigint support in range(), clarify demotion in Range()

* feat: range() support Fraction, better errors for single non-string argument

---------

Co-authored-by: Jos de Jong <[email protected]>
  • Loading branch information
gwhitney and josdejong authored Jan 30, 2025
1 parent 773a5c5 commit 1a85b87
Show file tree
Hide file tree
Showing 9 changed files with 170 additions and 27 deletions.
8 changes: 5 additions & 3 deletions src/expression/transform/index.transform.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import { isArray, isBigNumber, isMatrix, isNumber, isRange } from '../../utils/is.js'
import {
isArray, isBigInt, isBigNumber, isMatrix, isNumber, isRange
} from '../../utils/is.js'
import { factory } from '../../utils/factory.js'

const name = 'index'
Expand Down Expand Up @@ -26,14 +28,14 @@ export const createIndexTransform = /* #__PURE__ */ factory(name, dependencies,
if (getMatrixDataType(arg) !== 'boolean') {
arg = arg.map(function (v) { return v - 1 })
}
} else if (isNumber(arg)) {
} else if (isNumber(arg) || isBigInt(arg)) {
arg--
} else if (isBigNumber(arg)) {
arg = arg.toNumber() - 1
} else if (typeof arg === 'string') {
// leave as is
} else {
throw new TypeError('Dimension must be an Array, Matrix, number, string, or Range')
throw new TypeError('Dimension must be an Array, Matrix, number, bigint, string, or Range')
}

args[i] = arg
Expand Down
64 changes: 59 additions & 5 deletions src/function/matrix/range.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,37 +25,51 @@ export const createRange = /* #__PURE__ */ factory(name, dependencies, ({ typed,
*
* - `str: string`
* A string 'start:end' or 'start:step:end'
* - `start: {number | BigNumber | Unit}`
* - `start: {number | bigint | BigNumber | Fraction | Unit}`
* Start of the range
* - `end: number | BigNumber | Unit`
* - `end: number | bigint | BigNumber | Fraction | Unit`
* End of the range, excluded by default, included when parameter includeEnd=true
* - `step: number | BigNumber | Unit`
* - `step: number | bigint | BigNumber | Fraction | Unit`
* Step size. Default value is 1.
* - `includeEnd: boolean`
* Option to specify whether to include the end or not. False by default.
*
* Note that the return type of the range is taken from the type of
* the start/end. If only one these is a built-in `number` type, it will
* be promoted to the type of the other endpoint. However, in the case of
* Unit values, both endpoints must have compatible units, and the return
* value will have compatible units as well.
*
* Examples:
*
* math.range(2, 6) // [2, 3, 4, 5]
* math.range(2, -3, -1) // [2, 1, 0, -1, -2]
* math.range('2:1:6') // [2, 3, 4, 5]
* math.range(2, 6, true) // [2, 3, 4, 5, 6]
* math.range(2, math.fraction(8,3), math.fraction(1,3)) // [fraction(2), fraction(7,3)]
* math.range(math.unit(2, 'm'), math.unit(-3, 'm'), math.unit(-1, 'm')) // [2 m, 1 m, 0 m , -1 m, -2 m]
*
* See also:
*
* ones, zeros, size, subset
*
* @param {*} args Parameters describing the ranges `start`, `end`, and optional `step`.
* @param {*} args Parameters describing the range's `start`, `end`, and optional `step`.
* @return {Array | Matrix} range
*/
return typed(name, {
// TODO: simplify signatures when typed-function supports default values and optional arguments

// TODO: a number or boolean should not be converted to string here
string: _strRange,
'string, boolean': _strRange,

number: function (oops) {
throw new TypeError(`Too few arguments to function range(): ${oops}`)
},

boolean: function (oops) {
throw new TypeError(`Unexpected type of argument 1 to function range(): ${oops}, number|bigint|BigNumber|Fraction`)
},

'number, number': function (start, end) {
return _out(_range(start, end, 1, false))
},
Expand All @@ -69,6 +83,32 @@ export const createRange = /* #__PURE__ */ factory(name, dependencies, ({ typed,
return _out(_range(start, end, step, includeEnd))
},

// Handle bigints; if either limit is bigint, range should be too
'bigint, bigint|number': function (start, end) {
return _out(_range(start, end, 1n, false))
},
'number, bigint': function (start, end) {
return _out(_range(BigInt(start), end, 1n, false))
},
'bigint, bigint|number, bigint|number': function (start, end, step) {
return _out(_range(start, end, BigInt(step), false))
},
'number, bigint, bigint|number': function (start, end, step) {
return _out(_range(BigInt(start), end, BigInt(step), false))
},
'bigint, bigint|number, boolean': function (start, end, includeEnd) {
return _out(_range(start, end, 1n, includeEnd))
},
'number, bigint, boolean': function (start, end, includeEnd) {
return _out(_range(BigInt(start), end, 1n, includeEnd))
},
'bigint, bigint|number, bigint|number, boolean': function (start, end, step, includeEnd) {
return _out(_range(start, end, BigInt(step), includeEnd))
},
'number, bigint, bigint|number, boolean': function (start, end, step, includeEnd) {
return _out(_range(BigInt(start), end, BigInt(step), includeEnd))
},

'BigNumber, BigNumber': function (start, end) {
const BigNumber = start.constructor

Expand All @@ -85,6 +125,20 @@ export const createRange = /* #__PURE__ */ factory(name, dependencies, ({ typed,
'BigNumber, BigNumber, BigNumber, boolean': function (start, end, step, includeEnd) {
return _out(_range(start, end, step, includeEnd))
},

'Fraction, Fraction': function (start, end) {
return _out(_range(start, end, 1, false))
},
'Fraction, Fraction, Fraction': function (start, end, step) {
return _out(_range(start, end, step, false))
},
'Fraction, Fraction, boolean': function (start, end, includeEnd) {
return _out(_range(start, end, 1, includeEnd))
},
'Fraction, Fraction, Fraction, boolean': function (start, end, step, includeEnd) {
return _out(_range(start, end, step, includeEnd))
},

'Unit, Unit, Unit': function (start, end, step) {
return _out(_range(start, end, step, false))
},
Expand Down
9 changes: 6 additions & 3 deletions src/type/matrix/MatrixIndex.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ export const createIndexClass = /* #__PURE__ */ factory(name, dependencies, ({ I
const arg = arguments[i]
const argIsArray = isArray(arg)
const argIsMatrix = isMatrix(arg)
const argType = typeof arg
let sourceSize = null
if (isRange(arg)) {
this._dimensions.push(arg)
Expand All @@ -65,13 +66,15 @@ export const createIndexClass = /* #__PURE__ */ factory(name, dependencies, ({ I
if (size.length !== 1 || size[0] !== 1 || sourceSize !== null) {
this._isScalar = false
}
} else if (typeof arg === 'number') {
} else if (argType === 'number') {
this._dimensions.push(_createImmutableMatrix([arg]))
} else if (typeof arg === 'string') {
} else if (argType === 'bigint') {
this._dimensions.push(_createImmutableMatrix([Number(arg)]))
} else if (argType === 'string') {
// object property (arguments.count should be 1)
this._dimensions.push(arg)
} else {
throw new TypeError('Dimension must be an Array, Matrix, number, string, or Range')
throw new TypeError('Dimension must be an Array, Matrix, number, bigint, string, or Range')
}
this._sourceSize.push(sourceSize)
// TODO: implement support for wildcard '*'
Expand Down
25 changes: 16 additions & 9 deletions src/type/matrix/Range.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { isBigNumber } from '../../utils/is.js'
import { isBigInt, isBigNumber } from '../../utils/is.js'
import { format, sign } from '../../utils/number.js'
import { factory } from '../../utils/factory.js'

Expand All @@ -7,14 +7,21 @@ const dependencies = []

export const createRangeClass = /* #__PURE__ */ factory(name, dependencies, () => {
/**
* Create a range. A range has a start, step, and end, and contains functions
* to iterate over the range.
* Create a range of numbers. A range has a start, step, and end,
* and contains functions to iterate over the range.
*
* A range can be constructed as:
*
* const range = new Range(start, end)
* const range = new Range(start, end, step)
*
* Note that the endpoints and step may be specified with other numeric
* types such as bigint or BigNumber, but they will be demoted to the
* built-in `number` type and the Range will only contain numbers. The
* rationale for this demotion is that Range objects are primarily used
* for indexing Matrix objects, and Matrix objects may only be indexed
* with `number`s.
*
* To get the result of the range:
* range.forEach(function (x) {
* console.log(x)
Expand Down Expand Up @@ -49,22 +56,22 @@ export const createRangeClass = /* #__PURE__ */ factory(name, dependencies, () =
if (hasStart) {
if (isBigNumber(start)) {
start = start.toNumber()
} else if (typeof start !== 'number') {
throw new TypeError('Parameter start must be a number')
} else if (typeof start !== 'number' && !isBigInt(start)) {
throw new TypeError('Parameter start must be a number or bigint')
}
}
if (hasEnd) {
if (isBigNumber(end)) {
end = end.toNumber()
} else if (typeof end !== 'number') {
throw new TypeError('Parameter end must be a number')
} else if (typeof end !== 'number' && !isBigInt(end)) {
throw new TypeError('Parameter end must be a number or bigint')
}
}
if (hasStep) {
if (isBigNumber(step)) {
step = step.toNumber()
} else if (typeof step !== 'number') {
throw new TypeError('Parameter step must be a number')
} else if (typeof step !== 'number' && !isBigInt(step)) {
throw new TypeError('Parameter step must be a number or bigint')
}
}

Expand Down
4 changes: 2 additions & 2 deletions test/unit-tests/expression/node/IndexNode.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,8 @@ describe('IndexNode', function () {
})

it('should run forEach on an IndexNode', function () {
const b = new ConstantNode(2)
const c = new ConstantNode(1)
const b = new ConstantNode(2n)
const c = new ConstantNode(1n)
const n = new IndexNode([b, c])

const nodes = []
Expand Down
8 changes: 7 additions & 1 deletion test/unit-tests/expression/parse.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -829,7 +829,7 @@ describe('parse', function () {

assert.throws(function () {
parseAndEval('a[2, 2+3i]', scope)
}, /TypeError: Dimension must be an Array, Matrix, number, string, or Range/)
}, /TypeError: Dimension must be an Array,.*or Range/)
})

it('should throw an error for invalid matrix', function () {
Expand Down Expand Up @@ -1478,6 +1478,12 @@ describe('parse', function () {
assert.throws(function () { parseAndEval('2[1,2,3]') }, /Unexpected operator/)// index
})

it('should index when the number config is bigint', function () {
const bimath = math.create({ number: 'bigint' })
assert.strictEqual(bimath.evaluate('[1,2;3,4][2,2]'), 4n)
assert.strictEqual(bimath.evaluate('[5,6,7][2]'), 6n)
})

it('should tell the OperatorNode about implicit multiplications', function () {
assert.strictEqual(parse('2 + 3').implicit, false)
assert.strictEqual(parse('4 * a').implicit, false)
Expand Down
62 changes: 58 additions & 4 deletions test/unit-tests/function/matrix/range.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,24 @@ describe('range', function () {
assert.deepStrictEqual(math2.range(5, 0, -1), [5, 4, 3, 2, 1])
})

it('should create a range with bigints', function () {
assert.deepStrictEqual(range(1n, 3n), matrix([1n, 2n]))
assert.deepStrictEqual(range(3n, 1n, -1n), matrix([3n, 2n]))
assert.deepStrictEqual(range(1n, 3n, true), matrix([1n, 2n, 3n]))
assert.deepStrictEqual(range(3n, 1n, -1n, true), matrix([3n, 2n, 1n]))
})

it('should handle mixed numbers and bigints appropriately', function () {
assert.deepStrictEqual(range(1n, 3), matrix([1n, 2n]))
assert.deepStrictEqual(range(3, 1n, -1n), matrix([3n, 2n]))
assert.deepStrictEqual(range(3n, 1, -1), matrix([3n, 2n]))
assert.deepStrictEqual(range(1, 3n, true), matrix([1n, 2n, 3n]))
assert.deepStrictEqual(range(3n, 1, -1n, true), matrix([3n, 2n, 1n]))
assert.deepStrictEqual(range(3, 1n, -1, true), matrix([3n, 2n, 1n]))
assert.deepStrictEqual(range(1, 5, 2n), matrix([1, 3]))
assert.deepStrictEqual(range(5, 1, -2n, true), matrix([5, 3, 1]))
})

it('should create a range with bignumbers', function () {
assert.deepStrictEqual(range(bignumber(1), bignumber(3)), matrix([bignumber(1), bignumber(2)]))
assert.deepStrictEqual(range(bignumber(3), bignumber(1), bignumber(-1)), matrix([bignumber(3), bignumber(2)]))
Expand Down Expand Up @@ -132,6 +150,38 @@ describe('range', function () {
assert.deepStrictEqual(range(bignumber(3), bignumber(1), bignumber(-1), true), matrix([bignumber(3), bignumber(2), bignumber(1)]))
})

it('should handle Fractions', function () {
const frac = math.fraction
assert.deepStrictEqual(
range(frac(1, 3), frac(10, 3)),
matrix([frac(1, 3), frac(4, 3), frac(7, 3)]))
assert.deepStrictEqual(
range(frac(1, 3), frac(7, 3), true),
matrix([frac(1, 3), frac(4, 3), frac(7, 3)]))
assert.deepStrictEqual(
range(frac(1, 3), frac(4, 3), frac(1, 3)),
matrix([frac(1, 3), frac(2, 3), frac(1)]))
assert.deepStrictEqual(
range(frac(1, 3), frac(4, 3), frac(1, 3), true),
matrix([frac(1, 3), frac(2, 3), frac(1), frac(4, 3)]))
})

it('should allow mixed number and Fraction', function () {
const frac = math.fraction
assert.deepStrictEqual(
range(1, frac(10, 3)),
matrix([frac(1), frac(2), frac(3)]))
assert.deepStrictEqual(
range(frac(1, 3), 3, true),
matrix([frac(1, 3), frac(4, 3), frac(7, 3)]))
assert.deepStrictEqual(
range(frac(1, 3), 2, frac(1, 3)),
matrix([frac(1, 3), frac(2, 3), frac(1), frac(4, 3), frac(5, 3)]))
assert.deepStrictEqual(
range(0, frac(4, 3), frac(1, 3), true),
matrix([frac(0), frac(1, 3), frac(2, 3), frac(1), frac(4, 3)]))
})

it('should throw an error in case of invalid type of include end', function () {
assert.throws(function () { range(0, 10, 2, 0) }, /TypeError: Unexpected type of argument/)
assert.throws(function () { range(0, 10, 2, 1) }, /TypeError: Unexpected type of argument/)
Expand All @@ -147,10 +197,16 @@ describe('range', function () {
assert.throws(function () { range(math.unit('5cm')) }, TypeError)
})

it('should throw an error if called with a single only two units value', function () {
it('should throw an error if called with only two units value', function () {
assert.throws(function () { range(math.unit('0cm'), math.unit('5cm')) }, TypeError)
})

it('should throw an error when called with mismatching units', function () {
assert.throws(function () {
range(math.unit('0cm'), math.unit('2kg'), math.unit('1cm'))
}, Error, 'Cannot compare units with different base')
})

it('should throw an error if called with a complex number', function () {
assert.throws(function () { range(math.complex(2, 3)) }, TypeError)
})
Expand All @@ -169,9 +225,7 @@ describe('range', function () {
assert.throws(function () { range(1, 2, 3, true, 5) }, /TypeError: Too many arguments/)
})

// FIXME: should give the right error
// eslint-disable-next-line mocha/no-skipped-tests
it.skip('should not cast a single number or boolean to string', function () {
it('should not cast a single number or boolean to string', function () {
assert.throws(function () { range(2) }, /TypeError: Too few arguments/)
assert.throws(function () { range(true) }, /TypeError: Unexpected type of argument/)
})
Expand Down
11 changes: 11 additions & 0 deletions test/unit-tests/type/matrix/Index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,17 @@ describe('Index', function () {
assert.deepStrictEqual(new Index(10)._dimensions, [new ImmutableDenseMatrix([10])])
})

it('should create an Index from bigints', function () {
assert.deepStrictEqual(new Index(0n, 2n)._dimensions, [new ImmutableDenseMatrix([0]), new ImmutableDenseMatrix([2])])

assert.deepStrictEqual(new Index(new Range(0n, 10n))._dimensions, [new Range(0, 10, 1)])
assert.deepStrictEqual(new Index(new Range(0n, 10n, 2))._dimensions, [new Range(0, 10, 2)])
assert.deepStrictEqual(new Index(new Range(0n, 10n), new Range(4, 6))._dimensions, [
new Range(0, 10, 1),
new Range(4, 6, 1)
])
})

it('should create an Index from a Range', function () {
assert.deepStrictEqual(new Index(new Range(0, 10))._dimensions, [new Range(0, 10, 1)])
})
Expand Down
6 changes: 6 additions & 0 deletions test/unit-tests/type/matrix/function/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,12 @@ describe('index', function () {
assert.deepStrictEqual(index._dimensions, [new Range(2, 6, 1), new ImmutableDenseMatrix([3])])
})

it('should create an index from bigints (downgrades to numbers)', function () {
const index = math.index(new Range(2n, 6n), 3n)
assert.ok(index instanceof math.Index)
assert.deepStrictEqual(index._dimensions, [new Range(2, 6, 1), new ImmutableDenseMatrix([3])])
})

it('should LaTeX index', function () {
const expr1 = math.parse('index(1)')
const expr2 = math.parse('index(1,2)')
Expand Down

0 comments on commit 1a85b87

Please sign in to comment.