-
Notifications
You must be signed in to change notification settings - Fork 192
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix #5116 - Annoying FT warnings for always XX Scheduled missing the schedule type limits object #5117
Conversation
``` $ Products/openstudio_energyplus_tests -- --gtest_filter=*NoUselessWarnings* Running main() from /home/conan/w/BuildSingleReference/.conan/data/gtest/1.11.0/_/_/build/e019a06362b932ca5d1b082b6c112aa150c88de4/source_subfolder/googletest/src/gtest_main.cc Note: Google Test filter = *NoUselessWarnings* [==========] Running 1 test from 1 test suite. [----------] Global test environment set-up. [----------] 1 test from EnergyPlusFixture [ RUN ] EnergyPlusFixture.NoUselessWarnings /home/julien/Software/Others/OpenStudio/src/energyplus/Test/ForwardTranslator_GTest.cpp:987: Failure Expected equality of these values: 0 logMessages.size() Which is: 3 Expected no messages logged, got: ["Object of type 'Schedule:Constant' and named 'Always On Discrete', points to an object named OnOff from field 1, but that object cannot be located.", "Object of type 'Schedule:Constant' and named 'Always Off Discrete', points to an object named OnOff 1 from field 1, but that object cannot be located.", "Object of type 'Schedule:Constant' and named 'Always On Continuous', points to an object named Fractional from field 1, but that object cannot be located."] [ FAILED ] EnergyPlusFixture.NoUselessWarnings (16 ms) [----------] 1 test from EnergyPlusFixture (16 ms total) [----------] Global test environment tear-down [==========] 1 test from 1 test suite ran. (16 ms total) [ PASSED ] 0 tests. [ FAILED ] 1 test, listed below: [ FAILED ] EnergyPlusFixture.NoUselessWarnings ```
TEST_F(EnergyPlusFixture, NoUselessWarnings) { | ||
// Test for #5116 | ||
Model m; | ||
auto yd = m.getUniqueModelObject<model::YearDescription>(); | ||
EXPECT_TRUE(yd.setDayofWeekforStartDay("Tuesday")); | ||
|
||
ForwardTranslator ft; | ||
StringStreamLogSink sink; | ||
sink.setLogLevel(Warn); | ||
|
||
Workspace w = ft.translateModel(m); | ||
EXPECT_EQ(0u, ft.errors().size()); | ||
EXPECT_EQ(0u, ft.warnings().size()); | ||
|
||
std::vector<openstudio::LogMessage> logMessages = sink.logMessages(); | ||
|
||
std::vector<std::string> logStrings; | ||
std::transform(logMessages.cbegin(), logMessages.cend(), std::back_inserter(logStrings), | ||
[](const auto& logMessage) { return logMessage.logMessage(); }); | ||
EXPECT_EQ(0, logMessages.size()) << fmt::format("Expected no messages logged, got: {}", logStrings); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
New test.
Before fix:
$ Products/openstudio_energyplus_tests -- --gtest_filter=*NoUselessWarnings*
Running main() from /home/conan/w/BuildSingleReference/.conan/data/gtest/1.11.0/_/_/build/e019a06362b932ca5d1b082b6c112aa150c88de4/source_subfolder/googletest/src/gtest_main.cc
Note: Google Test filter = *NoUselessWarnings*
[==========] Running 1 test from 1 test suite.
[----------] Global test environment set-up.
[----------] 1 test from EnergyPlusFixture
[ RUN ] EnergyPlusFixture.NoUselessWarnings
/home/julien/Software/Others/OpenStudio/src/energyplus/Test/ForwardTranslator_GTest.cpp:987: Failure
Expected equality of these values:
0
logMessages.size()
Which is: 3
Expected no messages logged, got: ["Object of type 'Schedule:Constant' and named 'Always On Discrete', points to an object named OnOff from field 1, but that object cannot be located.", "Object of type 'Schedule:Constant' and named 'Always Off Discrete', points to an object named OnOff 1 from field 1, but that object cannot be located.", "Object of type 'Schedule:Constant' and named 'Always On Continuous', points to an object named Fractional from field 1, but that object cannot be located."]
[ FAILED ] EnergyPlusFixture.NoUselessWarnings (16 ms)
[----------] 1 test from EnergyPlusFixture (16 ms total)
[----------] Global test environment tear-down
[==========] 1 test from 1 test suite ran. (16 ms total)
[ PASSED ] 0 tests.
[ FAILED ] 1 test, listed below:
[ FAILED ] EnergyPlusFixture.NoUselessWarnings
// Make sure these get in the idf file | ||
{ | ||
auto schedule = model.alwaysOnDiscreteSchedule(); | ||
translateAndMapModelObject(schedule); | ||
schedule = model.alwaysOffDiscreteSchedule(); | ||
translateAndMapModelObject(schedule); | ||
schedule = model.alwaysOnContinuousSchedule(); | ||
translateAndMapModelObject(schedule); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just reordering things a bit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reordering is the fix?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yep. auto schedule = model.alwaysOnDiscreteSchedule(); => this creates a schedule BUT also a ScheduleTypeLimits, if it wasn't instantiated already.
I put it above and not below the loop to translate all ScheduleTypeLimits.
Translating a scheduleconstant does not trigger translate of the ScheduleTypeLimits attached to it.
CI Results for dc326bb:
|
Pull request overview
Pull Request Author
src/model/test
)src/energyplus/Test
)src/osversion/VersionTranslator.cpp
)Labels:
IDDChange
APIChange
Pull Request - Ready for CI
so that CI builds your PRReview Checklist
This will not be exhaustively relevant to every PR.