-
Notifications
You must be signed in to change notification settings - Fork 392
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
Allow Coil:WaterHeating:Desuperheater to use Coil:Cooling:DX #9051
Allow Coil:WaterHeating:Desuperheater to use Coil:Cooling:DX #9051
Conversation
…ater desuperheater, still with results diffs
src/EnergyPlus/HeatingCoils.cc
Outdated
// get RTF and NominalCapacity from Coil:CoolingDX | ||
{ | ||
auto &thisCoolingCoil = state.dataCoilCooingDX->coilCoolingDXs[SourceID]; | ||
HeatingCoil(CoilNum).RTF = thisCoolingCoil.runTimeFraction; |
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.
I see this is how is has been done in the past, but this should be PLR since PLR relates to runtime and RTF relates to power. This is a follow up issue.
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.
Since this->reclaimHeat.AvailCapacity = this->totalCoolingEnergyRate + this->elecCoolingPower, which is air-side energy (PLR) + electricity (RTF), there is a bit of a problem with the portion of the time step each of these represent. Again, for another time.
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.
Now I think RTF is correct in this context.
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.
Ready for review.
src/EnergyPlus/HeatingCoils.cc
Outdated
} else if (UtilityRoutines::SameString(Alphas(5), "Coil:Cooling:DX")) { | ||
HeatingCoil(CoilNum).ReclaimHeatingSource = HeatObjTypes::COIL_COOLING_DX_NEW; | ||
HeatingCoil(CoilNum).ReclaimHeatingSourceIndexNum = CoilCoolingDX::factory(state, Alphas(6)); | ||
if (HeatingCoil(CoilNum).ReclaimHeatingSourceIndexNum < 0) { | ||
ShowSevereError( | ||
state, | ||
format( | ||
"{}={}, could not find desuperheater coil {}={}", CurrentModuleObject, HeatingCoil(CoilNum).Name, Alphas(5), Alphas(6))); | ||
state.dataHeatingCoils->InputErrorsFound = true; | ||
} | ||
DataHeatBalance::HeatReclaimDataBase &HeatReclaim = | ||
state.dataCoilCooingDX->coilCoolingDXs[HeatingCoil(CoilNum).ReclaimHeatingSourceIndexNum].reclaimHeat; | ||
if (!allocated(HeatReclaim.HVACDesuperheaterReclaimedHeat)) { | ||
HeatReclaim.HVACDesuperheaterReclaimedHeat.allocate(state.dataHeatingCoils->NumDesuperheaterCoil); | ||
for (auto &num : HeatReclaim.HVACDesuperheaterReclaimedHeat) | ||
num = 0.0; | ||
} | ||
HeatReclaim.ReclaimEfficiencyTotal += HeatingCoil(CoilNum).Efficiency; | ||
if (HeatReclaim.ReclaimEfficiencyTotal > 0.3) { | ||
ShowSevereError(state, | ||
cAllCoilTypes(HeatingCoil(CoilNum).HCoilType_Num) + ", \"" + HeatingCoil(CoilNum).Name + | ||
"\" sum of heat reclaim recovery efficiencies from the same source coil: \"" + | ||
HeatingCoil(CoilNum).ReclaimHeatingCoilName + "\" cannot be over 0.3"); | ||
} | ||
state.dataHeatingCoils->ValidSourceType(CoilNum) = true; |
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 get input support for the new coil in Coil:WaterHeating:DesuperHeater. This is an upstream contribution from your branch @Myoldmopar.
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 looks correct.
src/EnergyPlus/HeatingCoils.cc
Outdated
} else if (HeatingCoil(CoilNum).ReclaimHeatingSource == HeatObjTypes::COIL_COOLING_DX_NEW) { | ||
DataHeatBalance::HeatReclaimDataBase &HeatReclaim = | ||
state.dataCoilCooingDX->coilCoolingDXs[HeatingCoil(CoilNum).ReclaimHeatingSourceIndexNum].reclaimHeat; | ||
if (!allocated(HeatReclaim.HVACDesuperheaterReclaimedHeat)) { | ||
HeatReclaim.HVACDesuperheaterReclaimedHeat.allocate(state.dataHeatingCoils->NumDesuperheaterCoil); | ||
for (auto &num : HeatReclaim.HVACDesuperheaterReclaimedHeat) | ||
num = 0.0; | ||
HeatReclaim.ReclaimEfficiencyTotal += HeatingCoil(CoilNum).Efficiency; | ||
if (HeatReclaim.ReclaimEfficiencyTotal > 0.3) { | ||
ShowSevereError(state, | ||
cAllCoilTypes(HeatingCoil(CoilNum).HCoilType_Num) + ", \"" + HeatingCoil(CoilNum).Name + | ||
"\" sum of heat reclaim recovery efficiencies from the same source coil: \"" + | ||
HeatingCoil(CoilNum).ReclaimHeatingCoilName + "\" cannot be over 0.3"); | ||
} | ||
} | ||
state.dataHeatingCoils->ValidSourceType(CoilNum) = true; |
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.
Additional upstream contribution from @Myoldmopar
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 also looks correct. I see that HVACDesuperheaterReclaimedHeat is allocated to the total number of desuperheater coils even if this source object only has 1 desuperheater coil attached. Not at all sure if that is necessary or how this array is accessed but just noting it here.
src/EnergyPlus/WaterThermalTanks.cc
Outdated
auto &DesupHtr = state.dataWaterThermalTanks->WaterHeaterDesuperheater(DesuperheaterNum); | ||
|
||
DesupHtr.Name = state.dataIPShortCut->cAlphaArgs(1); | ||
DesupHtr.Type = state.dataIPShortCut->cCurrentModuleObject; |
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 local reference to state.dataWaterThermalTanks->WaterHeaterDesuperheater(DesuperheaterNum)
throughout for readability.
src/EnergyPlus/WaterThermalTanks.cc
Outdated
DesupHtr.HeatingSourceName + "\" cannot be over 0.3"); | ||
ErrorsFound = true; | ||
} | ||
} | ||
} else if (UtilityRoutines::SameString(heatSourceObjType, "Coil:Cooling:DX")) { | ||
DesupHtr.ReclaimHeatingSource = CoilObjEnum::CoilCoolingDX; | ||
DesupHtr.ReclaimHeatingSourceIndexNum = CoilCoolingDX::factory(state, state.dataIPShortCut->cAlphaArgs(10)); | ||
if (DesupHtr.ReclaimHeatingSourceIndexNum < 0) { | ||
ShowSevereError(state, | ||
format("{}={}, could not find desuperheater coil {}={}", | ||
state.dataIPShortCut->cCurrentModuleObject, | ||
DesupHtr.Name, | ||
state.dataIPShortCut->cAlphaArgs(9), | ||
state.dataIPShortCut->cAlphaArgs(10))); | ||
ErrorsFound = true; | ||
} else { | ||
DataHeatBalance::HeatReclaimDataBase &HeatReclaim = | ||
state.dataCoilCooingDX->coilCoolingDXs[DesupHtr.ReclaimHeatingSourceIndexNum].reclaimHeat; | ||
if (!allocated(HeatReclaim.WaterHeatingDesuperheaterReclaimedHeat)) { | ||
HeatReclaim.WaterHeatingDesuperheaterReclaimedHeat.allocate(state.dataWaterThermalTanks->numWaterHeaterDesuperheater); | ||
for (auto &num : HeatReclaim.WaterHeatingDesuperheaterReclaimedHeat) | ||
num = 0.0; | ||
} | ||
DesupHtr.ValidSourceType = true; | ||
HeatReclaim.ReclaimEfficiencyTotal += DesupHtr.HeatReclaimRecoveryEff; | ||
if (HeatReclaim.ReclaimEfficiencyTotal > 0.3) { | ||
ShowSevereError(state, | ||
state.dataIPShortCut->cCurrentModuleObject + ", \"" + DesupHtr.Name + | ||
"\" sum of heat reclaim recovery efficiencies from the same source coil: \"" + DesupHtr.HeatingSourceName + |
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.
Get input for new coil support in the water heater desuperheater.
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.
Also looks correct.
src/EnergyPlus/WaterThermalTanks.cc
Outdated
} else if (DesupHtr.ReclaimHeatingSource == CoilObjEnum::CoilCoolingDX) { | ||
AverageWasteHeat = state.dataCoilCooingDX->coilCoolingDXs[DesupHtr.ReclaimHeatingSourceIndexNum].reclaimHeat.AvailCapacity - | ||
state.dataCoilCooingDX->coilCoolingDXs[DesupHtr.ReclaimHeatingSourceIndexNum].reclaimHeat.HVACDesuperheaterReclaimedHeatTotal; | ||
DesupHtr.DXSysPLR = state.dataCoilCooingDX->coilCoolingDXs[DesupHtr.ReclaimHeatingSourceIndexNum].partLoadRatioReport; // check this |
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.
Extending IF/ELSE ladder to support new coil object.
src/EnergyPlus/WaterThermalTanks.cc
Outdated
} else if (DesupHtr.ReclaimHeatingSource == CoilObjEnum::CoilCoolingDX) { | ||
state.dataCoilCooingDX->coilCoolingDXs[DesupHtr.ReclaimHeatingSourceIndexNum].reclaimHeat.WaterHeatingDesuperheaterReclaimedHeat(DesuperheaterNum) = DesupHtr.HeaterRate; | ||
state.dataCoilCooingDX->coilCoolingDXs[DesupHtr.ReclaimHeatingSourceIndexNum].reclaimHeat.WaterHeatingDesuperheaterReclaimedHeatTotal = 0.0; | ||
for (auto &num :state.dataCoilCooingDX->coilCoolingDXs[DesupHtr.ReclaimHeatingSourceIndexNum].reclaimHeat.WaterHeatingDesuperheaterReclaimedHeat) | ||
state.dataCoilCooingDX->coilCoolingDXs[DesupHtr.ReclaimHeatingSourceIndexNum].reclaimHeat.WaterHeatingDesuperheaterReclaimedHeatTotal += num; |
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.
Another IF/ELSE case to handle switching on the new coil.
@@ -0,0 +1,1639 @@ | |||
!-Generator IDFEditor 1.34 |
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 file takes the original CoileWaterDesuperheating
file and converts it to use the unitary system with a single-speed coil. We may not want to keep this extra file around, but this is the baseline I've been comparing against.
@@ -0,0 +1,1690 @@ | |||
!-Generator IDFEditor 1.34 |
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 file converts CoilWaterDesuperheatingUnitary
from using the old single-speed coil to using the new Coil:Cooling:DX
coil. This is what I've been comparing against the baseline.
} else if (DesupHtr.ReclaimHeatingSource == CoilObjEnum::CoilCoolingDX) { | ||
AverageWasteHeat = | ||
state.dataCoilCooingDX->coilCoolingDXs[DesupHtr.ReclaimHeatingSourceIndexNum].reclaimHeat.AvailCapacity - | ||
state.dataCoilCooingDX->coilCoolingDXs[DesupHtr.ReclaimHeatingSourceIndexNum].reclaimHeat.HVACDesuperheaterReclaimedHeatTotal; |
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 equation keeps confusing me. The *Total is the sum of all reclaimed heat so far? and AvailableCapacity is the max available? and the AverageWasteHeat is the difference. Just hard to wrap your head around this without seeing where HVACDesuperheaterReclaimedHeatTotal is set, or accumulated, or decremented, etc. in code. I just don't see that. But it's the same as everywhere else so this is great.
for (auto &num : state.dataHeatBal->HeatReclaimDXCoil(SourceID).HVACDesuperheaterReclaimedHeat)
state.dataHeatBal->HeatReclaimDXCoil(SourceID).HVACDesuperheaterReclaimedHeatTotal += num;
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.
So I left a line out. Zero out the var and then sum it.
state.dataHeatBal->HeatReclaimDXCoil(SourceID).HVACDesuperheaterReclaimedHeatTotal = 0.0;
for (auto &num : state.dataHeatBal->HeatReclaimDXCoil(SourceID).HVACDesuperheaterReclaimedHeat)
state.dataHeatBal->HeatReclaimDXCoil(SourceID).HVACDesuperheaterReclaimedHeatTotal += num;
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.
The only thing that worries me here is timing of when this happens versus when HVACDesuperheaterReclaimedHeat is set. Good for now, I guess.
AverageWasteHeat = | ||
state.dataCoilCooingDX->coilCoolingDXs[DesupHtr.ReclaimHeatingSourceIndexNum].reclaimHeat.AvailCapacity - | ||
state.dataCoilCooingDX->coilCoolingDXs[DesupHtr.ReclaimHeatingSourceIndexNum].reclaimHeat.HVACDesuperheaterReclaimedHeatTotal; | ||
DesupHtr.DXSysPLR = state.dataCoilCooingDX->coilCoolingDXs[DesupHtr.ReclaimHeatingSourceIndexNum].partLoadRatioReport; |
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.
OK, so this desuperheater tracks the PLR of the source. But what if this desuperheater is used in a location that has a PLR different from the source? Again, this is what is done everywhere else, I am just ensuring lack of sleep.
@mitchute @Myoldmopar it has been 28 days since this pull request was last updated. |
…addNewCoilToWaterTankDesuperheater
@mitchute @Myoldmopar it has been 28 days since this pull request was last updated. |
3 similar comments
@mitchute @Myoldmopar it has been 28 days since this pull request was last updated. |
@mitchute @Myoldmopar it has been 28 days since this pull request was last updated. |
@mitchute @Myoldmopar it has been 28 days since this pull request was last updated. |
I'll try to pull in develop here and see how things look with the density differences and misc things that @rraustad has resolved. |
@mitchute @Myoldmopar it has been 28 days since this pull request was last updated. |
Pulled upstream into this branch for kicks. Will be interesting to see how it looks with all the cleanups since the last run. |
Alright, Mac shows one regression, which is pretty expected. CI still has a few runs to make here, but it should be done today. I think the next step will be making sure we document the diffs that occur and that should get this on the glide path toward merge. |
Diff summary (I'll edit this as I look through them) ERR ** ~~~ ** or the requested variable is an advanced output which requires Output : Diagnostics, DisplayAdvancedReportVariables;
- ************* Key=*, VarName=COOLING COIL BASIN HEATER ELECTRICITY RATE, Frequency=Hourly
- ************* Key=*, VarName=COOLING COIL BASIN HEATER ELECTRICITY ENERGY, Frequency=Hourly
+ ************* Key=*, VarName=COOLING COIL SOURCE SIDE HEAT TRANSFER RATE, Frequency=Hourly
************* Key=*, VarName=UNITARY SYSTEM CYCLING RATIO, Frequency=Hourly This is showing that the basin heater report variables are no longer being reported as invalid. This is a good thing I think, indicating a report variable is no longer missing. However, there is also a new missing varibale for the cooling coil source side heat transfer rate. Is this because the report variable name changed? EIOThere are lots of small warmup convergence differences. Interestingly in the new branch, the convergence just seems a little tighter, which is causing some scientific notation to appear, causing the lines to look longer. Anyway, that isn't a concern. There are component sizing differences though. I took off the - COIL:COOLING:DX:VARIABLESPEED, MAIN COOLING COIL 1, Design Size Rated Total Cooling Capacity [W], 53882.19699
- COIL:COOLING:DX:VARIABLESPEED, MAIN COOLING COIL 1, Design Size Rated Air Flow Rate [m3/s], 5.50017
- COIL:COOLING:DX:VARIABLESPEED, MAIN COOLING COIL 1, Design Size Rated Sensible Cooling Capacity [W], 41169.34265
+ Coil:Cooling:DX:CurveFit:OperatingMode, MAIN COOLING COIL 1 OPERATING MODE, Design Size Rated Evaporator Air Flow Rate [m3/s], 2.62064
+ Coil:Cooling:DX:CurveFit:OperatingMode, MAIN COOLING COIL 1 OPERATING MODE, Design Size Rated Gross Total Cooling Capacity [W], 53882.19699
+ Coil:Cooling:DX:CurveFit:OperatingMode, MAIN COOLING COIL 1 OPERATING MODE, Design Size Rated Condenser Air Flow Rate [m3/s], 6.14257
+ Coil:Cooling:DX:CurveFit:Speed, MAIN COOLING COIL 1 SPEED, User-Specified Rated Air Flow Rate [m3/s], 2.62064
+ Coil:Cooling:DX:CurveFit:Speed, MAIN COOLING COIL 1 SPEED, User-Specified Gross Cooling Capacity [W], 53882.19699
+ Coil:Cooling:DX:CurveFit:Speed, MAIN COOLING COIL 1 SPEED, User-Specified Gross Sensible Heat Ratio, 0.75000
AirLoopHVAC:UnitarySystem, GASHEAT DXAC FURNACE 1, Design Size Nominal Cooling Capacity [W], 53882.19699
- AirLoopHVAC:UnitarySystem, GASHEAT DXAC FURNACE 1, Design Size Nominal Heating Capacity [W], 53882.19699
+ AirLoopHVAC:UnitarySystem, GASHEAT DXAC FURNACE 1, Design Size Nominal Heating Capacity [W], 28151.00449
AirLoopHVAC:UnitarySystem, GASHEAT DXAC FURNACE 1, Design Size Maximum Supply Air Temperature [C], 18.00000
AirLoopHVAC:UnitarySystem, GASHEAT DXAC FURNACE 1, User-Specified Maximum Supply Air Temperature [C], 80.00000
AirLoopHVAC:UnitarySystem, GASHEAT DXAC FURNACE 1, User-Specified Supplemental Heating Coil Nominal Capacity [W], 0.00000
Fan:VariableVolume, SUPPLY FAN 1, Design Size Maximum Flow Rate [m3/s], 2.62064
- Coil:Heating:Fuel, MAIN HEATING COIL 1, Design Size Nominal Capacity [W], 53882.19699
+ Coil:Heating:Fuel, MAIN HEATING COIL 1, Design Size Nominal Capacity [W], 28151.00449 The lines are changing both because of the new coil type names but also the sizing is way different. I'm not saying wrong with the new branch, just way different. So this needs to get discussed (if it hasn't already). |
The VS and new coil models are different beasts. I have been wondering how they would size and perform. VS coil back-calculates air flow so that would change. But capacity should still be similar to before (zone air flow * sizing data). VS coil sizing calcs are done in the VS coil model, new coil model uses sizing routines. |
Oh, I see. The gas heating coil may size the same as the other coils in the VS model sizing routine. That may be the difference in coil size. |
@rraustad what would you suggest for moving this forward with such a large diff. I don't want to bog this down unnecessarily, but is there something we can modify in an example file to get a similar value as this, to help us feel comfortable about the change? |
Just to show you where that heating coil capacity comes from, it's the raw heating capacity before the parent tries to set coil loads. The UnitarySystem knows that a DX cooling coil and a gas heating coil is not a heat pump, so lets them size individually. The VS coil model just sets the heating coil size equal to the cooling coil size as if the heating coil were always going to be a VS heating coil. So the VS coil model does not have the logic to discern between a HP and non-HP system. In this case the coil model is setting a flag to size the heating coil, not the parent. If I make 1 little change I get this:
With this change the coil capacity would be the same but the air flow will still show a difference. I was thinking that to allow the new coil model the size the same as the VS coil model we could add a new object that has the scaling ratios that the VS coil model has. I know Trane uses the VS coil model exclusively so to remove that model would create a problem with a 3rd party. |
Blocker #9576 should correct heating coil capacity to match. There will still be an issue with air flow difference. |
Similar change in #9250 |
…NewCoilToWaterTankDesuperheater
I feel like this is ready now. The coil capacity size looks perfect now, though the flow rate is different, as expected. I don't think we need to spend time trying to reconcile this difference, and move forward with the new coil. I'll leave this open for discussion for a little while but if no one is interested in discussion, I'll merge this one in shortly. Thanks for the capacity solution @rraustad, that's great! |
Pull request overview
Pull Request Author
Add to this list or remove from it as applicable. This is a simple templated set of guidelines.
Reviewer
This will not be exhaustively relevant to every PR.