From 661deac67260db208787db71480e3f0563928c39 Mon Sep 17 00:00:00 2001 From: Chris Campbell Date: Wed, 5 Jun 2024 18:01:03 -0700 Subject: [PATCH] fix: allow runModel to proceed without error when inputs array is empty --- packages/runtime-async/tests/runner.spec.ts | 10 +++++ packages/runtime/src/js-model/js-model.ts | 4 +- .../synchronous-model-runner.spec.ts | 14 ++++++- .../src/runnable-model/base-runnable-model.ts | 2 +- .../buffered-run-model-params.spec.ts | 41 +++++++++++++------ .../buffered-run-model-params.ts | 8 ++++ .../referenced-run-model-params.spec.ts | 7 ++++ .../src/wasm-model/_mocks/mock-wasm-module.ts | 10 ++++- packages/runtime/src/wasm-model/wasm-model.ts | 2 +- 9 files changed, 80 insertions(+), 18 deletions(-) diff --git a/packages/runtime-async/tests/runner.spec.ts b/packages/runtime-async/tests/runner.spec.ts index 40cb8a07..6b469bdc 100644 --- a/packages/runtime-async/tests/runner.spec.ts +++ b/packages/runtime-async/tests/runner.spec.ts @@ -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) diff --git a/packages/runtime/src/js-model/js-model.ts b/packages/runtime/src/js-model/js-model.ts index 8f57b7ef..5ebefb5e 100644 --- a/packages/runtime/src/js-model/js-model.ts +++ b/packages/runtime/src/js-model/js-model.ts @@ -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, @@ -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]) diff --git a/packages/runtime/src/model-runner/synchronous-model-runner.spec.ts b/packages/runtime/src/model-runner/synchronous-model-runner.spec.ts index 56128efe..9a7ca636 100644 --- a/packages/runtime/src/model-runner/synchronous-model-runner.spec.ts +++ b/packages/runtime/src/model-runner/synchronous-model-runner.spec.ts @@ -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 @@ -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 = ` { diff --git a/packages/runtime/src/runnable-model/base-runnable-model.ts b/packages/runtime/src/runnable-model/base-runnable-model.ts index b40baa32..05158e1f 100644 --- a/packages/runtime/src/runnable-model/base-runnable-model.ts +++ b/packages/runtime/src/runnable-model/base-runnable-model.ts @@ -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 diff --git a/packages/runtime/src/runnable-model/buffered-run-model-params.spec.ts b/packages/runtime/src/runnable-model/buffered-run-model-params.spec.ts index 19322f2d..9748e467 100644 --- a/packages/runtime/src/runnable-model/buffered-run-model-params.spec.ts +++ b/packages/runtime/src/runnable-model/buffered-run-model-params.spec.ts @@ -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) => { @@ -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])) }) @@ -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 @@ -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 @@ -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 diff --git a/packages/runtime/src/runnable-model/buffered-run-model-params.ts b/packages/runtime/src/runnable-model/buffered-run-model-params.ts index bfb266ea..e5340889 100644 --- a/packages/runtime/src/runnable-model/buffered-run-model-params.ts +++ b/packages/runtime/src/runnable-model/buffered-run-model-params.ts @@ -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) @@ -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 } diff --git a/packages/runtime/src/runnable-model/referenced-run-model-params.spec.ts b/packages/runtime/src/runnable-model/referenced-run-model-params.spec.ts index 87cf4a93..09d5c9b3 100644 --- a/packages/runtime/src/runnable-model/referenced-run-model-params.spec.ts +++ b/packages/runtime/src/runnable-model/referenced-run-model-params.spec.ts @@ -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', () => { diff --git a/packages/runtime/src/wasm-model/_mocks/mock-wasm-module.ts b/packages/runtime/src/wasm-model/_mocks/mock-wasm-module.ts index d5028a1d..fb6e9319 100644 --- a/packages/runtime/src/wasm-model/_mocks/mock-wasm-module.ts +++ b/packages/runtime/src/wasm-model/_mocks/mock-wasm-module.ts @@ -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 } diff --git a/packages/runtime/src/wasm-model/wasm-model.ts b/packages/runtime/src/wasm-model/wasm-model.ts index 891022a4..4a59e2f0 100644 --- a/packages/runtime/src/wasm-model/wasm-model.ts +++ b/packages/runtime/src/wasm-model/wasm-model.ts @@ -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 )