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

fix: allow runModel to proceed without error when inputs array is empty #499

Merged
merged 1 commit into from
Jun 6, 2024
Merged
Show file tree
Hide file tree
Changes from all 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
10 changes: 10 additions & 0 deletions packages/runtime-async/tests/runner.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,16 @@ describe.each([
expect(outOutputs.getSeriesForVar('_output_2')!.points).toEqual([p(2000, 4), p(2001, 5), p(2002, 6)])
})

it('should run the model (with empty inputs array)', async () => {
expect(runner).toBeDefined()
const inOutputs = runner.createOutputs()
const outOutputs = await runner.runModel([], inOutputs)
expect(outOutputs).toBeDefined()
expect(outOutputs.runTimeInMillis).toBeGreaterThan(0)
expect(outOutputs.getSeriesForVar('_output_1')!.points).toEqual([p(2000, 1), p(2001, 2), p(2002, 3)])
expect(outOutputs.getSeriesForVar('_output_2')!.points).toEqual([p(2000, 4), p(2001, 5), p(2002, 6)])
})

it('should run the model (with lookup overrides)', async () => {
const listing = new ModelListing(json)

Expand Down
4 changes: 2 additions & 2 deletions packages/runtime/src/js-model/js-model.ts
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ function runJsModel(
timeStep: number,
saveFreq: number,
numSavePoints: number,
inputs: Float64Array,
inputs: Float64Array | undefined,
outputs: Float64Array,
outputIndices: Int32Array | undefined,
lookups: LookupDef[] | undefined,
Expand Down Expand Up @@ -131,7 +131,7 @@ function runJsModel(
}
}

if (inputs.length > 0) {
if (inputs?.length > 0) {
// Set the user-defined input values. This needs to happen after `initConstants`
// since the input values will override the default constant values.
model.setInputs(index => inputs[index])
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,9 @@ function createMockWasmModule(): MockWasmModule {
outputVarIds: ['_output_1', '_output_2'],
onRunModel: (inputs, outputs, lookups, outputIndices) => {
// Verify inputs
expect(inputs).toEqual(new Float64Array([7, 8, 9]))
if (inputs.length > 0) {
expect(inputs).toEqual(new Float64Array([7, 8, 9]))
}

if (lookups.size > 0) {
// Pretend that outputs are derived from lookup data
Expand Down Expand Up @@ -130,6 +132,16 @@ describe.each([
expect(outOutputs.getSeriesForVar('_output_2').points).toEqual([p(2000, 4), p(2001, 5), p(2002, 6)])
})

it('should run the model (with empty inputs array)', async () => {
expect(runner).toBeDefined()
const inOutputs = runner.createOutputs()
const outOutputs = await runner.runModel([], inOutputs)
expect(outOutputs).toBeDefined()
expect(outOutputs.runTimeInMillis).toBeGreaterThan(0)
expect(outOutputs.getSeriesForVar('_output_1').points).toEqual([p(2000, 1), p(2001, 2), p(2002, 3)])
expect(outOutputs.getSeriesForVar('_output_2').points).toEqual([p(2000, 4), p(2001, 5), p(2002, 6)])
})

it('should run the model (with lookup overrides)', async () => {
const json = `
{
Expand Down
2 changes: 1 addition & 1 deletion packages/runtime/src/runnable-model/base-runnable-model.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import type { RunnableModel } from './runnable-model'
* @hidden This is not part of the public API; for internal use only.
*/
export type OnRunModelFunc = (
inputs: Float64Array,
inputs: Float64Array | undefined,
outputs: Float64Array,
options?: {
outputIndices?: Int32Array
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -142,8 +142,11 @@ describe('BufferedRunModelParams', () => {
const inputs = [1, 2, 3]
const outputs = new Outputs(['_x', '_y'], 2000, 2002, 1)

const params = new BufferedRunModelParams()
params.updateFromParams(inputs, outputs)
const runnerParams = new BufferedRunModelParams()
const workerParams = new BufferedRunModelParams()

runnerParams.updateFromParams(inputs, outputs)
workerParams.updateFromEncodedBuffer(runnerParams.getEncodedBuffer())

let array: Float64Array
const create = (numElements: number) => {
Expand All @@ -152,17 +155,27 @@ describe('BufferedRunModelParams', () => {
}

// Verify case where existing array is undefined
params.copyInputs(undefined, create)
workerParams.copyInputs(undefined, create)
expect(array).toEqual(new Float64Array([1, 2, 3]))

// Verify case where existing array is too small
array = new Float64Array(2)
params.copyInputs(array, create)
workerParams.copyInputs(array, create)
expect(array).toEqual(new Float64Array([1, 2, 3]))

// Verify case where existing array is large enough
array = new Float64Array([6, 6, 6, 6])
params.copyInputs(array, create)
workerParams.copyInputs(array, create)
expect(array).toEqual(new Float64Array([1, 2, 3, 6]))

// Verify case where params are updated with an empty inputs array. Note that
// it is expected that the existing data is retained in the destination array;
// it is up to the calling code to clear or ignore that existing data.
runnerParams.updateFromParams([], outputs)
runnerParams.copyInputs(array, create)
expect(array).toEqual(new Float64Array([1, 2, 3, 6]))
workerParams.updateFromEncodedBuffer(runnerParams.getEncodedBuffer())
workerParams.copyInputs(array, create)
expect(array).toEqual(new Float64Array([1, 2, 3, 6]))
})

Expand All @@ -172,8 +185,11 @@ describe('BufferedRunModelParams', () => {
const normalOutputs = new Outputs(['_x', '_y'], 2000, 2002, 1)
const implOutputs = listing.deriveOutputs(normalOutputs, ['_x', '_a', '_b'])

const params = new BufferedRunModelParams()
params.updateFromParams(inputs, implOutputs)
const runnerParams = new BufferedRunModelParams()
const workerParams = new BufferedRunModelParams()

runnerParams.updateFromParams(inputs, implOutputs)
workerParams.updateFromEncodedBuffer(runnerParams.getEncodedBuffer())

const expectedIndices = new Int32Array([
// _x
Expand All @@ -193,17 +209,17 @@ describe('BufferedRunModelParams', () => {
}

// Verify case where existing array is undefined
params.copyOutputIndices(undefined, create)
workerParams.copyOutputIndices(undefined, create)
expect(array).toEqual(expectedIndices)

// Verify case where existing array is too small
array = new Int32Array(2)
params.copyOutputIndices(array, create)
workerParams.copyOutputIndices(array, create)
expect(array).toEqual(expectedIndices)

// Verify case where existing array is large enough
array = new Int32Array(20).fill(6)
params.copyOutputIndices(array, create)
workerParams.copyOutputIndices(array, create)
expect(array).toEqual(
new Int32Array([
// _x
Expand All @@ -225,9 +241,10 @@ describe('BufferedRunModelParams', () => {
const outputs = new Outputs(['_x', '_y'], 2000, 2002, 1)

const runnerParams = new BufferedRunModelParams()
runnerParams.updateFromParams(inputs, outputs)

const workerParams = new BufferedRunModelParams()

// Run once
runnerParams.updateFromParams(inputs, outputs)
workerParams.updateFromEncodedBuffer(runnerParams.getEncodedBuffer())

// Pretend that the model writes the following values to its outputs buffer then
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,12 @@ export class BufferedRunModelParams implements RunModelParams {

// from RunModelParams interface
copyInputs(array: Float64Array | undefined, create: (numElements: number) => Float64Array): void {
if (this.inputs.lengthInElements === 0) {
// Note that the inputs section will be empty if the inputs parameter is empty,
// so we can skip copying in this case
return
}

// Allocate (or reallocate) an array, if needed
if (array === undefined || array.length < this.inputs.lengthInElements) {
array = create(this.inputs.lengthInElements)
Expand All @@ -129,6 +135,8 @@ export class BufferedRunModelParams implements RunModelParams {
// from RunModelParams interface
copyOutputIndices(array: Int32Array | undefined, create: (numElements: number) => Int32Array): void {
if (this.outputIndices.lengthInElements === 0) {
// Note that the output indices section can be empty if the output indices are
// not provided, so we can skip copying in this case
return
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,13 @@ describe('ReferencedRunModelParams', () => {
array = new Float64Array([6, 6, 6, 6])
params.copyInputs(array, create)
expect(array).toEqual(new Float64Array([1, 2, 3, 6]))

// Verify case where params are updated with an empty inputs array. Note that
// it is expected that the existing data is retained in the destination array;
// it is up to the calling code to clear or ignore that existing data.
params.updateFromParams([], outputs)
params.copyInputs(array, create)
expect(array).toEqual(new Float64Array([1, 2, 3, 6]))
})

it('should copy output indices', () => {
Expand Down
10 changes: 9 additions & 1 deletion packages/runtime/src/wasm-model/_mocks/mock-wasm-module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,15 @@ export class MockWasmModule implements WasmModule {
_malloc(lengthInBytes: number): number {
const currentOffset = this.mallocOffset
this.allocs.set(currentOffset, lengthInBytes)
this.mallocOffset += lengthInBytes
if (lengthInBytes > 0) {
// Update the offset so that the next allocation starts after this one
this.mallocOffset += lengthInBytes
} else {
// In the case where the length is zero, add a little padding so that
// the next allocation is recorded at a different start address than
// this one
this.mallocOffset += 8
}
return currentOffset
}

Expand Down
2 changes: 1 addition & 1 deletion packages/runtime/src/wasm-model/wasm-model.ts
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ class WasmModel implements RunnableModel {
// Run the model
const t0 = perfNow()
this.wasmRunModel(
this.inputsBuffer.getAddress(),
this.inputsBuffer?.getAddress() || 0,
this.outputsBuffer.getAddress(),
outputIndicesBuffer?.getAddress() || 0
)
Expand Down