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
PTAC hot water heating coil undersized for ventilation load #6079
PTAC hot water heating coil undersized for ventilation load #6079
Conversation
…-PTAC-heating-coil-undersized-for-ventilation-load-when-zone-design-heating-load-is-zero
OK, so the fix appears to work but it highlights another issue for sizing. Diff's in EMSCurveOverride_PTHP. The "design" flow rate is now the user specified in the PTHP since we tell the sizing routine what the PTHP wants the child component flow to size to. What the "design" flow was before was the design zone heating flow rate, what it is now is what the parent sized to. If the parent was autosized, the flow would be the "design". Since the parent is not autosized, the "design" for the children is the user specified parent flow (not the sizing results). So this is right but it's wrong.
|
I think I have a method to save the design values and report these instead of the parent object values. This isn't fully tested with things like scalable sizing, etc. but appears to work (i.e., logic of where to switch the saved value with the design value may change). We'll see what the CI thinks. |
Is that really what we want? One could argue that the size set by the parent is the design size, which the user might override with a hard value. Just thinking out loud. |
There are many options to what we may want to report. And your right, if the parent object passes information to the child, the parent object report of design vs user is still valid. So the user could look there for that type of info. I had also thought of supressing the "design" report for child components since it doesn't mean much when the parent sets the size. I just wanted to be able to get back to where we were and see if I could do it without much effort. Well, as to effort, this would have to be added in many places in ReportSizingManager if we choose this route. This one is borderline as to getting into V8.7 so getting back to where we were and making a decision later is not a bad move. Also, the water heating coil sizes are much larger now. This sort of makes sense but am weary of the consequences. |
OK, now OutdoorAirUnit fails and changes the design size from what is specified by the parent to the zone design air flow rate. This is wrong since the OA flow rate is not the same as the zone flow rate to meet the zone load. So unless non-zone related equipment is treated differently, then what we do now is OK for the time being. |
…-PTAC-heating-coil-undersized-for-ventilation-load-when-zone-design-heating-load-is-zero
Nothing unexpected here except the convergence issue with MicroCogeneration, loads exceed 0.04W and hits a whopping 0.06W max. Also a few more plant loop temp warnings (from 4 to 25 with max temps rising from 108 to 127). The water heating coils did increase by x3-x4 in size. |
MicroCogeneration passes warmup convergence in 6 warmup days on Windows. So why does this fail on Mac and Linux? |
…PTAC-heating-coil-undersized-for-ventilation-load-when-zone-design-heating-load-is-zero
@@ -1094,10 +1099,16 @@ namespace ReportSizingManager { | |||
NominalCapacityDes = DesMassFlow * DataWaterCoilSizHeatDeltaT * Cp * rho; | |||
// if coil is part of a zonal unit, calc coil load to get hot water flow rate | |||
} else { | |||
if ( ZoneEqSizing( CurZoneEqNum ).SystemAirFlow ) { | |||
DesMassFlow = ZoneEqSizing( CurZoneEqNum ).AirVolFlow * StdRhoAir; | |||
} else if ( ZoneEqSizing( CurZoneEqNum ).HeatingAirFlow ) { |
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 logic flow here seems a little suspect. I would expect an if - else-if block to be such that only one would be satisfied at a given time, but here both ZoneEqSizing( CurZoneEqNum ).SystemAirFlow and ZoneEqSizing( CurZoneEqNum ).HeatingAirFlow are often true at the same time, like it is in the unit test now. So it will flow to the if and skip the else-if even though it is also satisfied. That may be what you want but it seems hard to follow.
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 look at this as priority order when more than 1 is true.
@@ -4186,6 +4186,15 @@ namespace PackagedTerminalHeatPump { | |||
if ( CurZoneEqNum > 0 ) { | |||
ZoneEqSizing( CurZoneEqNum ).OAVolFlow = max( PTUnit( PTUnitNum ).CoolOutAirVolFlow, PTUnit( PTUnitNum ).HeatOutAirVolFlow ); | |||
ZoneEqSizing( CurZoneEqNum ).AirVolFlow = max( PTUnit( PTUnitNum ).MaxCoolAirVolFlow, PTUnit( PTUnitNum ).MaxHeatAirVolFlow ); | |||
// save air flow rates for child object sizing | |||
if( PTUnit( PTUnitNum ).MaxCoolAirVolFlow > 0.0 ) { |
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 bools SystemAirFlow, CoolingAirFlow, and HeatingAirFlow are true whenever the values are calculated, but what is really needed is to know which one needs to be used. So instead of a test like "PTUnit( PTUnitNum ).MaxCoolAirVolFlow > 0.0," I would look for conditions like "PTUnit( PTUnitNum ).MaxCoolAirVolFlow > PTUnit( PTUnitNum ).MaxHeatAirVolFlow." I just find it a bit confusing that both CoolingAirFlow and HeatingAirFlow can be true at the same time. just a minor comment since this is already the way it is.
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 PTAC and PTHP have inputs for cooling, heating and OA flow rates. So this was set up to allow cooling and heating flow rates to be sized differently (e.g., scalable sizing).
okay, think this is ready to merge. Only serious diff from CI is in MicroCogeneration integration test and the lack of convergence in the heating design day. The hot water loop is coming out 2.7 times larger, which is expected with these changes, but the model has a mix of hard sizes and autosizes. The increased flow rate on the stratified tank use side is now washing out the tank's stratification compared to the reference case and makes for less stable hot water supply. Increasing the tank's volume (a hardsized value) solves the convergence problem. commited a modified MicroCogeneration.idf file. Other diffs are expected. will merge once CI comes back. |
…-heating-coil-undersized-for-ventilation-load-when-zone-design-heating-load-is-zero Conflicts: src/EnergyPlus/ReportSizingManager.cc
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.
@rraustad I've merged in develop and tested the defect file (which fails in debug mode due to messed up geometry - probably need a new issue for that . . .) with both design days and annual. The 2000+ hours of setpoint not met during occupied heating are now down to 9. Success.
But spot checking a couple of coils show they are significantly oversized compared to the max load on the heating design day.
THERMAL ZONE FLR1 CORE1 PTAC HEATING COIL, Design Size 9486W, max load 3678W.
UNIT 116 PTAC HEATING COIL, Design Size 2475W, max load 1380W.
Is that expected? Could we do better?
Looking at HVACTemplate-5ZonePTAC, the SPACE2-1 PTAC HEATING COIL design coil load was 3890W and now it's 20272W. And the max load across design days and annual is only 4432W. Can we explain this?
The packaged units are sizing cooling and heating air flow rate as the max of either, when non-HP type units could operate at their own air flow rate as with the UnitarySystem?
Both heating and cooling air flow rate are sized as:
So yes, I'd say this is expected given how these models size air flow rate. Should this be changed, probably. |
I'll have to look at HVACTemplate-5ZonePTAC a little closer to see why that coil is sized so large. |
I haven't looked closely at the equations used here, but seems that this should be able to size closer to the zone heating load plus ventilation load, regardless of the supply air flow rate, similar to how reheat coils are sized. |
@mjwitte I like this one, it makes sense to me. |
The defect file looks good now. The peak heating capacity needed to meet the TSTAT temp is about 3000 W for Unit 116 and 4000 W for Unit 206. The coils sized at 4051 W and 5928 W. UA looks reasonable as well after I corrected the design inlet air temperature used for water coils.
|
} else { | ||
OutAirFrac = 0.0; | ||
} | ||
CoilInTemp = ( 1.0 - OutAirFrac ) * FinalZoneSizing( CurZoneEqNum ).ZoneTempAtHeatPeak + OutAirFrac * FinalZoneSizing( CurZoneEqNum ).OutTempAtHeatPeak; |
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.
What is changed throughout water coil sizing is how the inlet air temperature is calculated. The FinalZoneSizing variable for DesHeatCoilInTemp is wrong at times where the zone air flow rate is <= the OA flow rate. When this happens, the coil inlet temp is = the OA temp. But it's not really, when the water coil is in a parent that operates at a higher air flow rate. So to correct this, the operating coil inlet temp is calculated based on the OA fraction. This is done for water coil inlet T, w, and for the capacity and UA calculation. This method took many iterations as can be seen in the discussion. The result with this method is still a larger coil, but not 4 or 5 times larger as with the initial attempts.
@@ -806,10 +806,21 @@ namespace ReportSizingManager { | |||
} else if ( ZoneEqUnitHeater || ZoneEqVentedSlab ) { // for unit ventilator the cp value is calculated at 5.05(InitConvTemp) for the child and 60.0C for the unit ventilator //|| ZoneEqUnitVent | |||
AutosizeDes = ZoneEqSizing( CurZoneEqNum ).MaxHWVolFlow; | |||
} else { | |||
CoilInTemp = FinalZoneSizing( CurZoneEqNum ).DesHeatCoilInTemp; |
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 new sections get the parent air flow rate and then calculate the OA fraction so a "revised" variable can be calculated based on the zone temp/humrat and OA temp/humrat.
This is ready for final review. |
Except for the conflicts. |
…-PTAC-heating-coil-undersized-for-ventilation-load-when-zone-design-heating-load-is-zero Conflicts: tst/EnergyPlus/unit/PackagedTerminalHeatPump.unit.cc
…-heating-coil-undersized-for-ventilation-load-when-zone-design-heating-load-is-zero
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.
Defect file results look very good. Total boiler capacity is about 50% greater and unmet heating hours are down to 5 now from >2000 before. Merge up to latest develop - hoping the build warnings go away.
} else { | ||
OutAirFrac = 0.0; | ||
} | ||
CoilInTemp = ( 1.0 - OutAirFrac ) * FinalZoneSizing( CurZoneEqNum ).ZoneTempAtHeatPeak + OutAirFrac * FinalZoneSizing( CurZoneEqNum ).OutTempAtHeatPeak; | ||
CoilInHumRat = FinalZoneSizing( CurZoneEqNum ).DesHeatCoilInHumRat; |
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.
Doesn't CoilInHumRat also need to be calculated as a combination of OA and Zone conditions?
Also, more general comment, not following why essentially the same code needs to be inserted in this function multiple times. This is the 4th or 5th place 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.
Also, don't we need similar changes for cooling coil sizing?
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 didn't think that humrat would severely affect heating coil sizing, but yes, it should use the same method (and I was just trying the get this to work and ran out of steam). These changes evolved to a point where I think now I understand why the sizing data (T and w) isn't applicable when a zone coil is used in a parent object operating at a higher flow rate. So I think this type of change would be pervasive throughout sizing.
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.
Same code 4 or 5 times... anytime coil inlet T needs to be calculated, the new method should be used.
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.
Just seems like that could all happen in one block of code. It's fine for now, but at some future point, this could possibly be refactored to consolidate similar calcs.
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.
CoilInHumRat only needed to be changed here.
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.
- CoolingWaterDesAirInletTempSizing already uses this method for ZoneEqFanCoil except it needs to use the parent flow rate if applicable and the else needs to be fixed.
- CoolingWaterDesAirInletHumRatSizing already uses this method for ZoneEqFanCoil except it needs to use the parent flow rate if applicable and the else needs to be fixed.
- CoolingCapacitySizing needs these changes
I think that's it.
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.
It looks like there's a place in HeatingCapacitySizing (non-water coils) that could use this change, but without a defect file I'm scared to change it. I'm going to add a warning there to see if any example files use that line of code.
} | ||
FinalZoneSizing( CtrlZoneNum ).ZoneHumRatAtHeatPeak = ZoneSizing( DDNumF, CtrlZoneNum ).HeatZoneHumRatSeq( TimeStepAtPeakF ); | ||
FinalZoneSizing( CtrlZoneNum ).OutHumRatAtHeatPeak = ZoneSizing( DDNumF, CtrlZoneNum ).HeatOutHumRatSeq( TimeStepAtPeakF ); |
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.
Are we already saving OutTempAtCoolPeak and OutHumRatAtCoolPeak somwhere?
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.
Same comment as above, I was focusing on getting this fix to work. This same change needs to be applied to zone cooling coil.
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, do you want to add to this PR or open a new one for additional changes?
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 guess I still have time. I'll go through and update humrat for zone heating coils and take a look at what the cooling coils need. I may not change those due to diff's but I guess we know what diff's this change caused so any change in diff's would be due to changes in cooling coil sizing.
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.
If you remember this issue, these same changes were never applied to cooling since we've never really seen sizing problems with cooling coils.
// issue 6006, heating coils sizing to 0 when no heating load in zone
if ( ZoneSizing( DDNumF, CtrlZoneNum ).DesHeatSetPtSeq.empty() ) {
ShowSevereError( RoutineName + ": Thermostat heating set point temperatures not initialized for Zone = " + FinalZoneSizing( CtrlZoneNum ).ZoneName );
ShowFatalError( "Please send your input file to the EnergyPlus support/development team for further investigation." );
} else {
FinalZoneSizing( CtrlZoneNum ).ZoneTempAtHeatPeak = *std::max_element( ZoneSizing( DDNumF, CtrlZoneNum ).DesHeatSetPtSeq.begin(), ZoneSizing( DDNumF, CtrlZoneNum ).DesHeatSetPtSeq.end() );
FinalZoneSizing( CtrlZoneNum ).OutTempAtHeatPeak = *std::min_element( ZoneSizing( DDNumF, CtrlZoneNum ).HeatOutTempSeq.begin(), ZoneSizing( DDNumF, CtrlZoneNum ).HeatOutTempSeq.end() );
}
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.
Oh wait, I see it now. That change was made for both cooling and heating. Line 2776 of ZoneEquipmentManager.
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 the above code snippet, this line is missing for cooling at line 2782. Same for OutHumRatAtCoolPeak.
FinalZoneSizing( CtrlZoneNum ).OutTempAtHeatPeak = *std::min_element( ZoneSizing( DDNumF, CtrlZoneNum ).HeatOutTempSeq.begin(), ZoneSizing( DDNumF, CtrlZoneNum ).HeatOutTempSeq.end() );
…lLoadUsedForUASizing
@rraustad Is this ready for final review? |
No, I added this warning to find the files that used a place where I want to make a change. Now I have test files to look at while making this change.
|
Wow, this is so risky. If the equipment models are not set up properly I could break something. In addition, the SystemAirFlow flag should only be used for fans. HeatingAirFlow and CoolingAirFlow are used for coils. The way the equipment models are set up now, some set SystemAirFlow only, some don't set any, and some set one or both coil flags. In the ReportSizingManager sizing functions for coils (e.g., HeatingCapacitySizing or CoolingCapacitySizing) there should not be a SystemAirFlow test, nor a test for the opposite coil type. Then there's the zone equipment scaling functions, they all use SystemAirFlow. This all needs to be sorted out eventually, it's just that it's too close to release to be taking a crack at it now. |
…-PTAC-heating-coil-undersized-for-ventilation-load-when-zone-design-heating-load-is-zero
Sorry, I just see too many black holes. I updated issue #6172. |
@mjwitte I did take a crack at it tonight and saw too many issues that scared the steel out of me. |
Ran all of the files with diffs as annuals. Annual energy use and not met hrs diffs are tiny. |
Pull request overview
User file has PTACs with hot water heating coils. PTACs provide ventilation air. For zones with a zero design heating load, the PTAC heating coil is undersized and cannot temper the ventilation load. This change addresses #6028 and results in a 3 to 4 fold increase in water heating coil size in some cases. This change also results in component air flow sizing the same as the Packaged Terminal unit.
Work around: adding a small negative other equipment load to those zones in order to force a non-zero design heating load fixes the coil sizing.
Work Checklist
Add to this list or remove from it as applicable. This is a simple templated set of guidelines.
Review Checklist
This will not be exhaustively relevant to every PR.