Skip to content
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

EnergyPlus Crash Due to Zero Input for Variable Speed Coil Total Cooling #10470

Merged
merged 9 commits into from
Apr 24, 2024
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -3295,7 +3295,9 @@ \subsubsection{Inputs}\label{inputs-17-001}

\paragraph{Field: Gross Rated Total Cooling Capacity at Selected Nominal Speed Level}\label{field-gross-rated-total-cooling-capacity-at-selected-nominal-speed-level}

This numeric field contains the gross rated total cooling capacity at the nominal speed level. This field is autosizable. The gross rated total cooling capacity is used to determine a capacity scaling factor, as compared to the Reference Unit capacity at the nominal speed level.
This numeric field contains the gross rated total cooling capacity at the nominal speed level. This field is autosizable. Note that if this field is set to zero rather than a positive number or autosized, the coil will be ignored and will not run.

The gross rated total cooling capacity is used to determine a capacity scaling factor, as compared to the Reference Unit capacity at the nominal speed level.

\begin{equation}
{\rm{CapacityScaleFactor}} = \frac{{{\rm{Gross RatedTotalCoolingCapacity}}}}{{{\rm{ReferenceUnitCapacity}}@{\rm{NominalSpeedLevel}}}}
Expand Down Expand Up @@ -7521,7 +7523,9 @@ \subsubsection{Inputs}\label{inputs-31}

\paragraph{Field: Gross Rated Total Cooling Capacity at Selected Nominal Speed Level}\label{field-gross-rated-total-cooling-capacity-at-selected-nominal-speed-level-1}

This numeric field contains the gross rated total cooling capacity at the nominal speed level. This field is autosizable. The gross rated total cooling capacity is used to determine a capacity scaling factor, as compared to the Reference Unit capacity at the nominal speed level.
This numeric field contains the gross rated total cooling capacity at the nominal speed level. This field is autosizable. Note that if this field is set to zero rather than a positive number or autosized, the coil will be ignored and will not run.

The gross rated total cooling capacity is used to determine a capacity scaling factor, as compared to the Reference Unit capacity at the nominal speed level.

\begin{equation}
CapacityScaleFactor = \frac{{GrossRatedTotalCoolingCapacity}}{{ReferenceUnitCapacity@NominalSpeedLevel}}
Expand Down
10 changes: 7 additions & 3 deletions src/EnergyPlus/VariableSpeedCoils.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3987,7 +3987,10 @@ namespace VariableSpeedCoils {
!state.dataVariableSpeedCoils->MyPlantScanFlag(DXCoilNum)) {
// for each furnace, do the sizing once.
ErrorsFound = false;
SizeVarSpeedCoil(state, DXCoilNum, ErrorsFound);
if (state.dataVariableSpeedCoils->VarSpeedCoil(DXCoilNum).RatedCapCoolTotal > 0.0 ||
state.dataVariableSpeedCoils->VarSpeedCoil(DXCoilNum).RatedCapCoolTotal == AutoSize) {
SizeVarSpeedCoil(state, DXCoilNum, ErrorsFound);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So we won't report to the eio now? Shouldn't we show the user that they have 0 capacity? I think maybe protect the call to CalcCBF would allow sizing to proceed? or add a return in CalcCBF if capacity = 0 to protect all coil models?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rraustad I'll look at modifying that and see what I can come up with. I think a return at the top of CalcCBF would be a reasonable approach.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rraustad Ok, I reinstated the sizing routine...but it wasn't as simple as skipping the CalcCBF routine. I had to skip a couple of other things as well to avoid other issues. I just made another commit that fixes all that and produced the same results as before. Maybe the slight/couple issues in ci will also be taken care of by this iteration?

if (ErrorsFound) {
ShowFatalError(state, format("{}: Failed to size variable speed coil.", RoutineName));
}
Expand Down Expand Up @@ -5974,6 +5977,7 @@ namespace VariableSpeedCoils {
state.dataVariableSpeedCoils->CpAir_Init = PsyCpAirFnW(state.dataVariableSpeedCoils->LoadSideInletHumRat_Init);
state.dataVariableSpeedCoils->firstTime = false;
}

state.dataVariableSpeedCoils->LoadSideInletWBTemp_Init = PsyTwbFnTdbWPb(state,
state.dataVariableSpeedCoils->LoadSideInletDBTemp_Init,
state.dataVariableSpeedCoils->LoadSideInletHumRat_Init,
Expand Down Expand Up @@ -6105,7 +6109,7 @@ namespace VariableSpeedCoils {
state.dataVariableSpeedCoils->VarSpeedCoil(DXCoilNum).SimFlag = true;
}

if (CompressorOp == CompressorOperation::Off) {
if (CompressorOp == CompressorOperation::Off || state.dataVariableSpeedCoils->VarSpeedCoil(DXCoilNum).RatedCapCoolTotal <= 0.0) {
state.dataVariableSpeedCoils->VarSpeedCoil(DXCoilNum).SimFlag = false;
return;
}
Expand Down Expand Up @@ -7386,7 +7390,7 @@ namespace VariableSpeedCoils {
return;
}

if (CompressorOp == CompressorOperation::Off) {
if (CompressorOp == CompressorOperation::Off || state.dataVariableSpeedCoils->VarSpeedCoil(DXCoilNum).RatedCapCoolTotal <= 0.0) {
state.dataVariableSpeedCoils->VarSpeedCoil(DXCoilNum).SimFlag = false;
return;
}
Expand Down
5 changes: 3 additions & 2 deletions src/EnergyPlus/VariableSpeedCoils.hh
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,7 @@ namespace VariableSpeedCoils {
int WaterInletNodeNum; // Node Number of the Water Onlet
int WaterOutletNodeNum; // Node Number of the Water Outlet
PlantLocation plantLoc;
bool coilOperationFlag; // Set to false when the RatedCapCoolTotal from user is less than or equal to zero but not AutoSize
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not needed now?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch--forgot to get rid of that. And the old unit test which sent everything red. New commit just made to get rid of this and the old unit test.

// set by parent object and "pushed" to this structure in SetVSWSHPData subroutine
bool FindCompanionUpStreamCoil; // Flag to get the companion coil in Init
bool IsDXCoilInZone; // true means dx coil is in zone instead of outside
Expand Down Expand Up @@ -291,8 +292,8 @@ namespace VariableSpeedCoils {
OutletWaterEnthalpy(0.0), Power(0.0), QLoadTotal(0.0), QSensible(0.0), QLatent(0.0), QSource(0.0), QWasteHeat(0.0), Energy(0.0),
EnergyLoadTotal(0.0), EnergySensible(0.0), EnergyLatent(0.0), EnergySource(0.0), COP(0.0), RunFrac(0.0), PartLoadRatio(0.0),
RatedPowerHeat(0.0), RatedCOPHeat(0.0), RatedCapCoolSens(0.0), RatedPowerCool(0.0), RatedCOPCool(0.0), AirInletNodeNum(0),
AirOutletNodeNum(0), WaterInletNodeNum(0), WaterOutletNodeNum(0), plantLoc{}, FindCompanionUpStreamCoil(true), IsDXCoilInZone(false),
CompanionCoolingCoilNum(0), CompanionHeatingCoilNum(0), FanDelayTime(0.0),
AirOutletNodeNum(0), WaterInletNodeNum(0), WaterOutletNodeNum(0), plantLoc{}, coilOperationFlag(true), FindCompanionUpStreamCoil(true),
IsDXCoilInZone(false), CompanionCoolingCoilNum(0), CompanionHeatingCoilNum(0), FanDelayTime(0.0),
// This one calls into a std::vector, so it's 0-indexed, so we initialize it to -1
MSHPDesignSpecIndex(-1), MSErrIndex(DataHVACGlobals::MaxSpeedLevels, 0), MSRatedPercentTotCap(DataHVACGlobals::MaxSpeedLevels, 0.0),
MSRatedTotCap(DataHVACGlobals::MaxSpeedLevels, 0.0), MSRatedSHR(DataHVACGlobals::MaxSpeedLevels, 0.0),
Expand Down
26 changes: 26 additions & 0 deletions tst/EnergyPlus/unit/VariableSpeedCoils.unit.cc
Original file line number Diff line number Diff line change
Expand Up @@ -6963,4 +6963,30 @@ TEST_F(EnergyPlusFixture, VariableSpeedCoils_Coil_Defrost_Power_Fix_Test)
EXPECT_NEAR(state->dataVariableSpeedCoils->VarSpeedCoil(DXCoilNum).DefrostPower, 0.0, 1e-3);
}

TEST_F(EnergyPlusFixture, setVarSpeedCoilOperationFlagTest)
{
Real64 userVarSpeedCoilCoolCap;
bool functionResult;

// Case 1: coil capacity is positive-->result is true
userVarSpeedCoilCoolCap = 12345.6789;
functionResult = VariableSpeedCoils::setVarSpeedCoilOperationFlag(userVarSpeedCoilCoolCap);
EXPECT_TRUE(functionResult);

// Case 2: coil capacity is autosized-->result is true
userVarSpeedCoilCoolCap = DataSizing::AutoSize;
functionResult = VariableSpeedCoils::setVarSpeedCoilOperationFlag(userVarSpeedCoilCoolCap);
EXPECT_TRUE(functionResult);

// Case 3: coil capacity is zero-->result is false
userVarSpeedCoilCoolCap = 0.0;
functionResult = VariableSpeedCoils::setVarSpeedCoilOperationFlag(userVarSpeedCoilCoolCap);
EXPECT_FALSE(functionResult);

// Case 4: coil capacity is negative but not AutoSize-->result is false
userVarSpeedCoilCoolCap = -123.456;
functionResult = VariableSpeedCoils::setVarSpeedCoilOperationFlag(userVarSpeedCoilCoolCap);
EXPECT_FALSE(functionResult);
}

} // namespace EnergyPlus
Loading