Skip to content

Commit

Permalink
fix: cast index values to double when used in expression position in …
Browse files Browse the repository at this point in the history
…C code generation (#591)

Fixes #568
  • Loading branch information
chrispcampbell authored Dec 13, 2024
1 parent 0f741a5 commit 6e14176
Show file tree
Hide file tree
Showing 7 changed files with 95 additions and 10 deletions.
27 changes: 27 additions & 0 deletions models/subscript/subscript.dat
Original file line number Diff line number Diff line change
Expand Up @@ -185,3 +185,30 @@ v[A2]
v[A3]
0 0
1 0
w[X1,Y1]
0 0
1 0
w[X1,Y2]
0 -1
1 -1
w[X1,Y3]
0 -2
1 -2
w[X2,Y1]
0 1
1 1
w[X2,Y2]
0 0
1 0
w[X2,Y3]
0 -1
1 -1
w[X3,Y1]
0 2
1 2
w[X3,Y2]
0 1
1 1
w[X3,Y3]
0 0
1 0
8 changes: 8 additions & 0 deletions models/subscript/subscript.mdl
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,14 @@ v[DimA] = IF THEN ELSE (DimA = A2, 1, 0)
~:SUPPLEMENTARY
|

DimX: (X1-X3) ~~|
DimY: (Y1-Y3) ~~|
w[DimX,DimY] = DimX - DimY
~
~ dimensions used in expression position (where expression result can go negative)
~:SUPPLEMENTARY
|

********************************************************
.Control
********************************************************~
Expand Down
18 changes: 11 additions & 7 deletions packages/compile/src/generate/gen-equation-c.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -291,7 +291,7 @@ describe('generateEquation (Vensim -> C)', () => {
expect(genC(vars.get('_x'))).toEqual(['_x = 1.0;'])
expect(genC(vars.get('_y'))).toEqual([
'for (size_t i = 0; i < 2; i++) {',
'_y[i] = _IF_THEN_ELSE((i + 1) == _x, 1.0, 0.0);',
'_y[i] = _IF_THEN_ELSE((((double)i) + 1) == _x, 1.0, 0.0);',
'}'
])
})
Expand All @@ -304,7 +304,7 @@ describe('generateEquation (Vensim -> C)', () => {
expect(vars.size).toBe(1)
expect(genC(vars.get('_y'))).toEqual([
'for (size_t i = 0; i < 2; i++) {',
'_y[i] = _IF_THEN_ELSE((i + 1) == 2, 1.0, 0.0);',
'_y[i] = _IF_THEN_ELSE((((double)i) + 1) == 2, 1.0, 0.0);',
'}'
])
})
Expand Down Expand Up @@ -650,7 +650,11 @@ describe('generateEquation (Vensim -> C)', () => {
x[DimA] = DimB ~~|
`)
expect(vars.size).toBe(1)
expect(genC(vars.get('_x'))).toEqual(['for (size_t i = 0; i < 2; i++) {', '_x[i] = (__map_dimb_dima[i] + 1);', '}'])
expect(genC(vars.get('_x'))).toEqual([
'for (size_t i = 0; i < 2; i++) {',
'_x[i] = (((double)__map_dimb_dima[i]) + 1);',
'}'
])
})

it('should work for 1D equation with one mapped dimension name used in subscript position (separated/non-apply-to-all)', () => {
Expand Down Expand Up @@ -680,7 +684,7 @@ describe('generateEquation (Vensim -> C)', () => {
expect(genC(vars.get('_selected_a_index'), 'init-constants')).toEqual(['_selected_a_index = 1.0;'])
expect(genC(vars.get('_x'))).toEqual([
'for (size_t i = 0; i < 2; i++) {',
'_x[i] = _IF_THEN_ELSE((i + 1) == _selected_a_index, 1.0, 0.0);',
'_x[i] = _IF_THEN_ELSE((((double)i) + 1) == _selected_a_index, 1.0, 0.0);',
'}'
])
})
Expand Down Expand Up @@ -708,7 +712,7 @@ describe('generateEquation (Vensim -> C)', () => {
expect(genC(vars.get('_x'))).toEqual([
'for (size_t i = 0; i < 2; i++) {',
'for (size_t j = 0; j < 2; j++) {',
'_x[i][j] = ((i + 1) * 10.0) + (j + 1);',
'_x[i][j] = ((((double)i) + 1) * 10.0) + (((double)j) + 1);',
'}',
'}'
])
Expand Down Expand Up @@ -738,7 +742,7 @@ describe('generateEquation (Vensim -> C)', () => {
expect(genC(vars.get('_x'))).toEqual([
'for (size_t i = 0; i < 2; i++) {',
'for (size_t j = 0; j < 2; j++) {',
'_x[i][j] = ((i + 1) * 10.0) + (j + 1);',
'_x[i][j] = ((((double)i) + 1) * 10.0) + (((double)j) + 1);',
'}',
'}'
])
Expand Down Expand Up @@ -3224,7 +3228,7 @@ describe('generateEquation (Vensim -> C)', () => {
expect(genC(vars.get('_x[_five]'), 'init-constants')).toEqual(['_x[4] = 5.0;'])
expect(genC(vars.get('_y'))).toEqual([
'for (size_t i = 0; i < 3; i++) {',
'_y[i] = _x[_dimx[(size_t)(2 + ((i + 1) - 1.0))]];',
'_y[i] = _x[_dimx[(size_t)(2 + ((((double)i) + 1) - 1.0))]];',
'}'
])
})
Expand Down
31 changes: 30 additions & 1 deletion packages/compile/src/generate/gen-expr.js
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ export function generateExpr(expr, ctx) {
// plus one (since Vensim indices are one-based).
const dimId = expr.varId
const indexCode = ctx.cVarIndex(dimId)
return `(${indexCode} + 1)`
return `(${indexExpr(indexCode, ctx)} + 1)`
} else if (isIndex(expr.varId)) {
// This is a reference to a subscript/index that is being used in expression position.
// In place of the subscript, emit the numeric index value of the subscript plus one
Expand Down Expand Up @@ -1020,3 +1020,32 @@ function minFunc(ctx) {
throw new Error(`Unhandled output format '${ctx.outFormat}'`)
}
}

/**
* Return the C or JS code for a subscript or dimension (loop index variable) used in
* expression position.
*
* @param {string} indexValue The index number or code.
* @param {GenExprContext} ctx The context used when generating code for the expression.
* @return {string} The generated C/JS code.
*/
function indexExpr(indexValue, ctx) {
switch (ctx.outFormat) {
case 'c':
// In the C case, we need to cast to double since the index variable will be
// of type `size_t`, which is an unsigned type, but we want a signed type for
// the rare cases where math is involved that makes it go negative
if (isNaN(indexValue)) {
// This is a (non-numeric) loop index variable reference, so cast to double
return `((double)${indexValue})`
} else {
// This is a numeric index, no cast is necessary
return indexValue
}
case 'js':
// In the JS case, no cast is necessary
return indexValue
default:
throw new Error(`Unhandled output format '${ctx.outFormat}'`)
}
}
4 changes: 4 additions & 0 deletions packages/compile/src/model/read-equations.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10174,6 +10174,10 @@ describe('readEquations', () => {
refId: '_v',
subscripts: ['_dima']
}),
v('w[DimX,DimY]', 'DimX-DimY', {
refId: '_w',
subscripts: ['_dimx', '_dimy']
}),
v('FINAL TIME', '1', {
refId: '_final_time',
varType: 'const'
Expand Down
14 changes: 12 additions & 2 deletions packages/compile/src/model/read-subscripts.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -607,7 +607,9 @@ describe('readSubscriptRanges + resolveSubscriptRanges', () => {
dim('DimB', ['B1', 'B2', 'B3'], undefined, undefined, [dimMapping('DimA')], {
_dima: []
}),
dim('DimC', ['C1', 'C2', 'C3', 'C4', 'C5'])
dim('DimC', ['C1', 'C2', 'C3', 'C4', 'C5']),
dim('DimX', ['X1', 'X2', 'X3']),
dim('DimY', ['Y1', 'Y2', 'Y3'])
])

const resolvedSubs = readAndResolveSubscripts('subscript')
Expand Down Expand Up @@ -641,6 +643,8 @@ describe('readSubscriptRanges + resolveSubscriptRanges', () => {
_dima: ['_b1', '_b2', '_b3']
}),
dim('DimC', ['C1', 'C2', 'C3', 'C4', 'C5']),
dim('DimX', ['X1', 'X2', 'X3']),
dim('DimY', ['Y1', 'Y2', 'Y3']),
// { name: '_a1', value: 0, size: 1, family: '_dima', mappings: {} },
sub('A1', 'DimA', 0),
// { name: '_a2', value: 1, size: 1, family: '_dima', mappings: {} },
Expand All @@ -657,7 +661,13 @@ describe('readSubscriptRanges + resolveSubscriptRanges', () => {
sub('C2', 'DimC', 1),
sub('C3', 'DimC', 2),
sub('C4', 'DimC', 3),
sub('C5', 'DimC', 4)
sub('C5', 'DimC', 4),
sub('X1', 'DimX', 0),
sub('X2', 'DimX', 1),
sub('X3', 'DimX', 2),
sub('Y1', 'DimY', 0),
sub('Y2', 'DimY', 1),
sub('Y3', 'DimY', 2)
])
})
})
3 changes: 3 additions & 0 deletions packages/compile/src/model/read-variables.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1933,6 +1933,9 @@ describe('readVariables', () => {
v('v[DimA]', 'IF THEN ELSE(DimA=A2,1,0)', {
subscripts: ['_dima']
}),
v('w[DimX,DimY]', 'DimX-DimY', {
subscripts: ['_dimx', '_dimy']
}),
v('FINAL TIME', '1'),
v('INITIAL TIME', '0'),
v('SAVEPER', 'TIME STEP'),
Expand Down

0 comments on commit 6e14176

Please sign in to comment.