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: cast index values to double when used in expression position in C code generation #591

Merged
merged 6 commits into from
Dec 13, 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
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