Fix #11054 - Crash with schedule with missing day types by jmarrec · Pull Request #11085 · NatLabRockies/EnergyPlus

jmarrec · 2025-05-26T10:21:35Z

Pull request overview

Fixes Crash with schedule with missing day types #11054

Description of the purpose of this PR

Confirmed that the defect file segfaults before, and works with this PR, and the eplusout.err has the expected warning

   ** Warning ** ProcessScheduleInput: Schedule:Compact = WINDOWVENTSCHED
   **   ~~~   ** has missing day types in Through=12/31
   **   ~~~   ** Last "For" field=FOR: MONDAY TUESDAY WEDNESDAY THURSDAY FRIDAY SATURDAY
   **   ~~~   ** Missing day types= "Sunday", "Holiday", "SummerDesignDay", "WinterDesignDay", "CustomDay1", "CustomDay2"
   **   ~~~   ** Missing day types will have 0.0 as Schedule Values

Pull Request Author

Reviewer

``` [ RUN ] EnergyPlusFixture.ScheduleCompact_MissingDayTypes Process 1427200 stopped * thread #1, name = 'energyplus_test', stop reason = signal SIGSEGV: address not mapped to object (fault address: 0x70) frame #0: 0x0000555556a3e650 energyplus_tests`std::__cxx1998::vector<double, std::allocator<double>>::size(this=0x0000000000000068 size=0) const at stl_vector.h:993:40 990 _GLIBCXX_NODISCARD _GLIBCXX20_CONSTEXPR 991 size_type 992 size() const _GLIBCXX_NOEXCEPT -> 993 { return size_type(this->_M_impl._M_finish - this->_M_impl._M_start); } 994 995 /** Returns the size() of the largest possible %vector. */ 996 _GLIBCXX_NODISCARD _GLIBCXX20_CONSTEXPR (lldb) bt * thread #1, name = 'energyplus_test', stop reason = signal SIGSEGV: address not mapped to object (fault address: 0x70) * frame #0: 0x0000555556a3e650 energyplus_tests`std::__cxx1998::vector<double, std::allocator<double>>::size(this=0x0000000000000068 size=0) const at stl_vector.h:993:40 frame #1: 0x0000555557784d86 energyplus_tests`std::__debug::vector<double, std::allocator<double>>::operator[](this=0x0000000000000050 size=0, __n=27) const at vector:450:2 frame #2: 0x0000555559615269 energyplus_tests`EnergyPlus::Sched::ScheduleDetailed::getHrTsVal(this=0x000055555ed4b0c0, state=0x000055555e7b8190, hr=7, ts=4) const at ScheduleManager.cc:2520:82 ```

jmarrec · 2025-05-26T10:22:06Z

tst/EnergyPlus/unit/ScheduleManager.unit.cc

+TEST_F(EnergyPlusFixture, ScheduleCompact_MissingDayTypes)
+{
+    // Test for #11054
+    std::string const idf_objects = delimited_string({
+        "ScheduleTypeLimits,",
+        "  Any Number;              !- Name",
+
+        "Schedule:Compact,",
+        "  WindowVentSched,         !- Name",
+        "  Any Number,              !- Schedule Type Limits Name",
+        "  Through: 12/31,          !- Field 1",
+        "  For: Monday Tuesday Wednesday Thursday Friday Saturday, !- Field 2",
+        "  Until: 24:00,1;          !- Field 3",
+    });
+
+    ASSERT_TRUE(process_idf(idf_objects));
+
+    auto &s_glob = state->dataGlobal;
+
+    s_glob->TimeStepsInHour = 4;    // must initialize this to get schedules initialized
+    s_glob->MinutesInTimeStep = 15; // must initialize this to get schedules initialized
+    s_glob->TimeStepZone = 0.25;
+    s_glob->TimeStepZoneSec = s_glob->TimeStepZone * Constant::rSecsInHour;
+    state->dataEnvrn->CurrentYearIsLeapYear = false;
+
+    state->init_state(*state); // read schedules (this calls ProcessScheduleInput via ScheduleManagerData::init_state)
+
+    const std::string expected_error = delimited_string({
+        "   ** Warning ** ProcessScheduleInput: Schedule:Compact = WINDOWVENTSCHED",
+        "   **   ~~~   ** has missing day types in Through=12/31",
+        "   **   ~~~   ** Last \"For\" field=FOR: MONDAY TUESDAY WEDNESDAY THURSDAY FRIDAY SATURDAY",
+        R"(   **   ~~~   ** Missing day types= "Sunday", "Holiday", "SummerDesignDay", "WinterDesignDay", "CustomDay1", "CustomDay2")",
+        "   **   ~~~   ** Missing day types will have 0.0 as Schedule Values",
+    });
+
+    compare_err_stream(expected_error);
+
+    auto const *sch = dynamic_cast<Sched::ScheduleDetailed const *>(Sched::GetSchedule(*state, "WINDOWVENTSCHED"));
+    EXPECT_NE(sch, nullptr);
+    EXPECT_EQ(367, sch->weekScheds.size());
+    EXPECT_EQ(sch->weekScheds.front(), nullptr);
+    const auto &weekSched = sch->weekScheds[1];
+    EXPECT_EQ("WINDOWVENTSCHED_wk_1", weekSched->Name);
+    for (size_t i = 2; i < 367; ++i) {
+        EXPECT_EQ(weekSched, sch->weekScheds[i]);
+    }
+    EXPECT_EQ((int)Sched::DayType::Num, weekSched->dayScheds.size());
+
+    EXPECT_EQ(nullptr, weekSched->dayScheds[(int)Sched::DayType::Unused]);
+    EXPECT_EQ(nullptr, weekSched->dayScheds[(int)Sched::DayType::Sunday]);
+
+    ASSERT_NE(nullptr, weekSched->dayScheds[(int)Sched::DayType::Monday]);
+    ASSERT_NE(nullptr, weekSched->dayScheds[(int)Sched::DayType::Tuesday]);
+    ASSERT_NE(nullptr, weekSched->dayScheds[(int)Sched::DayType::Wednesday]);
+    ASSERT_NE(nullptr, weekSched->dayScheds[(int)Sched::DayType::Thursday]);
+    ASSERT_NE(nullptr, weekSched->dayScheds[(int)Sched::DayType::Friday]);
+    ASSERT_NE(nullptr, weekSched->dayScheds[(int)Sched::DayType::Saturday]);
+    auto const &daySched = weekSched->dayScheds[(int)Sched::DayType::Monday];
+    for (int i = (int)Sched::DayType::Monday; i <= (int)Sched::DayType::Saturday; ++i) {
+        EXPECT_EQ(daySched, weekSched->dayScheds[i]);
+    }
+
+    EXPECT_EQ(nullptr, weekSched->dayScheds[(int)Sched::DayType::Holiday]);
+    EXPECT_EQ(nullptr, weekSched->dayScheds[(int)Sched::DayType::SummerDesignDay]);
+    EXPECT_EQ(nullptr, weekSched->dayScheds[(int)Sched::DayType::WinterDesignDay]);
+    EXPECT_EQ(nullptr, weekSched->dayScheds[(int)Sched::DayType::CustomDay1]);
+    EXPECT_EQ(nullptr, weekSched->dayScheds[(int)Sched::DayType::CustomDay2]);
+
+    state->dataEnvrn->Month = 1;
+    state->dataEnvrn->DayOfMonth = 1;
+    state->dataEnvrn->DayOfYear_Schedule = General::OrdinalDay(state->dataEnvrn->Month, state->dataEnvrn->DayOfMonth, 1);
+    s_glob->HourOfDay = 1;
+    s_glob->TimeStep = 1;
+    state->dataEnvrn->DSTIndicator = 0;
+    state->dataEnvrn->HolidayIndex = 0;
+
+    // Monday is defined, so we should get 1.0
+    state->dataEnvrn->DayOfWeek = 2;
+    EXPECT_NEAR(1.0, sch->getHrTsVal(*state, 7, 4), 0.000001);


New test, until here, everything is good including on develop.

jmarrec · 2025-05-26T10:22:34Z

tst/EnergyPlus/unit/ScheduleManager.unit.cc

+    // Now test a day that is not defined, like Sunday
+    // We shouldn't segfault, and it should default to returning 0.0
+    state->dataEnvrn->DayOfWeek = 1;
+    ASSERT_NO_THROW(sch->getHrTsVal(*state, 7, 4));
+    EXPECT_NEAR(0.0, sch->getHrTsVal(*state, 7, 4), 0.000001);


This segfaults on develop. With the fix, it correctly returns 0.0 as we explicitly say in the warning

jmarrec · 2025-05-26T10:22:55Z

tst/EnergyPlus/unit/ScheduleManager.unit.cc

+        "Schedule:Compact,",
+        "  WindowVentSched,         !- Name",
+        "  Any Number,              !- Schedule Type Limits Name",
+        "  Through: 12/31,          !- Field 1",
+        "  For: Monday Tuesday Wednesday Thursday Friday Saturday, !- Field 2",
+        "  Until: 24:00,1;          !- Field 3",


For a Sunday, I get 0 because it's missing, not 1.0.

But today is Tuesday.

jmarrec · 2025-05-26T10:23:02Z

src/EnergyPlus/ScheduleManager.cc

+        if (daySched == nullptr) {
+            // We already warned in ProcessScheduleInput that there were missing days: Missing day types will have 0.0 as Schedule Values
+            return 0.0;
+        }


This is fine. In a turn of events, my personal preference is if (!daySched) here, but I'm 100% fine with comparing to nullptr.

github-actions · 2025-05-26T11:15:48Z

⚠️ Regressions detected on macos-14 for commit `7204fe3`

Regression Summary

Table Big Diffs: 3

Myoldmopar

Great little fix here with tests, thanks @jmarrec

Myoldmopar · 2025-05-27T17:58:49Z

src/EnergyPlus/ScheduleManager.cc

+        if (daySched == nullptr) {
+            // We already warned in ProcessScheduleInput that there were missing days: Missing day types will have 0.0 as Schedule Values
+            return 0.0;
+        }


This is fine. In a turn of events, my personal preference is if (!daySched) here, but I'm 100% fine with comparing to nullptr.

Myoldmopar · 2025-05-27T17:59:03Z

tst/EnergyPlus/unit/ScheduleManager.unit.cc

+        "Schedule:Compact,",
+        "  WindowVentSched,         !- Name",
+        "  Any Number,              !- Schedule Type Limits Name",
+        "  Through: 12/31,          !- Field 1",
+        "  For: Monday Tuesday Wednesday Thursday Friday Saturday, !- Field 2",
+        "  Until: 24:00,1;          !- Field 3",


But today is Tuesday.

Myoldmopar · 2025-05-27T17:59:13Z

tst/EnergyPlus/unit/ScheduleManager.unit.cc

+    // Now test a day that is not defined, like Sunday
+    // We shouldn't segfault, and it should default to returning 0.0
+    state->dataEnvrn->DayOfWeek = 1;
+    ASSERT_NO_THROW(sch->getHrTsVal(*state, 7, 4));
+    EXPECT_NEAR(0.0, sch->getHrTsVal(*state, 7, 4), 0.000001);


Myoldmopar · 2025-05-28T13:11:15Z

And it's passing happily locally. Thanks @jmarrec, merging.

jmarrec added 2 commits May 26, 2025 10:29

Fix #11054 - Crash with schedule with missing day types

2fdd216

jmarrec self-assigned this May 26, 2025

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

jmarrec commented May 26, 2025

View reviewed changes

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

Myoldmopar approved these changes May 27, 2025

View reviewed changes

Myoldmopar merged commit 951d0fc into develop May 28, 2025
10 checks passed

Myoldmopar deleted the 11054_ScheduleCrash_MissingDayTypes branch May 28, 2025 13:11

jmarrec mentioned this pull request Aug 26, 2025

#11054 - Part 2 - Crash with schedule with missing day types #11173

Merged

20 tasks

mjwitte mentioned this pull request Sep 22, 2025

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

Closed

Conversation

jmarrec commented May 26, 2025

Pull request overview

Description of the purpose of this PR

Pull Request Author

Reviewer

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented May 26, 2025

⚠️ Regressions detected on macos-14 for commit 7204fe3

Uh oh!

Myoldmopar left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

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!

3 participants

⚠️ Regressions detected on macos-14 for commit `7204fe3`