-
Notifications
You must be signed in to change notification settings - Fork 460
Fix #11156 - WaterThermalTanks's GetWaterThermalTankInput should trap ErrorsFound and throw a FatalError #11172
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 #11156 - WaterThermalTanks's GetWaterThermalTankInput should trap ErrorsFound and throw a FatalError #11172
Conversation
…t doesn't fatal due to missing zone ``` + ** Severe ** WaterHeater:HeatPump:PumpedCondenser=\"HPWHPUMPED\", not found + ** ~~~ ** Inlet Air Zone Name=\"ZONE1\". + ** Severe ** WaterHeater:HeatPump:WrappedCondenser=\"HPWHWRAPPED\", not found + ** ~~~ ** Inlet Air Zone Name=\"ZONE1\". + ** Severe ** WaterHeater:Mixed = HPWHPUMPED MIXED TANK: Ambient Temperature Zone not found = ZONE1 + ** Severe ** WaterHeater:Stratified = HPWHWRAPPED STRATIFIED TANK: Ambient Temperature Zone not found = ZONE1 + ** Severe ** WaterHeater:HeatPump:PumpedCondenser = HPWHPUMPED: + ** ~~~ ** ZoneHVAC:EquipmentList and ZoneHVAC:EquipmentConnections objects are required when Inlet Air Configuration is either ZoneAirOnly or ZoneAndOutdoorAir. + ** Severe ** WaterHeater:HeatPump:WrappedCondenser = HPWHWRAPPED: + ** ~~~ ** ZoneHVAC:EquipmentList and ZoneHVAC:EquipmentConnections objects are required when Inlet Air Configuration is either ZoneAirOnly or ZoneAndOutdoorAir. + ** Fatal ** GetWaterThermalTankInput: Errors found in processing Water Thermal Tank input ```
| } | ||
|
|
||
| bool GetWaterThermalTankInput(EnergyPlusData &state) | ||
| void GetWaterThermalTankInput(EnergyPlusData &state) |
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.
void
| if (ErrorsFound) { | ||
| ShowFatalError(state, format("GetWaterThermalTankInput: Errors found in processing Water Thermal Tank input.")); | ||
| } |
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.
And throw
| state->dataEnvrn->StdRhoAir = 1.0; | ||
|
|
||
| ASSERT_FALSE(WaterThermalTanks::GetWaterThermalTankInput(*state)); | ||
| WaterThermalTanks::GetWaterThermalTankInput(*state); |
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.
No need to wrap into "EXPECT_NO_THROW". We just rely on gtest catching the exception if needed. This is what we do in most tests right now
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, let it fail if it's gonna fail.
| DataZoneEquipment::GetZoneEquipmentData(*state); | ||
| ASSERT_FALSE(ErrorsFound); |
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.
Missing call to GetZoneEquipmentData here, GetWaterThermalTankInput would eventually complain that ZoneEquipConfig isn't allocated
EnergyPlus/src/EnergyPlus/WaterThermalTanks.cc
Lines 1974 to 1976 in acf7908
| if ((HPWH.InletAirConfiguration == WTTAmbientTemp::TempZone || HPWH.InletAirConfiguration == WTTAmbientTemp::ZoneAndOA) && | |
| HPWH.AmbientTempZone > 0) { | |
| if (allocated(state.dataZoneEquip->ZoneEquipConfig)) { |
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.
You need to remove the format from the new fatal at WaterThermalTanks.cc line 4906 so you might as well also remove line 1052 above since it doesn't do anything (i.e., GetZoneEquipmentData doesn't have the ErrorsFound argument so of course ErrorsFound is still false). Other than that this is good to go.
| EXPECT_THROW(WaterThermalTanks::GetWaterThermalTankInput(*state), EnergyPlus::FatalError); | ||
|
|
||
| std::string const error_string = | ||
| delimited_string({" ** Warning ** ProcessScheduleInput: Schedule:Constant = DUMMYSCH", | ||
| " ** ~~~ ** Schedule Type Limits Name is empty.", | ||
| " ** ~~~ ** Schedule will not be validated.", | ||
| " ** Severe ** WaterHeater:HeatPump:PumpedCondenser=\"ZONE4HEATPUMPWATERHEATER\":", | ||
| " ** ~~~ ** When Inlet Air Configuration=\"OUTDOORAIRONLY\".", | ||
| " ** ~~~ ** Outdoor Air Node Name and Exhaust Air Node Name must be specified.", | ||
| " ** Severe ** WaterHeater:HeatPump:PumpedCondenser=\"ZONE4HEATPUMPWATERHEATER\":", | ||
| " ** ~~~ ** Heat pump water heater fan outlet node name does not match next connected component.", | ||
| " ** ~~~ ** Fan outlet node name = ZONE4AIRINLETNODE"}); | ||
| std::string const error_string = delimited_string({ | ||
| " ** Warning ** ProcessScheduleInput: Schedule:Constant = DUMMYSCH", | ||
| " ** ~~~ ** Schedule Type Limits Name is empty.", | ||
| " ** ~~~ ** Schedule will not be validated.", | ||
| " ** Severe ** WaterHeater:HeatPump:PumpedCondenser=\"ZONE4HEATPUMPWATERHEATER\":", | ||
| " ** ~~~ ** When Inlet Air Configuration=\"OUTDOORAIRONLY\".", | ||
| " ** ~~~ ** Outdoor Air Node Name and Exhaust Air Node Name must be specified.", | ||
| " ** Severe ** WaterHeater:HeatPump:PumpedCondenser=\"ZONE4HEATPUMPWATERHEATER\":", | ||
| " ** ~~~ ** Heat pump water heater fan outlet node name does not match next connected component.", | ||
| " ** ~~~ ** Fan outlet node name = ZONE4AIRINLETNODE", | ||
| " ** Fatal ** GetWaterThermalTankInput: Errors found in processing Water Thermal Tank input.", | ||
| " ...Summary of Errors that led to program termination:", | ||
| " ..... Reference severe error count=2", | ||
| " ..... Last severe error=WaterHeater:HeatPump:PumpedCondenser=\"ZONE4HEATPUMPWATERHEATER\":", | ||
| }); |
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.
HPWHOutdoorAirMissingNodeNameWarning: change to capture the FatalError
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 is fine, although in the future if you want to just match some small portion of it and not worry about the ERR formatting, you can use compare_err_stream_substring... :)
| " OutdoorAirOnly, !- Inlet Air Configuration", | ||
| " , !- Air Inlet Node Name", | ||
| " , !- Air Outlet Node Name", | ||
| " HPWHPumped Air Inlet Node, !- Outdoor Air Node Name", | ||
| " HPWHPumped Air Outlet Node,!- Exhaust Air Node Name", |
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.
HPWH_Both_Pumped_and_Wrapped_InputProcessing: change to Outdoors so it doesn't fatal due to missing zone
Initial err stream
+ ** Severe ** WaterHeater:HeatPump:PumpedCondenser=\"HPWHPUMPED\", not found
+ ** ~~~ ** Inlet Air Zone Name=\"ZONE1\".
+ ** Severe ** WaterHeater:HeatPump:WrappedCondenser=\"HPWHWRAPPED\", not found
+ ** ~~~ ** Inlet Air Zone Name=\"ZONE1\".
+ ** Severe ** WaterHeater:Mixed = HPWHPUMPED MIXED TANK: Ambient Temperature Zone not found = ZONE1
+ ** Severe ** WaterHeater:Stratified = HPWHWRAPPED STRATIFIED TANK: Ambient Temperature Zone not found = ZONE1
+ ** Severe ** WaterHeater:HeatPump:PumpedCondenser = HPWHPUMPED:
+ ** ~~~ ** ZoneHVAC:EquipmentList and ZoneHVAC:EquipmentConnections objects are required when Inlet Air Configuration is either ZoneAirOnly or ZoneAndOutdoorAir.
+ ** Severe ** WaterHeater:HeatPump:WrappedCondenser = HPWHWRAPPED:
+ ** ~~~ ** ZoneHVAC:EquipmentList and ZoneHVAC:EquipmentConnections objects are required when Inlet Air Configuration is either ZoneAirOnly or ZoneAndOutdoorAir.
+ ** Fatal ** GetWaterThermalTankInput: Errors found in processing Water Thermal Tank input
rraustad
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.
Good to go.
Myoldmopar
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.
Nice.
| state->dataEnvrn->StdRhoAir = 1.0; | ||
|
|
||
| ASSERT_FALSE(WaterThermalTanks::GetWaterThermalTankInput(*state)); | ||
| WaterThermalTanks::GetWaterThermalTankInput(*state); |
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, let it fail if it's gonna fail.
| EXPECT_THROW(WaterThermalTanks::GetWaterThermalTankInput(*state), EnergyPlus::FatalError); | ||
|
|
||
| std::string const error_string = | ||
| delimited_string({" ** Warning ** ProcessScheduleInput: Schedule:Constant = DUMMYSCH", | ||
| " ** ~~~ ** Schedule Type Limits Name is empty.", | ||
| " ** ~~~ ** Schedule will not be validated.", | ||
| " ** Severe ** WaterHeater:HeatPump:PumpedCondenser=\"ZONE4HEATPUMPWATERHEATER\":", | ||
| " ** ~~~ ** When Inlet Air Configuration=\"OUTDOORAIRONLY\".", | ||
| " ** ~~~ ** Outdoor Air Node Name and Exhaust Air Node Name must be specified.", | ||
| " ** Severe ** WaterHeater:HeatPump:PumpedCondenser=\"ZONE4HEATPUMPWATERHEATER\":", | ||
| " ** ~~~ ** Heat pump water heater fan outlet node name does not match next connected component.", | ||
| " ** ~~~ ** Fan outlet node name = ZONE4AIRINLETNODE"}); | ||
| std::string const error_string = delimited_string({ | ||
| " ** Warning ** ProcessScheduleInput: Schedule:Constant = DUMMYSCH", | ||
| " ** ~~~ ** Schedule Type Limits Name is empty.", | ||
| " ** ~~~ ** Schedule will not be validated.", | ||
| " ** Severe ** WaterHeater:HeatPump:PumpedCondenser=\"ZONE4HEATPUMPWATERHEATER\":", | ||
| " ** ~~~ ** When Inlet Air Configuration=\"OUTDOORAIRONLY\".", | ||
| " ** ~~~ ** Outdoor Air Node Name and Exhaust Air Node Name must be specified.", | ||
| " ** Severe ** WaterHeater:HeatPump:PumpedCondenser=\"ZONE4HEATPUMPWATERHEATER\":", | ||
| " ** ~~~ ** Heat pump water heater fan outlet node name does not match next connected component.", | ||
| " ** ~~~ ** Fan outlet node name = ZONE4AIRINLETNODE", | ||
| " ** Fatal ** GetWaterThermalTankInput: Errors found in processing Water Thermal Tank input.", | ||
| " ...Summary of Errors that led to program termination:", | ||
| " ..... Reference severe error count=2", | ||
| " ..... Last severe error=WaterHeater:HeatPump:PumpedCondenser=\"ZONE4HEATPUMPWATERHEATER\":", | ||
| }); |
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 is fine, although in the future if you want to just match some small portion of it and not worry about the ERR formatting, you can use compare_err_stream_substring... :)
|
All happy here with develop pulled in. Thanks @jmarrec |
Pull request overview
Description of the purpose of this PR
The defect file in DevSupport segfaults before fix. After fix it fatals out as expected.
Pull Request Author
Reviewer