Fix #11026 - Bad thermostat control type value results in crash due to null schedule pointer #11074

jmarrec · 2025-05-20T16:22:30Z

Pull request overview

Fixes Bad thermostat control type value results in crash due to null schedule pointer #11026

Description of the purpose of this PR

This PR reverts to the 24.2.0 functionality which catches these errors during the GetInput (GetZoneAirSetPoints).

Related Schedule API #10848

The Defect file, when run with this PR will no longer segfault, but will Fatal out as expected, as was done in 24.2.0

   ** Warning ** processZoneEquipmentInput: ZoneHVAC:EquipmentList = LIVING_EQUIPMENT
   **   ~~~   ** ...zero assigned to Zone Equipment Heating or No-Load Sequence=1, apparent gap in sequence assignments in this equipment list.
   ** Severe  ** Control Type Schedule=ALWAYS_ON
   **   ~~~   ** ..specifies 1 (ThermostatSetpoint:SingleHeating) as the control type. Not valid for this zone.
   **   ~~~   ** ..reference ZoneControl:Thermostat=THERMOSTAT_LIVING
   **   ~~~   ** ..reference ZONE=LIVING
   ** Severe  ** GetStagedDualSetpoint: Errors with invalid names in ZoneControl:Thermostat:StagedDualSetpoint objects.
   **   ~~~   ** ...These will not be read in.  Other errors may occur.
   **  Fatal  ** Errors getting Zone Control input data.  Preceding condition(s) cause termination.

Pull Request Author

Reviewer

jmarrec · 2025-05-20T16:22:53Z

tst/EnergyPlus/unit/ZoneTempPredictorCorrector.unit.cc

+        "ZoneControl:Thermostat,",
+        "  Zone1 Thermostat,                       !- Name",
+        "  Zone1,                                  !- Zone or ZoneList Name",
+        "  Single HEATING Control Type Sched,      !- Control Type Schedule Name",
+        "  ThermostatSetpoint:SingleCooling,       !- Control 1 Object Type",
+        "  Thermostat Setpoint Single Cooling;     !- Control 1 Name",
+
+        "Schedule:Constant,",
+        "  Single HEATING Control Type Sched,      !- Name",
+        "  Control Type,                           !- Schedule Type Limits Name",
+        "  1;                                      !- Hourly Value", // <-------- 1 = Single Heating, which is WRONG


Test reproduces the defect

jmarrec · 2025-05-20T16:23:20Z

tst/EnergyPlus/unit/ZoneTempPredictorCorrector.unit.cc

+    GetZoneData(*state, ErrorsFound);
+    ASSERT_FALSE(ErrorsFound);
+
+    EXPECT_THROW(GetZoneAirSetPoints(*state), EnergyPlus::FatalError);
+    std::string const error_string = delimited_string({
+        "   ** Severe  ** Control Type Schedule=SINGLE HEATING CONTROL TYPE SCHED",
+        "   **   ~~~   ** ..specifies 1 (ThermostatSetpoint:SingleHeating) as the control type. Not valid for this zone.",
+        "   **   ~~~   ** ..reference ZoneControl:Thermostat=ZONE1 THERMOSTAT",
+        "   **   ~~~   ** ..reference ZONE=ZONE1",
+        "   ** Severe  ** GetStagedDualSetpoint: Errors with invalid names in ZoneControl:Thermostat:StagedDualSetpoint objects.",
+        "   **   ~~~   ** ...These will not be read in.  Other errors may occur.",
+        "   **  Fatal  ** Errors getting Zone Control input data.  Preceding condition(s) cause termination.",
+        "   ...Summary of Errors that led to program termination:",
+        "   ..... Reference severe error count=2",
+        "   ..... Last severe error=GetStagedDualSetpoint: Errors with invalid names in ZoneControl:Thermostat:StagedDualSetpoint objects.",
+    });
+    EXPECT_TRUE(compare_err_stream(error_string, true));
+    // Avoid calling InitZoneAirSetPoints(*state); but still initialize needed arrays
+    state->dataHeatBalFanSys->TempControlType.allocate(1);
+    state->dataHeatBalFanSys->TempControlTypeRpt.allocate(1);
+    state->dataHeatBalFanSys->zoneTstatSetpts.allocate(1);
+
+    CalcZoneAirTempSetPoints(*state);


Initially

/home/julien/Software/Others/EnergyPlus/tst/EnergyPlus/unit/ZoneTempPredictorCorrector.unit.cc:1152: Failure Expected: GetZoneAirSetPoints(*state) throws an exception of type EnergyPlus::FatalError. Actual: it throws nothing. /home/julien/Software/Others/EnergyPlus/tst/EnergyPlus/unit/Fixtures/EnergyPlusFixture.cc:211: Failure Expected equality of these values: expected_string Which is: "\n" stream_str Which is: "" /home/julien/Software/Others/EnergyPlus/tst/EnergyPlus/unit/ZoneTempPredictorCorrector.unit.cc:1154: Failure Value of: compare_err_stream(error_string, true) Actual: false Expected: true Process 92525 stopped * thread #1, name = 'energyplus_test', stop reason = signal SIGSEGV: address not mapped to object (fault address: 0x4c) frame #0: 0x0000555556fc818a energyplus_tests`EnergyPlus::Sched::Schedule::getCurrentVal(this=0x0000000000000000) const at ScheduleManager.hh:275:20 272 273 Real64 getCurrentVal() const 274 { -> 275 return EMSActuatedOn ? EMSVal : currentVal; 276 } 277 278 // Looks up a given Schedule value for an hour & timestep, minding whether DST is enabled or not

jmarrec · 2025-05-20T16:24:13Z

src/EnergyPlus/ZoneTempPredictorCorrector.cc

        for (HVAC::SetptType setptType : HVAC::setptTypes) {
            auto const &setpt = tempZone.setpts[(int)setptType];
-            if (!setpt.isUsed) continue;
+
+            if (!setpt.isUsed) {
+                // Catch early issues
+                if (tempZone.setptTypeSched->hasVal(state, (int)setptType)) {
+                    ShowSevereError(state, format("Control Type Schedule={}", tempZone.setptTypeSched->Name));
+                    ShowContinueError(
+                        state,
+                        format("..specifies {} ({}) as the control type. Not valid for this zone.", (int)setptType, setptTypeNames[(int)setptType]));
+                    ShowContinueError(state, format("..reference {}={}", cZControlTypes((int)ZoneControlTypes::TStat), tempZone.Name));
+                    ShowContinueError(state, format("..reference ZONE={}", tempZone.ZoneName));
+                    ErrorsFound = true;
+                }
+                continue;
+            }


Fix:

if !setpt.isUsed but setptTypeSched hasVal => throw

```shell /home/julien/Software/Others/EnergyPlus/tst/EnergyPlus/unit/ZoneTempPredictorCorrector.unit.cc:1152: Failure Expected: GetZoneAirSetPoints(*state) throws an exception of type EnergyPlus::FatalError. Actual: it throws nothing. /home/julien/Software/Others/EnergyPlus/tst/EnergyPlus/unit/Fixtures/EnergyPlusFixture.cc:211: Failure Expected equality of these values: expected_string Which is: "\n" stream_str Which is: "" /home/julien/Software/Others/EnergyPlus/tst/EnergyPlus/unit/ZoneTempPredictorCorrector.unit.cc:1154: Failure Value of: compare_err_stream(error_string, true) Actual: false Expected: true Process 92525 stopped * thread #1, name = 'energyplus_test', stop reason = signal SIGSEGV: address not mapped to object (fault address: 0x4c) frame #0: 0x0000555556fc818a energyplus_tests`EnergyPlus::Sched::Schedule::getCurrentVal(this=0x0000000000000000) const at ScheduleManager.hh:275:20 272 273 Real64 getCurrentVal() const 274 { -> 275 return EMSActuatedOn ? EMSVal : currentVal; 276 } 277 278 // Looks up a given Schedule value for an hour & timestep, minding whether DST is enabled or not ```

setptTypes was sized to 5 element, but only 4 were explicitly declared in the initializer. The 5th is zero initialized, which added SetptType::Uncontrolled cf https://godbolt.org/z/jbbzc9hnn

github-actions · 2025-05-23T16:32:27Z

⚠️ Regressions detected on macos-14 for commit `0b04a3d`

Regression Summary

Table Big Diffs: 3

jmarrec · 2025-05-26T10:29:37Z

Not sure what the macos-14 regression are tripping on.

Myoldmopar

This looks great. Catching the control type value looks fine and the unit tests look sufficient.

Myoldmopar · 2025-05-28T14:05:23Z

src/EnergyPlus/ZoneTempPredictorCorrector.cc

+                    ShowContinueError(state, format("..reference {}={}", cZControlTypes((int)ZoneControlTypes::TStat), tempZone.Name));
+                    ShowContinueError(state, format("..reference ZONE={}", tempZone.ZoneName));
+                    ErrorsFound = true;
+                }


I'm trying to think about what happens if someone uses an API actuator to adjust the schedule value later. I know there are probably lots of places in the code that this could cause some issues, so not necessarily saying we need to do anything here, just thinking about possible impacts.

Myoldmopar · 2025-05-28T14:10:14Z

Looking good here, merging this. Thanks @jmarrec

jmarrec self-assigned this May 20, 2025

jmarrec added the Defect Includes code to repair a defect in EnergyPlus label May 20, 2025

jmarrec commented May 20, 2025

View reviewed changes

jmarrec changed the title ~~FIx #11026 - Bad thermostat control type value results in crash due to null schedule pointer~~ Fix #11026 - Bad thermostat control type value results in crash due to null schedule pointer May 20, 2025

jmarrec force-pushed the 11026_Bad_Thermostat_Control_Type branch from c900f8a to e1f17a1 Compare May 22, 2025 13:10

jmarrec added 7 commits May 23, 2025 17:33

#11026 - Update expected error message to match 24.2.0

95c4619

Fix #11026 - Catch wrong Control Type Schedule Values like on 24.2.0

4f5b39b

Remove subsequent operations after fatal

5202a6d

Fix initialization missing in unit tests

69d4785

Don't check uncontrolled to error

6d40332

Actually there was an issue with setptTypes

c37009f

setptTypes was sized to 5 element, but only 4 were explicitly declared in the initializer. The 5th is zero initialized, which added SetptType::Uncontrolled cf https://godbolt.org/z/jbbzc9hnn

jmarrec force-pushed the 11026_Bad_Thermostat_Control_Type branch from 87f404a to c37009f Compare May 23, 2025 15:38

jmarrec requested a review from Myoldmopar May 26, 2025 10:23

Myoldmopar approved these changes May 28, 2025

View reviewed changes

Myoldmopar merged commit ab397a3 into develop May 28, 2025
9 checks passed

Myoldmopar deleted the 11026_Bad_Thermostat_Control_Type branch May 28, 2025 14:10

JasonGlazer mentioned this pull request Sep 22, 2025

Crash with empty err file when Schedule:Compact missing For days #11222

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix #11026 - Bad thermostat control type value results in crash due to null schedule pointer #11074

Fix #11026 - Bad thermostat control type value results in crash due to null schedule pointer #11074

Uh oh!

jmarrec commented May 20, 2025

Uh oh!

jmarrec May 20, 2025

Uh oh!

jmarrec May 20, 2025

Uh oh!

jmarrec May 20, 2025

Uh oh!

github-actions bot commented May 23, 2025

Uh oh!

jmarrec commented May 26, 2025

Uh oh!

Myoldmopar left a comment

Uh oh!

Myoldmopar May 28, 2025

Uh oh!

Myoldmopar commented May 28, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Fix #11026 - Bad thermostat control type value results in crash due to null schedule pointer #11074

Fix #11026 - Bad thermostat control type value results in crash due to null schedule pointer #11074

Uh oh!

Conversation

jmarrec commented May 20, 2025

Pull request overview

Description of the purpose of this PR

Pull Request Author

Reviewer

Uh oh!

jmarrec May 20, 2025

Choose a reason for hiding this comment

Uh oh!

jmarrec May 20, 2025

Choose a reason for hiding this comment

Uh oh!

jmarrec May 20, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented May 23, 2025

⚠️ Regressions detected on macos-14 for commit 0b04a3d

Uh oh!

jmarrec commented May 26, 2025

Uh oh!

Myoldmopar left a comment

Choose a reason for hiding this comment

Uh oh!

Myoldmopar May 28, 2025

Choose a reason for hiding this comment

Uh oh!

Myoldmopar commented May 28, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

⚠️ Regressions detected on macos-14 for commit `0b04a3d`