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: make new implementation of read and code gen phases the default #446

Merged
merged 2 commits into from
Mar 12, 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
36 changes: 25 additions & 11 deletions models/prune/prune_check.sh
Original file line number Diff line number Diff line change
Expand Up @@ -44,17 +44,31 @@ expect_present "_constant_partial_2"
expect_present "_initial_partial"
expect_present "_partial"
expect_present "_test_1_result = _IF_THEN_ELSE(_input_1 == 10.0, _test_1_t, _test_1_f);"
expect_present "_test_2_result = (_test_2_f);"
expect_present "_test_3_result = (_test_3_t);"
expect_present "_test_4_result = (_test_4_f);"
expect_present "_test_5_result = (_test_5_t);"
expect_present "_test_6_result = (_test_6_f);"
expect_present "_test_7_result = (_test_7_t);"
expect_present "_test_8_result = (_test_8_f);"
expect_present "_test_9_result = (_test_9_t);"
expect_present "_test_10_result = _IF_THEN_ELSE(_ABS(_test_10_cond), _test_10_t, _test_10_f);"
expect_present "_test_11_result = (_test_11_f);"
expect_present "_test_12_result = (_test_12_t);"
if [[ $SDE_NONPUBLIC_USE_NEW_PARSE == "0" ]]; then
expect_present "_test_2_result = (_test_2_f);"
expect_present "_test_3_result = (_test_3_t);"
expect_present "_test_4_result = (_test_4_f);"
expect_present "_test_5_result = (_test_5_t);"
expect_present "_test_6_result = (_test_6_f);"
expect_present "_test_7_result = (_test_7_t);"
expect_present "_test_8_result = (_test_8_f);"
expect_present "_test_9_result = (_test_9_t);"
expect_present "_test_10_result = _IF_THEN_ELSE(_ABS(_test_10_cond), _test_10_t, _test_10_f);"
expect_present "_test_11_result = (_test_11_f);"
expect_present "_test_12_result = (_test_12_t);"
else
expect_present "_test_2_result = _test_2_f;"
expect_present "_test_3_result = _test_3_t;"
expect_present "_test_4_result = _test_4_f;"
expect_present "_test_5_result = _test_5_t;"
expect_present "_test_6_result = _test_6_f;"
expect_present "_test_7_result = _test_7_t;"
expect_present "_test_8_result = _test_8_f;"
expect_present "_test_9_result = _test_9_t;"
expect_present "_test_10_result = _test_10_t;"
expect_present "_test_11_result = _test_11_f;"
expect_present "_test_12_result = _test_12_t;"
fi
expect_present "_test_13_result = (_test_13_t1 + _test_13_t2) \* 10.0;"

# Verify that unreferenced variables do not appear in the generated C file
Expand Down
2 changes: 1 addition & 1 deletion packages/compile/src/_tests/test-support.ts
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ export function parseVensimModel(modelName: string): ParsedModel {
const modelDir = sampleModelDir(modelName)
const modelFile = resolve(modelDir, `${modelName}.mdl`)
let mdlContent: string
if (process.env.SDE_NONPUBLIC_USE_NEW_PARSE === '1') {
if (process.env.SDE_NONPUBLIC_USE_NEW_PARSE !== '0') {
// Note that the new parser implicitly runs the preprocessor on the input model text,
// so we don't need to do that here. (We should make it configurable so that we can
// skip the preprocess step in `parse-and-generate.js` when the input model text has
Expand Down
2 changes: 1 addition & 1 deletion packages/compile/src/generate/expand-var-names.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ export function expandVarNames(canonical) {
* @returns {string[]} An array of expanded names for the given variable.
*/
function namesForVar(v) {
if (process.env.SDE_NONPUBLIC_USE_NEW_PARSE !== '1') {
if (process.env.SDE_NONPUBLIC_USE_NEW_PARSE === '0') {
// TODO: When the old parsing code is active, use the old ModelLHSReader. This code path
// will be removed when the old parsing code is removed.
let modelLHSReader = new ModelLHSReader()
Expand Down
2 changes: 1 addition & 1 deletion packages/compile/src/generate/expand-var-names.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ describe('expandVarNames', () => {
// in the EXCEPT clause), it will pick up the `A1` and include it in the set of expanded names
// even though this doesn't seem appropriate. We will treat this as a quirk of the legacy
// reader and therefore skip this test when the legacy code is in use.
it.skipIf(process.env.SDE_NONPUBLIC_USE_NEW_PARSE !== '1')(
it.skipIf(process.env.SDE_NONPUBLIC_USE_NEW_PARSE === '0')(
'should return names for a subscripted 1D variable that uses a disjoint EXCEPT clause',
() => {
readInlineModel(`
Expand Down
2 changes: 1 addition & 1 deletion packages/compile/src/generate/gen-equation-c.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ function genC(
}

let lines: string[]
if (process.env.SDE_NONPUBLIC_USE_NEW_PARSE === '1') {
if (process.env.SDE_NONPUBLIC_USE_NEW_PARSE !== '0') {
lines = generateEquation(variable, mode, opts?.extData, directData, opts?.modelDir)
} else {
// TODO: The `flat` call is only needed because the legacy EquationGen adds a nested array unnecessarily
Expand Down
2 changes: 1 addition & 1 deletion packages/compile/src/model/model.js
Original file line number Diff line number Diff line change
Expand Up @@ -800,7 +800,7 @@ function vensimName(cVarName) {
function cName(vensimVarName) {
// Convert a Vensim variable name to a C name.
// This function requires model analysis to be completed first when the variable has subscripts.
if (process.env.SDE_NONPUBLIC_USE_NEW_PARSE !== '1') {
if (process.env.SDE_NONPUBLIC_USE_NEW_PARSE === '0') {
// TODO: For now we use the legacy VarNameReader when the old parser is active; this
// code will be removed once the old parser is removed
return new VarNameReader().read(vensimVarName)
Expand Down
6 changes: 3 additions & 3 deletions packages/compile/src/model/read-equations.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2899,7 +2899,7 @@ describe('readEquations', () => {
// where the new reader differs from the old (in `IF THEN ELSE(integer supply, ...)`
// where the condition resolves to a constant). We should add an option to disable
// the pruning code so that we can test this more deterministically.
it.skipIf(process.env.SDE_NONPUBLIC_USE_NEW_PARSE === '1')('should work for Vensim "allocate" model', () => {
it.skipIf(process.env.SDE_NONPUBLIC_USE_NEW_PARSE !== '0')('should work for Vensim "allocate" model', () => {
const vars = readSubscriptsAndEquations('allocate')
expect(vars).toEqual([
v('demand[region]', '3,2,4', {
Expand Down Expand Up @@ -6279,7 +6279,7 @@ describe('readEquations', () => {

// TODO: This test depends on the dependency trimming code that isn't yet implemented
// in the new reader, so skip it when `NEW_PARSE` flag is enabled
it.skipIf(process.env.SDE_NONPUBLIC_USE_NEW_PARSE === '1')('should work for Vensim "prune" model', () => {
it.skipIf(process.env.SDE_NONPUBLIC_USE_NEW_PARSE !== '0')('should work for Vensim "prune" model', () => {
const vars = readSubscriptsAndEquations('prune')
expect(vars).toEqual([
v('A Totals', 'SUM(A Values[DimA!])', {
Expand Down Expand Up @@ -6851,7 +6851,7 @@ describe('readEquations', () => {
// where the new reader differs from the old (in `IF THEN ELSE(switch=1,1,0)`
// where the condition resolves to a constant). We should add an option to disable
// the pruning code so that we can test this more deterministically.
it.skipIf(process.env.SDE_NONPUBLIC_USE_NEW_PARSE === '1')('should work for Vensim "sample" model', () => {
it.skipIf(process.env.SDE_NONPUBLIC_USE_NEW_PARSE !== '0')('should work for Vensim "sample" model', () => {
const vars = readSubscriptsAndEquations('sample')
expect(vars).toEqual([
v('a', 'SAMPLE IF TRUE(MODULO(Time,5)=0,Time,0)', {
Expand Down
4 changes: 2 additions & 2 deletions packages/compile/src/model/reduce-variables.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ function v(lhs: string, formula: string, overrides?: Partial<Variable>): Variabl
return variable as Variable
}

describe.skipIf(process.env.SDE_NONPUBLIC_USE_NEW_PARSE !== '1')(
describe.skipIf(process.env.SDE_NONPUBLIC_USE_NEW_PARSE === '0')(
'reduceVariables (default mode: reduce conditionals only)',
() => {
it('should reduce a simple equation when the condition resolves to a constant', () => {
Expand Down Expand Up @@ -156,7 +156,7 @@ describe.skipIf(process.env.SDE_NONPUBLIC_USE_NEW_PARSE !== '1')(
}
)

describe.skipIf(process.env.SDE_NONPUBLIC_USE_NEW_PARSE !== '1')(
describe.skipIf(process.env.SDE_NONPUBLIC_USE_NEW_PARSE === '0')(
'reduceVariables (aggressive mode: reduce everything)',
() => {
it('should reduce a simple equation to a constant', () => {
Expand Down
2 changes: 1 addition & 1 deletion packages/compile/src/parse-and-generate.js
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ export function printNames(namesPathname, operation) {
* @return {*} A parsed tree representation of the model.
*/
export function parseModel(input, modelDir, sort = false) {
if (process.env.SDE_NONPUBLIC_USE_NEW_PARSE !== '1') {
if (process.env.SDE_NONPUBLIC_USE_NEW_PARSE === '0') {
// Use the legacy parser
return {
kind: 'vensim-legacy',
Expand Down