-
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
AirloopHVAC:UnitarySystem uses outdoor air temperature to size water source dx cooling coils and incorrectly reports autosized supplemental heating coil capacity for CoolReheat control #9501
Conversation
This branch is a pre-cursor to other maintenance changes that require uniformity in component sizing. Some zone coil models use the zone return air temperature for coil sizing, this is not accurate since the actual zone temperature is what enters the component because the zone models draw air directly from the zone. Additionally, a new method to determine the variable-speed water source coil source temperature (i.e., outdoor temperature or water entering temperature) was implemented that is included here. |
@@ -140,8 +140,7 @@ Real64 CoolingCapacitySizer::size(EnergyPlusData &state, Real64 _originalValue, | |||
CoilInTemp = this->finalZoneSizing(this->curZoneEqNum).DesCoolCoilInTemp; | |||
CoilInHumRat = this->finalZoneSizing(this->curZoneEqNum).DesCoolCoilInHumRat; | |||
} else { | |||
CoilInTemp = this->finalZoneSizing(this->curZoneEqNum) | |||
.ZoneRetTempAtCoolPeak; // Question whether zone equipment should use return temp for sizing | |||
CoilInTemp = this->finalZoneSizing(this->curZoneEqNum).ZoneTempAtCoolPeak; |
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
@@ -167,6 +166,7 @@ Real64 CoolingCapacitySizer::size(EnergyPlusData &state, Real64 _originalValue, | |||
if (DDNum > 0 && TimeStepNumAtMax > 0) { | |||
OutTemp = state.dataSize->DesDayWeath(DDNum).Temp(TimeStepNumAtMax); | |||
} | |||
if (this->dataCoolCoilType > -1) OutTemp = GetCoilSourceTempUsedForSizing(this->dataCoolCoilType, OutTemp); |
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.
Protected call to new function that mimics GetVSCoilRatedSourceTemp added 7 days ago. If a parent object (or unit test) does not set the coil type, notihng happens, and OutTemp remains unchanged.
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.
OutTemp is only used in the CapFT equation at line 189 below.
TotCapTempModFac = CurveManager::CurveValue(state, this->dataTotCapCurveIndex, CoilInWetBulb, OutTemp);
@@ -1534,6 +1534,8 @@ namespace UnitarySystems { | |||
} else { | |||
state.dataHVACGlobal->DXCT = 1; // uses normal DX coil flow limits | |||
} | |||
// sizing may need to know what type of coil is being sized | |||
state.dataSize->DataCoolCoilType = this->m_CoolingCoilType_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.
UnitarySystem sets this to mimic the sizing of the VS coil model. Currently only cooling is accounted for.
src/EnergyPlus/UnitarySystem.cc
Outdated
@@ -15211,6 +15213,9 @@ namespace UnitarySystems { | |||
} else { | |||
this->m_FirstPass = false; | |||
} | |||
// reset the global system type flags | |||
state.dataSize->ZoneEqDXCoil = false; | |||
// state.dataSize->ZoneEqFanCoil = false; // not used yet |
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.
These should have been set to false, as is normally done in parent objects.
} | ||
return sourceTemp; | ||
} | ||
|
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 new sizing member function mimics the VS coil model function used to set the coil source temperature (e.g., OAT or entering water T). That function will need to be modified in the near future. This function sets up a method for a parent object (i.e., UnitarySystem) to gather the same information used for sizing calculations. There will obviously be more changes here as the VS coil model sizing calculations evolve.
Diffs in DOAToPTAC_DrawReturnAir.
|
Are you sure this is not stepping on the changes to let zone equipment draw from a plenum? See #8719 |
I'll hold from diving in until @mjwitte's comment is addressed. |
This comment was marked as off-topic.
This comment was marked as off-topic.
I'll put this change to which coil inlet temp to use in a different branch. That leaves only the new VS coil function in this branch. |
I'm sure now |
@rraustad Is there a defect file for this 9497? |
Yes, and no. This change is to correct the last eio diff in #9273. ZoneVSWSHP_wDOAS uses ZoneHVAC:WaterToAirHeatPump (PTUnit). I guess I could try to create one using UnitarySystem if it were necessary. When I merge this in with 9273 the diff should be gone. You want an example file to see the coil size change? |
No, you can't. You'll only see the change when UnitarySystem manages the PTUnit. I am making a defect file now. |
Are you going to merge this branch into 9273 locally to see the change? |
Now I'm more confused. I thought that's what 9273 did (switch to UnitarySystem running the PTUnit).
I wasn't planning to. I'm still misunderstanding the connections here. |
Oh, I see. Running ZoneVSWSHP_wDOAS doesn't exercise the changed code unless it's merged with 9273 so that it's actually running UnitarySystem. If you've made a Unitary idf version of this, I'll use that. Otherwise I may make one here. |
I see example file UnitarySystem_VSHeatPumpWaterToAirEquationFit. So I guess with this branch I will create a diff in that file. |
Actually, it won't because that one isn't autosized. I'm borrowing from that to make a unitary version that is autosized. |
@mjwitte this seems like a good place to stop. 2 files had a large increase in UnitarySystem air flow to match the cooling coil air flow size. 1 had an increase in supp heater size since it was a CoolReheat application. 1 added a design-size supp heater capacity to compare to the user specified value. 5Zone_Unitary_VSDesuperheatWaterHeater
5Zone_Unitary_HXAssistedCoil
Big change here same as above, 16.14 tons cooling = 193680 Btu/hr supp heating coil capacity. UnitarySystem_MultiSpeedCoils_SingleMode
|
} else if (this->m_Humidistat && this->m_DehumidControlType_Num == DehumCtrlType::CoolReheat) { | ||
state.dataSize->SuppHeatCap = max(this->m_DesignCoolingCapacity, this->m_DesignHeatingCapacity); | ||
} else { | ||
state.dataSize->SuppHeatCap = this->m_DesignHeatingCapacity; | ||
} |
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 one issue with this is that this->m_DesignHeatingCapacity is the capacity of the heating coil. For a HP in a cooling dominated climate, this is the same size as the cooling coil (heating capacity was upsized to match). So this is not the raw heating load when a HP is used. If that HP does not use CoolReheat, do we want the supplemental heating coil capacity to match the heating coil, or be sized to meet the heating load? I just ran a local test suite to see what diffs this correction would cause (i.e., using saveRawHeatingCapacity here in the else). Only 1 additional example file had a diff, the HVACTemplate-5ZoneUnitarySystem file.
Not the diff I was expecting, I thought the supp heater would have a lower capacity. That means this HP is not meeting the heating load?
HVACTemplate-5ZoneUnitarySystem diff:
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.
Ah, the UnitarySystem finds cooling = 5kW and heating = 10kW. The WSHP has it's own sizing routine and finds cooling = 5kW and sets heating cap = cooling cap. The UnitarySystem calls over to the WSHP and gets the coil sizes. So the 10 kW was the original heating capacity. Just another never ending thing that needs fixing. I'll post an 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.
I'm surprised we can size anything in E+. Here's a single-zone HP of 5 kW size where total cooling = 5 kW and heating (supposedly) = 10 kW. The 5 kW HP has no problem meeting the winter design day zone load and holds Tstat temp. On the design days, the coils never exceed 3.5 kW although the cooling coil design sensible capacity is 3.8 kW, so that's close, and how these coils were sized. Is heating performance just luck? The heating size = 10 kW comes from a capacity calculation using the cooling design flow (e.g., main system flow = max(coolingFlow, heatingFlow) and a system Central Heating Design Supply Air Temperature = 50 C (Sizing:Zone SAT also = 50 C). The cooling air flow is 5 times larger than heating. At 5x flow, you don't need 50 C SAT to meet the zone heating load. That could be just dumb luck that the cooling size worked here. How often is the peak cooling load similar to the heating load? It seems HP sizing based on geography is a little more complex than first thought.
System heating flows for Sys 4 from 2 design days shown in eplusssz.csv and zone air flow from epluszsz.csv:
When I request the system heating flow rate in UnitarySystem this is what I get. Someone's messin with me.
if (this->dataFlowUsedForSizing > 0.0) { | ||
DesVolFlow = this->dataFlowUsedForSizing; | ||
} | ||
} else if (this->zoneEqSizing(this->curZoneEqNum).HeatingCapacity) { |
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.
UnitarySystem now sets the flag in the HeatingCoils::SimulateHeatingCoilComponents call so that sizing knows this is a supplemental heating coil. With that information UnitarySystem can set the capacity of the coil according to whether CoolReheat dehumidification control is used.
@@ -322,6 +322,8 @@ void resetHVACSizingGlobals(EnergyPlusData &state, | |||
state.dataSize->DataCoilSizingAirInHumRat = 0.0; | |||
state.dataSize->DataCoilSizingAirOutTemp = 0.0; | |||
state.dataSize->DataCoilSizingAirOutHumRat = 0.0; | |||
state.dataSize->DataCoolCoilType = -1; | |||
state.dataSize->DataCoolCoilIndex = -1; |
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.
Needed to call over to VS coils to get T for CapFT curve.
@@ -1534,6 +1534,9 @@ namespace UnitarySystems { | |||
} else { | |||
state.dataHVACGlobal->DXCT = 1; // uses normal DX coil flow limits | |||
} | |||
// sizing may need to know what type of coil is being sized | |||
state.dataSize->DataCoolCoilType = this->m_CoolingCoilType_Num; | |||
state.dataSize->DataCoolCoilIndex = this->m_CoolingCoilIndex; |
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 information is set only in UnitarySystem.
|
||
// STEP 4: set heat pump coil capacities equal to greater of cooling or heating capacity | ||
if (this->m_HeatPump) { // if a heat pump, use maximum values and set main air flow and capacity variables | ||
// if a heat pump, use maximum values and set main air flow and capacity variables | ||
if (this->m_HeatPump && (state.dataSize->CurZoneEqNum == 0 || !isWSVarSpeedCoolCoil)) { |
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.
Don't need CurZoneEqNum == 0 here. Will need to reevaluate this logic when this branch gets merged with #9273 so will address that then.
EqSizing.DesCoolingLoad = max(EqSizing.DesCoolingLoad, EqSizing.DesHeatingLoad); | ||
EqSizing.DesHeatingLoad = EqSizing.DesCoolingLoad; | ||
state.dataSize->DXCoolCap = EqSizing.DesCoolingLoad; | ||
} |
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 WS VS coil retains the cooling/heating coil size even though it's a heat pump.
@@ -616,6 +616,7 @@ add_simulation_test(IDF_FILE UnitarySystem_TwoStageDXWithHumidityControl.idf EPW | |||
add_simulation_test(IDF_FILE UnitarySystem_VSCoolingCoil.idf EPW_FILE USA_IL_Chicago-OHare.Intl.AP.725300_TMY3.epw) | |||
add_simulation_test(IDF_FILE UnitarySystem_VSHeatPumpWaterToAirEquationFit.idf EPW_FILE USA_IL_Chicago-OHare.Intl.AP.725300_TMY3.epw) | |||
add_simulation_test(IDF_FILE UnitarySystem_WaterCoils_wMultiSpeedFan.idf EPW_FILE USA_IL_Chicago-OHare.Intl.AP.725300_TMY3.epw) | |||
add_simulation_test(IDF_FILE UnitarySystem_ZoneVSWSHP_wDOAS.idf EPW_FILE USA_CO_Golden-NREL.724666_TMY3.epw) |
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 were no WS VS coils wrapped in a UnitarySystem.
@@ -7461,7 +7523,7 @@ namespace UnitarySystems { | |||
|
|||
if ((CoilType_Num == DataHVACGlobals::Coil_HeatingGasOrOtherFuel) || (CoilType_Num == DataHVACGlobals::Coil_HeatingElectric)) { | |||
HeatingCoils::SimulateHeatingCoilComponents( | |||
state, CompName, FirstHVACIteration, _, this->m_SuppHeatCoilIndex, _, _, this->m_FanOpMode, this->m_SuppHeatPartLoadFrac); | |||
state, CompName, FirstHVACIteration, _, this->m_SuppHeatCoilIndex, _, true, this->m_FanOpMode, this->m_SuppHeatPartLoadFrac); |
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.
In addition to being used in sizing this flag also selects the report variable in ReportHeatingCoil. Probably should eliminate the Supp variable here.
if (coilIsSuppHeater) {
state.dataHVACGlobal->SuppHeatingCoilPower = heatingCoil.ElecUseLoad;
} else {
state.dataHVACGlobal->ElecHeatingCoilPower = heatingCoil.ElecUseLoad;
}
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.
Well, can't get rid of that variable. These are used in Parent objects to sum the power for each component. So if there is a main heating coil and supplemental heating coil, these need to be kept separate.
This is finally ready for review. |
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, we're near the finish line (after about 20 laps around the track).
Unit tests run locally - check.
Sizing for Unitary-ZoneVSWSHP_wDOAS now aligns with ZoneVSWSHP_wDOAS - check.
CI diffs for 5Zone_Unitary_HXAssistedCoil show reheat coil cap matches cooling coil cap - check
CI diffs for UnitarySystem_MultiSpeedCoils_SingleMode, now the supp heat coil reports a design size that aligns with the UnitarySystem heating capacity (this has no reheat) - check.
The only things left are to clear up the build warning and make sure the top comment description aligns with the final answer here.
…7-UnitarySystem-VS-coil-sizing
I went over the issue and PR descriptions last night. Reading again I don't see any missing discussion. Each of the 4 bullets is addressed in this PR. |
Thanks @rraustad ! Merging. |
Pull request overview
NOTE: ENHANCEMENTS MUST FOLLOW A SUBMISSION PROCESS INCLUDING A FEATURE PROPOSAL AND DESIGN DOCUMENT PRIOR TO SUBMITTING CODE
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.