refactor: add generated variables in bulk before calling readEquations (

#567)

Fixes #566

packages/compile/src/model/read-equation-fn-delay.js

@@ -81,20 +81,22 @@ export function generateDelayVariables(v, callExpr, context) {
 }
 
 /**
- * Generate a single level variable that is used to implement a `DELAY` function call.
+ * Generate equation text for a single level variable that is used to implement a `DELAY`
+ * function call, and add a reference to `levelRefId`.
  */
 function generateDelayLevel(context, levelLHS, levelRefId, input, aux, init) {
   const levelEqn = `${levelLHS} = INTEG(${input} - ${aux}, ${init}) ~~|`
-  context.defineVariable(levelEqn)
   context.addVarReference(levelRefId)
+  return levelEqn
 }
 
 //
 // DELAY1[I]
 //
 
 /**
- * TODO: Docs
+ * Generate and define variables that are needed to implement a `DELAY1[I]` function call
+ * for a non-subscripted or apply-to-all variable.
  */
 function generateDelay1VarsForNormalVar(v, context, subs, argInput, argDelay, argInit) {
   // Generate a single level variable instance
@@ -103,17 +105,23 @@ function generateDelay1VarsForNormalVar(v, context, subs, argInput, argDelay, ar
   const levelVarRefId = canonicalName(levelVarBaseName)
   const levelLHS = `${levelVarBaseName}${subs}`
 
+  // Generate the equation text for the delay level variable
+  const eqns = []
   // TODO: There should be parens around the factors in this init expression
   const argInitTimesDelay = `${argInit} * ${argDelay}`
-  generateDelayLevel(context, levelLHS, levelVarRefId, argInput, varLHS, argInitTimesDelay)
+  eqns.push(generateDelayLevel(context, levelLHS, levelVarRefId, argInput, varLHS, argInitTimesDelay))
   v.delayVarRefId = levelVarRefId
 
   // Generate an aux variable to hold the delay time expression
-  generateDelay1Aux(v, context, subs, argDelay)
+  eqns.push(generateDelay1Aux(v, context, subs, argDelay))
+
+  // Add the generated variables to the model
+  context.defineVariables(eqns)
 }
 
 /**
- * TODO: Docs
+ * Generate and define variables that are needed to implement a `DELAY1[I]` function call
+ * for a non-apply-to-all variable.
  */
 function generateDelay1VarsForSeparatedVar(v, context, subs, argInput, argDelay, argInit) {
   // XXX: This code that deals with separated variables is largely copied from the legacy
@@ -151,17 +159,23 @@ function generateDelay1VarsForSeparatedVar(v, context, subs, argInput, argDelay,
   const levelVarRefId = canonicalVensimName(levelLHS)
   Model.addNonAtoAVar(canonicalName(levelVarBaseName), [true])
 
+  // Generate the equation text for the delay level variable
+  const eqns = []
   // TODO: There should be parens around the factors in this init expression
   const argInitTimesDelay = `${argInit} * ${argDelay}`
-  generateDelayLevel(context, levelLHS, levelVarRefId, argInput, varLHS, argInitTimesDelay)
+  eqns.push(generateDelayLevel(context, levelLHS, levelVarRefId, argInput, varLHS, argInitTimesDelay))
   v.delayVarRefId = levelVarRefId
 
   // Generate an aux variable to hold the delay time expression
-  generateDelay1Aux(v, context, subs, argDelay)
+  eqns.push(generateDelay1Aux(v, context, subs, argDelay))
+
+  // Add the generated variables to the model
+  context.defineVariables(eqns)
 }
 
 /**
- * TODO: Docs
+ * Generate equation text for a single "delay time" aux variable that is used to implement
+ * a `DELAY1[I]` function call, and add a reference to the delay time variable.
  */
 function generateDelay1Aux(v, context, subs, argDelay) {
   const delayTimeVarName = newAuxVarName()
@@ -176,16 +190,17 @@ function generateDelay1Aux(v, context, subs, argDelay) {
   }
 
   const delayTimeEqn = `${delayTimeLHS} = ${argDelay} ~~|`
-  context.defineVariable(delayTimeEqn)
   context.addVarReference(delayTimeVarRefId)
+  return delayTimeEqn
 }
 
 //
 // DELAY3[I]
 //
 
 /**
- * TODO: Docs
+ * Generate and define variables that are needed to implement a `DELAY3[I]` function call
+ * for a non-subscripted or apply-to-all variable.
  */
 function generateDelay3VarsForNormalVar(v, context, subs, argInput, argDelay, argInit) {
   // Generate names for the 3 level variables and 4 aux variables
@@ -208,28 +223,33 @@ function generateDelay3VarsForNormalVar(v, context, subs, argInput, argDelay, ar
   const level3RefId = canonicalName(level3)
 
   // Generate level variables
+  const eqns = []
   const delay3Val = `((${argDelay}) / 3)`
   // TODO: There should be parens around these factors
   const initArg = `${argInit} * ${delay3Val}`
-  generateDelayLevel(context, level3LHS, level3RefId, aux2LHS, aux3LHS, initArg)
-  generateDelayLevel(context, level2LHS, level2RefId, aux1LHS, aux2LHS, initArg)
-  generateDelayLevel(context, level1LHS, level1RefId, argInput, aux1LHS, initArg)
+  eqns.push(generateDelayLevel(context, level3LHS, level3RefId, aux2LHS, aux3LHS, initArg))
+  eqns.push(generateDelayLevel(context, level2LHS, level2RefId, aux1LHS, aux2LHS, initArg))
+  eqns.push(generateDelayLevel(context, level1LHS, level1RefId, argInput, aux1LHS, initArg))
   v.delayVarRefId = canonicalName(level3)
 
   // Generate aux variable equations using the subs in the generated level vars
-  context.defineVariable(`${aux1LHS} = ${level1LHS} / ${delay3Val} ~~|`)
-  context.defineVariable(`${aux2LHS} = ${level2LHS} / ${delay3Val} ~~|`)
-  context.defineVariable(`${aux3LHS} = ${level3LHS} / ${delay3Val} ~~|`)
+  eqns.push(`${aux1LHS} = ${level1LHS} / ${delay3Val} ~~|`)
+  eqns.push(`${aux2LHS} = ${level2LHS} / ${delay3Val} ~~|`)
+  eqns.push(`${aux3LHS} = ${level3LHS} / ${delay3Val} ~~|`)
 
   // Generate an aux variable to hold the delay time expression
   v.delayTimeVarName = canonicalName(aux4)
   const delayTimeVarRefId = canonicalVensimName(aux4LHS)
-  context.defineVariable(`${aux4LHS} = ${delay3Val} ~~|`)
+  eqns.push(`${aux4LHS} = ${delay3Val} ~~|`)
   context.addVarReference(delayTimeVarRefId)
+
+  // Add the generated variables to the model
+  context.defineVariables(eqns)
 }
 
 /**
- * TODO: Docs
+ * Generate and define variables that are needed to implement a `DELAY3[I]` function call
+ * for a non-apply-to-all variable.
  */
 function generateDelay3VarsForSeparatedVar(v, context, subs, argInput, argDelay, argInit) {
   // XXX: This code that deals with separated variables is largely copied from the legacy
@@ -291,22 +311,26 @@ function generateDelay3VarsForSeparatedVar(v, context, subs, argInput, argDelay,
   Model.addNonAtoAVar(canonicalName(level3), [true])
 
   // Generate level variables
-  generateDelayLevel(context, level3LHS, level3RefId, aux2LHS, aux3LHS, initArg)
-  generateDelayLevel(context, level2LHS, level2RefId, aux1LHS, aux2LHS, initArg)
-  generateDelayLevel(context, level1LHS, level1RefId, argInput, aux1LHS, initArg)
+  const eqns = []
+  eqns.push(generateDelayLevel(context, level3LHS, level3RefId, aux2LHS, aux3LHS, initArg))
+  eqns.push(generateDelayLevel(context, level2LHS, level2RefId, aux1LHS, aux2LHS, initArg))
+  eqns.push(generateDelayLevel(context, level1LHS, level1RefId, argInput, aux1LHS, initArg))
   v.delayVarRefId = level3RefId
 
   // Generate aux variable equations using the subs in the generated level vars
-  context.defineVariable(`${aux1LHS} = ${level1LHS} / ${delay3Val} ~~|`)
-  context.defineVariable(`${aux2LHS} = ${level2LHS} / ${delay3Val} ~~|`)
-  context.defineVariable(`${aux3LHS} = ${level3LHS} / ${delay3Val} ~~|`)
+  eqns.push(`${aux1LHS} = ${level1LHS} / ${delay3Val} ~~|`)
+  eqns.push(`${aux2LHS} = ${level2LHS} / ${delay3Val} ~~|`)
+  eqns.push(`${aux3LHS} = ${level3LHS} / ${delay3Val} ~~|`)
 
   // Generate an aux variable to hold the delay time expression
   v.delayTimeVarName = canonicalName(aux4)
   if (isSeparatedVar(v)) {
     Model.addNonAtoAVar(v.delayTimeVarName, [true])
   }
   const delayTimeVarRefId = canonicalVensimName(aux4LHS)
-  context.defineVariable(`${aux4LHS} = ${delay3Val} ~~|`)
+  eqns.push(`${aux4LHS} = ${delay3Val} ~~|`)
   context.addVarReference(delayTimeVarRefId)
+
+  // Add the generated variables to the model
+  context.defineVariables(eqns)
 }

packages/compile/src/model/read-equation-fn-game.js

@@ -33,9 +33,9 @@ export function generateGameVariables(v, callExpr, context) {
   }
 
   // Define a variable for the synthesized game inputs lookup
-  const gameLookupVars = context.defineVariable(`${gameLookupVarName}${subs} ~~|`)
+  const gameLookupVars = context.defineVariables([`${gameLookupVarName}${subs} ~~|`])
 
-  // Normally `defineVariable` sets `includeInOutput` to false for generated
+  // Normally `defineVariables` sets `includeInOutput` to false for generated
   // variables, but we want the generated lookup variable to appear in the
   // model listing so that the user can reference it, so set `includeInOutput`
   // to true.  Also change the `varType` to 'lookup' instead of 'data'.  We

packages/compile/src/model/read-equation-fn-npv.js

@@ -26,6 +26,10 @@ export function generateNpvVariables(v, callExpr, context) {
   // const subs = this.genSubs(stream, discountRate, initVal, factor)
   const subs = ''
 
+  // Build an array of equation strings; variables will be defined for these at the
+  // end of this step once all equations are known
+  const eqns = []
+
   // Level 1:
   //   df = INTEG((-df * discount rate) / (1 + discount rate * TIME STEP), 1)
   const dfVarName = newLevelVarName()
@@ -34,15 +38,15 @@ export function generateNpvVariables(v, callExpr, context) {
   // TODO: There should be parens around the `discount rate` argument in case it is an expression
   // and not a simple constant
   const dfEqn = `${dfLHS} = INTEG((-${dfLHS} * ${argDiscountRate}) / (1 + ${argDiscountRate} * TIME STEP), 1) ~~|`
-  context.defineVariable(dfEqn)
+  eqns.push(dfEqn)
 
   // Level 2:
   //   ncum = INTEG(stream * df, init val)
   const ncumVarName = newLevelVarName()
   const ncumVarId = canonicalName(ncumVarName)
   const ncumLHS = `${ncumVarName}${subs}`
   const ncumEqn = `${ncumLHS} = INTEG(${argStream} * ${dfLHS}, ${argInitVal}) ~~|`
-  context.defineVariable(ncumEqn)
+  eqns.push(ncumEqn)
 
   // Aux:
   //   npv = (ncum + stream * TIME STEP * df) * factor
@@ -52,7 +56,7 @@ export function generateNpvVariables(v, callExpr, context) {
   // TODO: There should be parens around the `stream` argument in case it is an expression
   // and not a simple constant
   const auxEqn = `${auxLHS} = (${ncumVarName} + ${argStream} * TIME STEP * ${dfVarName}) * ${argFactor} ~~|`
-  context.defineVariable(auxEqn)
+  eqns.push(auxEqn)
   v.npvVarName = auxVarId
 
   // Add references to the generated variables
@@ -62,6 +66,9 @@ export function generateNpvVariables(v, callExpr, context) {
   context.addVarReference(dfVarId)
   context.addVarReference(auxVarId)
 
+  // Add the generated variables to the model
+  context.defineVariables(eqns)
+
   // TODO: Check on this comment from the legacy reader to see if it's still applicable:
   // If they have subscripts, the refIds are still just the var name, because they are apply-to-all arrays.
 }

packages/compile/src/model/read-equation-fn-smooth.js

@@ -45,17 +45,21 @@ export function generateSmoothVariables(v, callExpr, context) {
   const fnId = callExpr.fnId
   if (fnId === '_SMOOTH' || fnId === '_SMOOTHI') {
     // Generate 1 level variable that will replace the `SMOOTH[I]` function call
-    const levelVarName = generateSmoothLevel(v, context, argInput, argDelay, argInit, 1)
+    const level = generateSmoothLevel(v, context, argInput, argDelay, argInit, 1)
     // For `SMOOTH[I]`, the smoothVarRefId is the level var's refId
-    v.smoothVarRefId = levelVarName[0]
+    v.smoothVarRefId = level.varRefId
+    // Add the generated variable to the model
+    context.defineVariables([level.eqn])
   } else {
     // Generate 3 level variables that will replace the `SMOOTH3[I]` function call
     const delay3Val = `(${argDelay} / 3)`
-    const level1VarName = generateSmoothLevel(v, context, argInput, delay3Val, argInit, 1)
-    const level2VarName = generateSmoothLevel(v, context, level1VarName[1], delay3Val, argInit, 2)
-    const level3VarName = generateSmoothLevel(v, context, level2VarName[1], delay3Val, argInit, 3)
+    const level1 = generateSmoothLevel(v, context, argInput, delay3Val, argInit, 1)
+    const level2 = generateSmoothLevel(v, context, level1.varFullName, delay3Val, argInit, 2)
+    const level3 = generateSmoothLevel(v, context, level2.varFullName, delay3Val, argInit, 3)
     // For `SMOOTH3[I]`, the smoothVarRefId is the final level var's refId
-    v.smoothVarRefId = level3VarName[0]
+    v.smoothVarRefId = level3.varRefId
+    // Add the generated variables to the model
+    context.defineVariables([level1.eqn, level2.eqn, level3.eqn])
   }
 }
 
@@ -126,11 +130,14 @@ function generateSmoothLevel(v, context, argInput, argDelay, argInit, levelNumbe
   if (isSeparatedVar(v)) {
     Model.addNonAtoAVar(canonicalName(levelVarBaseName), [true])
   }
-  context.defineVariable(levelEqn)
   context.addVarReference(levelVarRefId)
 
   // The name of the level variable returned here includes the original subscript/dimension names
   // (not the separated subscript names) so that the `argInput` is correct for the 2nd and 3rd levels
   const levelVarFullName = `${levelVarBaseName}${origSubs}`
-  return [levelVarRefId, levelVarFullName]
+  return {
+    varRefId: levelVarRefId,
+    varFullName: levelVarFullName,
+    eqn: levelEqn
+  }
 }

packages/compile/src/model/read-equation-fn-trend.js

@@ -24,12 +24,13 @@ export function generateTrendVariables(v, callExpr, context) {
   const subs = context.extractSubscriptsFromVarNames(argInput, argAvgTime, argInitVal)
 
   // Generate a level variable that will be used in place of the `TREND` call
+  const eqns = []
   const levelVarName = newLevelVarName()
   const levelLHS = `${levelVarName}${subs}`
   // TODO: There should be parens around the arguments in this equation in case any of them
   // is an expression and not a simple constant
   const levelEqn = `${levelLHS} = INTEG((${argInput} - ${levelLHS}) / ${argAvgTime}, ${argInput} / (1 + ${argInitVal} * ${argAvgTime})) ~~|`
-  context.defineVariable(levelEqn)
+  eqns.push(levelEqn)
   context.addVarReference(canonicalName(levelVarName))
 
   // Generate a aux variable that will be used in place of the `TREND` call
@@ -38,7 +39,10 @@ export function generateTrendVariables(v, callExpr, context) {
   // TODO: There should be parens around the arguments in this equation in case any of them
   // is an expression and not a simple constant
   const auxEqn = `${auxLHS} = ZIDZ(${argInput} - ${levelLHS}, ${argAvgTime} * ABS(${levelLHS})) ~~|`
-  context.defineVariable(auxEqn)
+  eqns.push(auxEqn)
   v.trendVarName = canonicalName(auxVarName)
   context.addVarReference(v.trendVarName)
+
+  // Add the generated variables to the model
+  context.defineVariables(eqns)
 }

packages/compile/src/model/read-equation-fn-with-lookup.js

@@ -23,7 +23,7 @@ export function generateLookup(v, callExpr, context) {
   const lookupVarName = newLookupVarName()
   const lookupDef = toPrettyString(lookupArg, { compact: true })
   const lookupEqn = `${lookupVarName}${lookupDef} ~~|`
-  context.defineVariable(lookupEqn)
+  context.defineVariables([lookupEqn])
 
   // Keep track of all lookup variables that are referenced.  This will be used later to decide
   // whether a lookup variable needs to be included in generated code.

packages/compile/src/model/read-equations.js

@@ -104,16 +104,26 @@ class Context {
   }
 
   /**
-   * Define a new variable with the given equation.  This will add the variable to the `Model`
-   * and then perform the same `readEquation` step that is applied to all other regular variables.
+   * Define new variables for the given equations that are generated at compile time.  This will
+   * add the variables for the given equations to the `Model` first, and after they are added,
+   * it will perform the* same `readEquation` step on each that is applied to all other regular
+   * variables.
    *
-   * @param {*} eqnText The equation in Vensim format.
+   * NOTE: In the case where multiple equations are generated at compile time for the purposes
+   * of implementing a complex function (e.g., `DELAY3`), this should be called once only after
+   * the equation text for those generated variables is known.  This ensures that all variables
+   * are defined in the model before `readEquation` performs further processing, similar to the
+   * process we use when reading the original model (we first call `readVariables` on all
+   * model variable definitions before calling `readEquation` on each).
+   *
+   * @param {string[]} eqnStrings An array of individual equation strings in Vensim format.
    */
-  defineVariable(eqnText) {
+  defineVariables(eqnStrings) {
     // Parse the equation text
+    const eqnText = eqnStrings.join('\n')
     const parsedModel = { kind: 'vensim', root: parseVensimModel(eqnText) }
 
-    // Create one or more `Variable` instances from the equation
+    // Create one or more `Variable` instances from the equations
     const vars = readVariables(parsedModel)
 
     // Add the variables to the `Model`