Merge pull request #11525 from NatLabRockies/unused_actuator_warn

mitchute · web-flow · commit 35b4876f2fcf · 2026-04-22T14:06:45.000-06:00
Fix #10944: unused EMS actuator warning never fires
diff --git a/src/EnergyPlus/DataRuntimeLanguage.hh b/src/EnergyPlus/DataRuntimeLanguage.hh
@@ -327,6 +327,7 @@ namespace DataRuntimeLanguage {
         bool CheckedOkay;              // set to true once matched to available actuator
         int ErlVariableNum;            // points to global Erl variable, matches Name
         int ActuatorVariableNum;       // points to index match in EMSActuatorAvailable structure
+        bool wasActuated = false;      // issue #10944: true once any Erl program has set this actuator
 
         // Default Constructor
         ActuatorUsedType() : CheckedOkay(false), ErlVariableNum(0), ActuatorVariableNum(0)
diff --git a/src/EnergyPlus/EMSManager.cc b/src/EnergyPlus/EMSManager.cc
@@ -325,7 +325,7 @@ namespace EMSManager {
                                      state.dataRuntimeLang->NumExternalInterfaceFunctionalMockupUnitImportActuatorsUsed +
                                      state.dataRuntimeLang->NumExternalInterfaceFunctionalMockupUnitExportActuatorsUsed;
              ++ActuatorUsedLoop) {
-            auto const &thisActuatorUsed = state.dataRuntimeLang->EMSActuatorUsed(ActuatorUsedLoop);
+            auto &thisActuatorUsed = state.dataRuntimeLang->EMSActuatorUsed(ActuatorUsedLoop);
 
             int ErlVariableNum = thisActuatorUsed.ErlVariableNum;
             if (ErlVariableNum <= 0) {
@@ -1983,9 +1983,9 @@ namespace EMSManager {
     void checkForUnusedActuatorsAtEnd(EnergyPlusData &state)
     {
         // call at end of simulation to check if any of the user's actuators were never initialized.
-        // Could be a mistake we want to help users catch // Issue #4404.
+        // Could be a mistake we want to help users catch // Issues #4404, #10944.
         for (int actuatorUsedLoop = 1; actuatorUsedLoop <= state.dataRuntimeLang->numActuatorsUsed; ++actuatorUsedLoop) {
-            if (!state.dataRuntimeLang->ErlVariable(state.dataRuntimeLang->EMSActuatorUsed(actuatorUsedLoop).ErlVariableNum).Value.initialized) {
+            if (!state.dataRuntimeLang->EMSActuatorUsed(actuatorUsedLoop).wasActuated) {
                 ShowWarningError(state,
                                  "checkForUnusedActuatorsAtEnd: Unused EMS Actuator detected, suggesting possible unintended programming error or "
                                  "spelling mistake.");
diff --git a/src/EnergyPlus/RuntimeLanguageProcessor.cc b/src/EnergyPlus/RuntimeLanguageProcessor.cc
@@ -458,6 +458,16 @@ void ParseStack(EnergyPlusData &state, int const StackNum)
                 VariableNum = NewEMSVariable(state, Variable, StackNum);
                 // Check for invalid variable name
 
+                // issue #10944: static reference check — if this SET targets an actuator's ErlVariable,
+                // mark the actuator as referenced. Catches typos at parse time; avoids false positives
+                // for conditional NULL-branch patterns that never fire during the run.
+                for (int aUsed = 1; aUsed <= state.dataRuntimeLang->numActuatorsUsed; ++aUsed) {
+                    if (state.dataRuntimeLang->EMSActuatorUsed(aUsed).ErlVariableNum == VariableNum) {
+                        state.dataRuntimeLang->EMSActuatorUsed(aUsed).wasActuated = true;
+                        break;
+                    }
+                }
+
                 if (Pos + 1 < Remainder.length()) {
                     Expression = stripped(Remainder.substr(Pos + 1));
                 } else {
diff --git a/src/EnergyPlus/api/datatransfer.cc b/src/EnergyPlus/api/datatransfer.cc
@@ -421,6 +421,14 @@ int getActuatorHandle(EnergyPlusState state, const char *componentType, const ch
         std::string const actuatorIDUC = EnergyPlus::Util::makeUPPER(availActuator.UniqueIDName);
         std::string const actuatorControlUC = EnergyPlus::Util::makeUPPER(availActuator.ControlTypeName);
         if (typeUC == actuatorTypeUC && keyUC == actuatorIDUC && controlUC == actuatorControlUC) {
+            // issue #10944: mark any IDF-declared EMSActuatorUsed entry as referenced once Python
+            // retrieves its handle — Python catches typos itself, so handle retrieval is the usage signal.
+            for (auto &usedActuator : thisState->dataRuntimeLang->EMSActuatorUsed) {
+                if (usedActuator.ActuatorVariableNum == handle) {
+                    usedActuator.wasActuated = true;
+                    break;
+                }
+            }
             if (availActuator.handleCount > 0) {
                 // If the handle is already used by an IDF EnergyManagementSystem:Actuator, we should warn the user
                 bool foundActuator = false;
diff --git a/testfiles/ASHRAE901_Hospital_STD2019_Denver.idf b/testfiles/ASHRAE901_Hospital_STD2019_Denver.idf
@@ -28006,12 +28006,6 @@
     Schedule:Compact,        !- Actuated Component Type
     Schedule Value;          !- Actuated Component Control Type
 
-  EnergyManagementSystem:Actuator,
-    BypassHXStatus,          !- Name
-    Bypass Heat Exchanger Status,  !- Actuated Component Unique Name
-    Schedule:Constant,       !- Actuated Component Type
-    Schedule Value;          !- Actuated Component Control Type
-
   EnergyManagementSystem:Actuator,
     HRC_SPM_Reset,           !- Name
     Heat Recovery Chiller Bypass Loop Setpoint Schedule,  !- Actuated Component Unique Name
@@ -28036,12 +28030,6 @@
     Plant Component Pump:ConstantSpeed,  !- Actuated Component Type
     On/Off Supervisory;      !- Actuated Component Control Type
 
-  EnergyManagementSystem:Actuator,
-    Chiller1PumpFlow,        !- Name
-    COOLSYS1 CHILLER1 PUMP,  !- Actuated Component Unique Name
-    Pump,                    !- Actuated Component Type
-    Pump Mass Flow Rate;     !- Actuated Component Control Type
-
   EnergyManagementSystem:Actuator,
     Chiller2PumpFlow,        !- Name
     COOLSYS1 CHILLER2 PUMP,  !- Actuated Component Unique Name
diff --git a/testfiles/ASHRAE901_OfficeLarge_STD2019_Denver.idf b/testfiles/ASHRAE901_OfficeLarge_STD2019_Denver.idf
@@ -13925,12 +13925,6 @@
     Plant Component Pump:ConstantSpeed,  !- Actuated Component Type
     On/Off Supervisory;      !- Actuated Component Control Type
 
-  EnergyManagementSystem:Actuator,
-    Chiller1PumpFlow,        !- Name
-    COOLSYS1 CHILLER1 PUMP,  !- Actuated Component Unique Name
-    Pump,                    !- Actuated Component Type
-    Pump Mass Flow Rate;     !- Actuated Component Control Type
-
   EnergyManagementSystem:Actuator,
     Chiller2PumpFlow,        !- Name
     COOLSYS1 CHILLER2 PUMP,  !- Actuated Component Unique Name
diff --git a/testfiles/ASHRAE901_OfficeLarge_STD2019_Denver_Chiller205.idf b/testfiles/ASHRAE901_OfficeLarge_STD2019_Denver_Chiller205.idf
@@ -13911,12 +13911,6 @@
     Plant Component Pump:ConstantSpeed,  !- Actuated Component Type
     On/Off Supervisory;      !- Actuated Component Control Type
 
-  EnergyManagementSystem:Actuator,
-    Chiller1PumpFlow,        !- Name
-    COOLSYS1 CHILLER1 PUMP,  !- Actuated Component Unique Name
-    Pump,                    !- Actuated Component Type
-    Pump Mass Flow Rate;     !- Actuated Component Control Type
-
   EnergyManagementSystem:Actuator,
     Chiller2PumpFlow,        !- Name
     COOLSYS1 CHILLER2 PUMP,  !- Actuated Component Unique Name
diff --git a/testfiles/ASHRAE901_OfficeLarge_STD2019_Denver_Chiller205_Detailed.idf b/testfiles/ASHRAE901_OfficeLarge_STD2019_Denver_Chiller205_Detailed.idf
@@ -13946,12 +13946,6 @@
     Plant Component Pump:ConstantSpeed,  !- Actuated Component Type
     On/Off Supervisory;      !- Actuated Component Control Type
 
-  EnergyManagementSystem:Actuator,
-    Chiller1PumpFlow,        !- Name
-    COOLSYS1 CHILLER1 PUMP,  !- Actuated Component Unique Name
-    Pump,                    !- Actuated Component Type
-    Pump Mass Flow Rate;     !- Actuated Component Control Type
-
   EnergyManagementSystem:Actuator,
     Chiller2PumpFlow,        !- Name
     COOLSYS1 CHILLER2 PUMP,  !- Actuated Component Unique Name
diff --git a/testfiles/ASHRAE901_OfficeLarge_STD2019_Denver_ChillerEIR.idf b/testfiles/ASHRAE901_OfficeLarge_STD2019_Denver_ChillerEIR.idf
@@ -13962,12 +13962,6 @@
     Plant Component Pump:ConstantSpeed,  !- Actuated Component Type
     On/Off Supervisory;      !- Actuated Component Control Type
 
-  EnergyManagementSystem:Actuator,
-    Chiller1PumpFlow,        !- Name
-    COOLSYS1 CHILLER1 PUMP,  !- Actuated Component Unique Name
-    Pump,                    !- Actuated Component Type
-    Pump Mass Flow Rate;     !- Actuated Component Control Type
-
   EnergyManagementSystem:Actuator,
     Chiller2PumpFlow,        !- Name
     COOLSYS1 CHILLER2 PUMP,  !- Actuated Component Unique Name
diff --git a/testfiles/EMSDemandManager_LargeOffice.idf b/testfiles/EMSDemandManager_LargeOffice.idf
@@ -10141,12 +10141,6 @@
     0.0000,                  !- On Cycle Parasitic Electric Load {W}
     1.0000;                  !- Sizing Factor
 
-  EnergyManagementSystem:Actuator,
-    HeatSys1_Boiler_Setpoint,!- Name
-    HeatSys1 Supply Equipment Outlet Node,  !- Actuated Component Unique Name
-    System Node Setpoint,    !- Actuated Component Type
-    Temperature Setpoint;    !- Actuated Component Control Type
-
   Branch,
     HeatSys1 Supply Equipment Bypass Branch,  !- Name
     ,                        !- Pressure Drop Curve Name
diff --git a/testfiles/EMSReplaceTraditionalManagers_LargeOffice.idf b/testfiles/EMSReplaceTraditionalManagers_LargeOffice.idf
@@ -10129,12 +10129,6 @@
     0.0000,                  !- On Cycle Parasitic Electric Load {W}
     1.0000;                  !- Sizing Factor
 
-  EnergyManagementSystem:Actuator,
-    HeatSys1_Boiler_Setpoint,!- Name
-    HeatSys1 Supply Equipment Outlet Node,  !- Actuated Component Unique Name
-    System Node Setpoint,    !- Actuated Component Type
-    Temperature Setpoint;    !- Actuated Component Control Type
-
   Branch,
     HeatSys1 Supply Equipment Bypass Branch,  !- Name
     ,                        !- Pressure Drop Curve Name
diff --git a/tst/EnergyPlus/unit/EMSManager.unit.cc b/tst/EnergyPlus/unit/EMSManager.unit.cc
@@ -3019,3 +3019,98 @@ TEST_F(EnergyPlusFixture, EMSManager_Sensor_On_ScheduleConstant)
     EXPECT_FALSE(schedDirect->EMSActuatedOn);
     EXPECT_EQ(0.0, schedDirect->EMSVal);
 }
+
+TEST_F(EnergyPlusFixture, UnusedActuatorWarning)
+{
+    // Issue #10944: user declares actuator A1, typos it as Al in the program.
+    // Al becomes a fresh local Erl variable; A1 is never assigned.
+    // End-of-sim check must warn about unused actuator A1.
+    std::string const idf_objects = delimited_string({
+
+        "OutdoorAir:Node, Test node;",
+
+        "EnergyManagementSystem:Actuator,",
+        "A1,                              !- Name",
+        "Test node,                       !- Actuated Component Unique Name",
+        "System Node Setpoint,            !- Actuated Component Type",
+        "Temperature Setpoint;            !- Actuated Component Control Type",
+
+        "EnergyManagementSystem:ProgramCallingManager,",
+        "Typo Test Manager,               !- Name",
+        "BeginTimestepBeforePredictor,    !- EnergyPlus Model Calling Point",
+        "TypoProgram;                     !- Program Name 1",
+
+        "EnergyManagementSystem:Program,",
+        "TypoProgram,",
+        "Set Al = 2;",
+
+    });
+
+    ASSERT_TRUE(process_idf(idf_objects));
+    state->init_state(*state);
+
+    OutAirNodeManager::SetOutAirNodes(*state);
+
+    EMSManager::CheckIfAnyEMS(*state);
+
+    state->dataEMSMgr->FinishProcessingUserInput = true;
+
+    bool anyRan;
+    EMSManager::ManageEMS(*state, EMSManager::EMSCallFrom::SetupSimulation, anyRan, ObjexxFCL::Optional_int_const());
+    EMSManager::ManageEMS(*state, EMSManager::EMSCallFrom::BeginTimestepBeforePredictor, anyRan, ObjexxFCL::Optional_int_const());
+
+    EMSManager::checkForUnusedActuatorsAtEnd(*state);
+
+    EXPECT_TRUE(compare_err_stream_substring("Unused EMS Actuator detected", false));
+    EXPECT_TRUE(compare_err_stream_substring("A1", true));
+}
+
+TEST_F(EnergyPlusFixture, UnusedActuatorWarning_ConditionalNullBranchNotFalsePositive)
+{
+    // Issue #10944: idiomatic `IF cond SET act = v ELSE SET act = NULL` pattern must NOT
+    // trigger the unused-actuator warning just because the condition branch never fires at runtime.
+    // Static parse-time reference check sees the SET target, flag flips even if only NULL branch runs.
+    std::string const idf_objects = delimited_string({
+
+        "OutdoorAir:Node, Test node;",
+
+        "EnergyManagementSystem:Actuator,",
+        "A1,                              !- Name",
+        "Test node,                       !- Actuated Component Unique Name",
+        "System Node Setpoint,            !- Actuated Component Type",
+        "Temperature Setpoint;            !- Actuated Component Control Type",
+
+        "EnergyManagementSystem:ProgramCallingManager,",
+        "Cond Null Manager,               !- Name",
+        "BeginTimestepBeforePredictor,    !- EnergyPlus Model Calling Point",
+        "CondNullProgram;                 !- Program Name 1",
+
+        // Condition always false at runtime -> only NULL branch fires -> old runtime flag would stay false.
+        // Static parse check sees `SET A1 = ...` in the IF branch -> flag flips at parse time regardless.
+        "EnergyManagementSystem:Program,",
+        "CondNullProgram,",
+        "IF 0 > 1,",
+        "SET A1 = 42,",
+        "ELSE,",
+        "SET A1 = NULL,",
+        "ENDIF;",
+
+    });
+
+    ASSERT_TRUE(process_idf(idf_objects));
+    state->init_state(*state);
+
+    OutAirNodeManager::SetOutAirNodes(*state);
+
+    EMSManager::CheckIfAnyEMS(*state);
+
+    state->dataEMSMgr->FinishProcessingUserInput = true;
+
+    bool anyRan;
+    EMSManager::ManageEMS(*state, EMSManager::EMSCallFrom::SetupSimulation, anyRan, ObjexxFCL::Optional_int_const());
+    EMSManager::ManageEMS(*state, EMSManager::EMSCallFrom::BeginTimestepBeforePredictor, anyRan, ObjexxFCL::Optional_int_const());
+
+    EMSManager::checkForUnusedActuatorsAtEnd(*state);
+
+    EXPECT_FALSE(compare_err_stream_substring("Unused EMS Actuator detected", false, false));
+}
diff --git a/tst/EnergyPlus/unit/api/datatransfer.unit.cc b/tst/EnergyPlus/unit/api/datatransfer.unit.cc
@@ -708,6 +708,41 @@ TEST_F(DataExchangeAPIUnitTestFixture, DataTransfer_Python_EMS_Override)
     EXPECT_TRUE(compare_err_stream(expectedError, true));
 }
 
+TEST_F(DataExchangeAPIUnitTestFixture, DataTransfer_PythonHandle_MarksActuatorAsUsed)
+{
+    // Issue #10944: when Python retrieves a handle for an IDF-declared actuator,
+    // mark wasActuated so the end-of-sim unused-actuator check doesn't false-positive
+    // on legitimate Python-driven actuators (Python catches typos itself at handle lookup).
+    std::string const idf_objects = delimited_string({
+        "OutdoorAir:Node, Test node;",
+        "EnergyManagementSystem:Actuator,",
+        "TempSetpointLo,          !- Name",
+        "Test node,               !- Actuated Component Unique Name",
+        "System Node Setpoint,    !- Actuated Component Type",
+        "Temperature Minimum Setpoint;    !- Actuated Component Control Type",
+    });
+
+    ASSERT_TRUE(process_idf(idf_objects));
+    OutAirNodeManager::SetOutAirNodes(*state);
+    EMSManager::CheckIfAnyEMS(*state);
+    state->dataEMSMgr->FinishProcessingUserInput = true;
+    bool anyRan;
+    EMSManager::ManageEMS(*state, EMSManager::EMSCallFrom::SetupSimulation, anyRan);
+    ASSERT_EQ(1, state->dataRuntimeLang->numActuatorsUsed);
+
+    // Before Python handle retrieval, the actuator is un-referenced
+    EXPECT_FALSE(state->dataRuntimeLang->EMSActuatorUsed(1).wasActuated);
+
+    int hActuator = getActuatorHandle(state, "System Node Setpoint", "Temperature Minimum Setpoint", "Test node");
+    EXPECT_GT(hActuator, -1);
+
+    // getActuatorHandle should have flipped the flag
+    EXPECT_TRUE(state->dataRuntimeLang->EMSActuatorUsed(1).wasActuated);
+
+    EMSManager::checkForUnusedActuatorsAtEnd(*state);
+    EXPECT_FALSE(compare_err_stream_substring("Unused EMS Actuator detected", false, false));
+}
+
 TEST_F(DataExchangeAPIUnitTestFixture, DataTransfer_Python_Python_Override)
 {
     // Test for #8084, should warn when getting a handle for an actuator via API Twice
