Catch EMS variables initialized after reference, follow-up#11551
Catch EMS variables initialized after reference, follow-up#11551joseph-robertson wants to merge 11 commits intodevelopfrom
Conversation
|
|
|
|
|
|
|
|
||
| } else if (thisOperand.Type == Value::Variable) { | ||
| auto const &thisErlVar = state.dataRuntimeLang->ErlVariable(thisOperand.Variable); | ||
| auto &thisErlVar = state.dataRuntimeLang->ErlVariable(thisOperand.Variable); |
There was a problem hiding this comment.
Remove const since we may be updating Value.SetupInit.
| EnergyManagementSystem:GlobalVariable, | ||
| A, !- Erl Variable 1 Name | ||
| B, !- Erl Variable 2 Name | ||
| C, !- Erl Variable 3 Name | ||
| D, !- <none> | ||
| E, !- <none> | ||
| F, !- <none> | ||
| G, !- <none> | ||
| H, !- <none> | ||
| I, !- <none> | ||
| J, !- <none> | ||
| K, !- <none> | ||
| L, !- <none> | ||
| M, !- <none> | ||
| N, !- <none> | ||
| O, !- <none> | ||
| P, !- <none> | ||
| Q, !- <none> | ||
| R, !- <none> | ||
| S, !- <none> | ||
| T, !- <none> | ||
| U, !- <none> | ||
| V, !- <none> | ||
| W, !- <none> | ||
| X, !- <none> | ||
| Y, !- <none> | ||
| Z, !- <none> | ||
| AA, !- <none> | ||
| BB, !- <none> | ||
| CC, !- <none> | ||
| DD, !- <none> | ||
| EE, !- <none> | ||
| FF, !- <none> | ||
| GG, !- <none> | ||
| HH, !- <none> | ||
| II; !- <none> |
There was a problem hiding this comment.
This reverts the IDF additions from #11409.
| EnergyManagementSystem:GlobalVariable, | ||
| Qducts, !- Erl Variable 1 Name | ||
| Qsupply, !- Erl Variable 2 Name | ||
| Qexhaust; !- Erl Variable 3 Name |
There was a problem hiding this comment.
This reverts the IDF additions from #11409.
| EnergyManagementSystem:Program, | ||
| init, !- Name | ||
| Set CENTRAL_AC_AND_FURNACE_AIRLOOP_0_DUCTIMBALLKSUPFANEQUIVDZ = 0.0, !- <none> | ||
| Set CENTRAL_AC_AND_FURNACE_AIRLOOP_0_DUCTIMBALLKEXHFANEQUIVDZ = 0.0, !- <none> | ||
| Set CENTRAL_AC_AND_FURNACE_AIRLOOP_0_DUCTIMBALLKSUPFANEQUIVCOND = 0.0, | ||
| Set CENTRAL_AC_AND_FURNACE_AIRLOOP_0_DUCTIMBALLKEXHFANEQUIVCOND = 0.0; | ||
|
|
||
| EnergyManagementSystem:ProgramCallingManager, | ||
| init calling manager, !- Name | ||
| BeginNewEnvironment, !- EnergyPlus Model Calling Point | ||
| init; !- Program Name 1 |
There was a problem hiding this comment.
This fixes the legitimate error that exists in the EMS programs.
| DeltaT, !- Erl Variable 2 Name | ||
| ratioT, !- Erl Variable 3 Name |
There was a problem hiding this comment.
This reverts the IDF additions from #11409.
| if (!state.dataGlobal->DoingSizing) { | ||
| cEnvHeader = " During Setup, Environment="; | ||
| } else { | ||
| cEnvHeader = " During Setup & Sizing, Environment="; | ||
| } |
There was a problem hiding this comment.
This is entered during SetupSimulation.
| ReturnValue.Error = "EvaluateExpression: Variable = '" + thisErlVar.Name + "' used in expression has not been initialized!"; | ||
| // Use SetupInit in BeginEnvrnInitializeRuntimeLanguage for "un-initializing" Erl variables that may have been | ||
| // initialized to zero during setup. This can happen since SetupSimulation does not call BeginNewEnvironment. | ||
| thisErlVar.Value.SetupInit = false; |
There was a problem hiding this comment.
Revert the change from #11409, and instead just add this single line.
| if (!state.dataRuntimeLang->ErlVariable(ErlVariableNum).Value.SetupInit) { | ||
| state.dataRuntimeLang->ErlVariable(ErlVariableNum).Value.initialized = false; | ||
| } |
There was a problem hiding this comment.
Right before BeginNewEnvironment, "un-initialize" any Erl variables that had originally hit the EvaluateExpression: Variable = 'xyz' used in expression has not been initialized! error. If BeginNewEnvironment initializes them, then initialized is set back to true and we're good; if it doesn't, or there are other legitimate Erl variable initialization errors, the EvaluateExpression ... will throw.
There was a problem hiding this comment.
Pull request overview
This PR refines EMS runtime diagnostics and initialization behavior to better catch “variable used before initialization” cases and to make EDD/trace timing labels distinguish SetupSimulation from Warmup.
Changes:
- Add
SetupFlagto tag SetupSimulation timesteps and use it in EMS trace + timestamped error outputs (“During Setup” vs “During Warmup”). - Add
SetupInittracking on Erl values to support re-initialization behavior across environments when uninitialized-variable errors occur. - Update several test IDFs to avoid/clean up EMS initialization and variable declarations.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| testfiles/_ResidentialBase.idf | Adds a BeginNewEnvironment EMS init program to pre-initialize specific duct imbalance variables. |
| testfiles/RetailPackagedTESCoil.idf | Adjusts EMS initialization and global variable declarations for the roof microclimate EMS. |
| testfiles/HospitalBaselineReheatReportEMS.idf | Removes a large block of temporary EMS global variable declarations (A..II). |
| src/EnergyPlus/UtilityRoutines.cc | Updates timestamp headers to label “During Setup” when applicable. |
| src/EnergyPlus/SimulationManager.cc | Sets/clears the new SetupFlag around SetupSimulation. |
| src/EnergyPlus/RuntimeLanguageProcessor.cc | Updates EMS trace timing strings and Erl variable/environment re-init behavior; refines uninitialized-variable handling. |
| src/EnergyPlus/DataRuntimeLanguage.hh | Adds SetupInit to ErlValueType and updates related constructors/defaults. |
| src/EnergyPlus/DataGlobals.hh | Adds SetupFlag global state. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| InitializeParameters, !- Name | ||
| Set Troof = 0.0, |
There was a problem hiding this comment.
InitializeParameters is called every timestep via the Model Flat Roof Microclimate ProgramCallingManager (BeginTimestepBeforePredictor). Adding Set Troof = 0.0 here resets Troof each timestep, which changes the microclimate algorithm because CalcBouyancyTerms uses Troof earlier in the same timestep. If the goal is only to avoid an uninitialized-variable error on the very first call, initialize Troof once at BeginNewEnvironment (or with a one-time init program/flag) rather than re-zeroing it every timestep.
| InitializeParameters, !- Name | |
| Set Troof = 0.0, | |
| InitializeRoofTemperature, !- Name | |
| Set Troof = 0.0; !- Program Line 1 | |
| EnergyManagementSystem:ProgramCallingManager, | |
| InitializeRoofTemperatureOnBeginNewEnvironment, !- Name | |
| BeginNewEnvironment, !- EnergyPlus Model Calling Point | |
| InitializeRoofTemperature; !- Program Name 1 | |
| EnergyManagementSystem:Program, | |
| InitializeParameters, !- Name |
There was a problem hiding this comment.
This fixes the legitimate uninitialized Troof error.
| int TrendVarPointer; // index to match in TrendVariable structure | ||
| std::string Error; // holds error message string for reporting | ||
| bool initialized; // true if number value has been SET (ie. has been on LHS in SET expression) | ||
| bool SetupInit; // false if EvaluateExpression is thrown for Erl variable |
There was a problem hiding this comment.
The SetupInit field comment is unclear/inaccurate: EvaluateExpression doesn't "throw", it returns an error ErlValueType. Consider renaming the flag or rewording the comment to describe the actual behavior (e.g., that it's used to mark variables involved in an uninitialized-variable error so they can be treated as uninitialized at BeginNewEnvironment).
| bool SetupInit; // false if EvaluateExpression is thrown for Erl variable | |
| bool SetupInit; // false when marked by an uninitialized-variable evaluation error so it can be treated as uninitialized later |
|
|
Pull request overview
Description of the purpose of this PR
Pull Request Author
Reviewer