-
Notifications
You must be signed in to change notification settings - Fork 222
Fix #5343 - Design Range Temperature for OS:CoolingTower:SingleSpeed is not being converted into EnergyPlus #5345
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
Conversation
…is not being converted into EnergyPlus
| Test/Construction_GTest.cpp | ||
| Test/ConstructionWithInternalSource_GTest.cpp | ||
| Test/ControllerOutdoorAir_GTest.cpp | ||
| Test/CoolingTowerSingleSpeed_GTest.cpp |
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.
There was no FT test for the object, so I started by adding one
| EXPECT_TRUE(ct.setDesignApproachTemperature(3.89)); | ||
| // ct.autosizeDesignRangeTemperature(); | ||
| EXPECT_TRUE(ct.setDesignRangeTemperature(5.55)); |
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.
Note
| EXPECT_EQ(3.89, idfObject.getDouble(CoolingTower_SingleSpeedFields::DesignApproachTemperature).get()); | ||
| EXPECT_EQ(5.55, idfObject.getDouble(CoolingTower_SingleSpeedFields::DesignRangeTemperature).get()); |
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.
$ ninja && Products/openstudio_energyplus_tests -- --gtest_filter=*ForwardTranslator_CoolingTowerSingleSpeed*
[100%][2/2] Linking CXX executable Products/openstudio_energyplus_tests
Running main() from gmock_main.cc
Note: Google Test filter = *ForwardTranslator_CoolingTowerSingleSpeed*
[==========] Running 1 test from 1 test suite.
[----------] Global test environment set-up.
[----------] 1 test from EnergyPlusFixture
[ RUN ] EnergyPlusFixture.ForwardTranslator_CoolingTowerSingleSpeed
/media/DataExt4/Software/Others/OpenStudio3/src/energyplus/Test/CoolingTowerSingleSpeed_GTest.cpp:115: Failure
Expected equality of these values:
5.55
idfObject.getDouble(CoolingTower_SingleSpeedFields::DesignRangeTemperature).get()
Which is: 3.89
[ FAILED ] EnergyPlusFixture.ForwardTranslator_CoolingTowerSingleSpeed (217 ms)
[----------] 1 test from EnergyPlusFixture (217 ms total)
[----------] Global test environment tear-down
[==========] 1 test from 1 test suite ran. (218 ms total)
[ PASSED ] 0 tests.
[ FAILED ] 1 test, listed below:
[ FAILED ] EnergyPlusFixture.ForwardTranslator_CoolingTowerSingleSpeed
| if (modelObject.isDesignRangeTemperatureAutosized()) { | ||
| idfObject.setString(openstudio::CoolingTower_SingleSpeedFields::DesignRangeTemperature, "Autosize"); | ||
| } else if ((d = modelObject.designApproachTemperature())) { | ||
| } else if ((d = modelObject.designRangeTemperature())) { |
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.
This was a copy paste error
|
CI Results for d438c3b:
|
|
also fixes (duplicate issue): #5338 |
|
Thanks @eringold I just took the issue list from the top down and didn't get there! |
joseph-robertson
left a comment
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.
Probably a copy-paste mistake of modelObject.designApproachTemperature()? New FT test 👍
|
Yeah, defiitely a copy paste issue. Strange that it took us (and I mean the entire community) 8 years to notice it though. |
Pull request overview
Pull Request Author
src/model/test): No Changesrc/energyplus/Test): Added a new unit test filewater_economizer.osmandcoolingtowers.osmsrc/osversion/VersionTranslator.cpp)Labels:
IDDChangeAPIChangePull Request - Ready for CIso that CI builds your PRReview Checklist
This will not be exhaustively relevant to every PR.