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
Include fan heat when sizing zone cooling coils and adjust airloop coil inlet/outlet conditions #6753
Include fan heat when sizing zone cooling coils and adjust airloop coil inlet/outlet conditions #6753
Conversation
@Myoldmopar these results concern me since this change should have shown diff's for the i386-Windows-7-VisualStudio-14 build. I'm seeing the same thing (intermittently) on the FSEC CI. Any ideas? |
@rraustad I haven't trusted diffs on the Win platforms for a long time. I frequently see no diffs from Win when the other platforms show diffs. I don't think it's building the correct baseline branch, but I can't prove that. I focus on the diffs from Mac and Linux and only address failures on Win. |
There is a section in CI.rb that tests for a build folder and skips the regression tests if that folder exists. My guess is that the build folders on i386 Win are not getting completely deleted. at line 337:
|
You might be onto something big time @rraustad.... That code right there does a couple things:
I do know we have had trouble deleting some folders in the past because the operating system thought a process was still holding onto a random file deep in the build folder. If that's the case, then this code will fallthrough and the build folder will be there so the code won't rebuild. Then when regressions run, the base folder won't be there, so the regressions won't show up as failures. This could explain how @mjwitte doesn't see diffs on Windows when he expects to see them. Unfortunately, @lefticus has tried really hard to delete the build folders using a variety of methods. If you recall, we used to have trouble with the CI machine hard drives filling up because the old builds wouldn't get deleted. I believe he exhausted his known options the last time we had to deal with this issue, but maybe there's something new we can try. I'll add this to my list of things to look into, and in the meantime keep looking to the Linux and Mac boxes for diffs, and we'll rely on Windows for unit and integration test failures, and Windows-only build issues. (How acceptable would it be to just disable regressions on Windows...? If we can rely on diffs being reported by Linux and Mac, it would drastically speed up the Windows CI runs to not have to build a wasted baseline.) |
No regressions on Windows? Short-term, sure, but long-term should work to reliably restore them, even if we always build regardless of an existing build folder. But even then, would we still have trouble with possibly locked files? |
Feel free to offer a pull request if you want and if you haven't already.
Great work!
On Sat, May 26, 2018 at 20:58 Richard Raustad ***@***.***> wrote:
After some edits in the scripts, we are getting identical answers between
i386 and Win64, as it should be.
[image: fsec_ci_results]
<https://user-images.githubusercontent.com/5778156/40581820-ca08c2f4-612f-11e8-90c2-eadcfc7afe2c.jpg>
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#6753 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAz3oFQd3c2IqOyDcv8MX5d7UuRQKBX8ks5t2ggpgaJpZM4UFFHS>
.
--
Edwin Lee, PhD
Engineer, Commercial Buildings Research Staff
National Renewable Energy Laboratory
|
Well I may have simplified the answer. Our IT has been looking into how to reliably delete the build folders and the answer may be outside the scripts so a PR wouldn't be possible. I'll let you know what I find. |
Thanks for letting me know anything you do find @rraustad; I'm excited to see the build folders get reliably deleted. As for this feature, I was thinking I'd just comment on the NFP here? I didn't see a separate conversation, so my apologies if some of these have already been discussed.
|
OutAirUnit(OAUnitNum).EFanMaxAirVolFlow = HVACFan::fanObjs[OutAirUnit(OAUnitNum).SFan_Index]->designAirVolFlowRate; | ||
OutAirUnit(OAUnitNum).ExtFanAvailSchedPtr = HVACFan::fanObjs[OutAirUnit(OAUnitNum).SFan_Index]->availSchedIndex; | ||
OutAirUnit(OAUnitNum).EFanMaxAirVolFlow = HVACFan::fanObjs[OutAirUnit(OAUnitNum).ExtFan_Index]->designAirVolFlowRate; | ||
OutAirUnit(OAUnitNum).ExtFanAvailSchedPtr = HVACFan::fanObjs[OutAirUnit(OAUnitNum).ExtFan_Index]->availSchedIndex; |
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.
Fixed typo in exhaust fan.
…-Feature---Account-for-fan-heat-is-zone-cooling-coils
Adding documentation to Engineering Reference. The fan heat calculations are in section Loop, Equipment Sizing and other Design Data, right after Fan Sizing is a new section for Design Fan Heat. Then in the coil objects, a term for design fan heat is added to the design coil load calculation and the fan heat section is referenced in the equation variable description (i.e., see Section 10.7.3).
|
@@ -968,8 +1019,10 @@ \subsubsection{System Coils}\label{system-coils-5} | |||
|
|||
\emph{CapModFac} = \emph{CurveValue}(CCapFTemp,\emph{T\(_{mix,wb}\)},\emph{T\(_{outside}\)}) | |||
|
|||
\emph{Q\(_{fan,heat,des}\)} = design fan heat (W) - see Section \ref{design-fan-heat} |
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.
How do I get the \dot{Q} inside the \emph{}. I tried a few different ways and nothing worked.
\frac{\dot{m}_{air,des}\PB{48000 -H_{out}}}{TotCapTempModFac} & otherwise | ||
\end{array} | ||
\right. | ||
\dot{Q}_{coil,des,total} = \frac{\dot{m}_{air,des}\PB{H_{in}-H_{out}}}{TotCapTempModFac} + \dot{Q}_{fan,heat,des} |
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 Coil:Cooling:WaterToAirHeatPump:EquationFit model doesn't check for Hin>Hout, the variable speed model does. So I corrected the doc. I could have corrected the code but I never did like that test. If Hin<Hout then there should be 0 capacity.
I don't see anything unexpected in the CI diff's. Coil sizes are a little larger and associated coil properties (e.g., water coil UA and DX coil SHR) are adjusted accordingly. |
@Myoldmopar I'll need some help with the warning. I didn't change that? | location | message -- | -- | -- warning | src/EnergyPlus/DataAirSystems.hh:404:44 | unused variable EnergyPlus::DataAirSystems::PrimaryAirSystem' [-Wunused-variable] |
The full error message is several lines long, so the CI parser doesn't grab the whole thing. Here's what I see when I compile locally:
This tells me that in the |
I am looking at the results of water coil sizing. I was bothered about the first water coil I looked at where the reported coil capacity increased by only 1 W. The fan heat is getting added to the coil capacity calculation. for example file 5ZoneFanCoilDOASCool fan heat is on the order of 17 W. This additional load does translate directly to an increase in water flow rate of 6E-7 m3/s. MaxColdWaterVolFlowDes = DesCoilLoad / (WaterCoilSizDeltaT * Cp * rho);
However, this resulted in a reported coil capacity increase of 1 W and a UA decrease of 2 W/C and corresponding decrease in coil surface area.
So something is wrong here.
This is a fan coil unit.
|
…-Feature---Account-for-fan-heat-is-zone-cooling-coils
I've reviewed many of the diff's and they all appear to reflect fan heat added to capacity and inlet or outlet condition with slight increases in water cooling coil water flow rate. So this appears ready. Just so you know I revised this so that fan heat increases inlet air temp for blow-thru and decreases outlet air temp for draw-thru (I originally put the fan heat at the coil inlet which was wrong for draw-thru configurations). I was scared of that at first and then thought that we do that all the time with the mixed air SPM so why no go ahead and do it for sizing. |
…-Feature---Account-for-fan-heat-is-zone-cooling-coils
The example file results I have reviewed so far show an increase in capacity and a change in inlet or outlet air conditions based on blow- or draw-thru fan configuration. This is looking pretty good so far. |
Things still look good but noticed a few things a reviewer would need to understand:
|
…e---Account-for-fan-heat-is-zone-cooling-coils
@rraustad Have you reviewed the err diffs? Some of these may need to be addressed. CoolingTower_FluidBypass and similar warnings for CoolingTower_TwoSpeed_MultiCell, CoolingTower_VariableSpeed_MultiCell, EMSDemandManager_LargeOffice, EMSReplaceTraditionalManagers_LargeOffice. On the good new side, coil sizing warnings are now gone for RefBldgSecondarySchoolNew2004_Chicago and RefBldgSecondarySchoolNew2004_Chicago - woohoo! RefBldgSmallHotelNew2004_Chicago And strange messages for _5ZoneAirCooled_annual under Failure here:
|
Those warnings are all a result of changing the inlet or outlet temperature based on fan heat. I knew the water coil would be a thorn in my side when deciding to adjust the outlet air temp for draw-thru configurations. But since we do that during the simulation with the mixed air temp SPM I thought the problems would be minimal and went ahead and adjusted coil air properties accordingly to fan location. I also think that the warning for bypass factor calculation failure points to some other issue since that warning is only printed if bypass factor > 0.5 (highly unlikely). Given the temperatures in the messages (i.e, inlet water temp vs inlet and outlet air temp) the bypass factor should not be that large so something else is going on (I think ADP is calculated much colder than the inlet water temp). I probably don't have time to correct that in this branch but will start investigating the correction. I also have another check in since I just found that water coils are counting fan heat twice when reporting outlet air temperature (e.g., water coil calls CoolingWaterDesAirInletTempSizing and CoolingWaterDesAirOutletTempSizing where fan heat is accounted for then passes those data over to CoolingCapacitySizing in RequestSizing. This would double count fan heat for water coils with draw-thru fans. I'm trying to figure out the best way around this. Maybe it's a simple matter of only accounting for fan heat when capacity is calculated, not sure at this point.
|
Perfect example of the bypass factor warning that existed before this change and is exacerbated when SAT is lowered. Example file: RefBldgLargeHotelNew2004_Chicago I looked through all err files and don't see anything that should hold this up.
|
I'm going to post a followup issue then to clean up the example files and get rid of the coil sizing warnings. |
@Myoldmopar Can you help diagnose why tablediff is failing on _5ZoneAirCooled_annual? Look here, expand the regression test failures, and search for "traceback". |
That code is failing here. It is in a section of TableDiff where the diff tables are being written. Specifically, it's trying to write a value to a cell inside a diff table. That code has an IF block, where if the header of this is "DummyPlaceholder" or "Subcategory", it just writes the value out to the table. If it isn't one of those values, it assumes the value is a list, and unpacks two values from it and only writes the first one. It's failing because it seems like this header is Dummy or Subcategory, yet it has a list instead of a single value. I looked over the code and didn't see anything obvious in the tabular reports. Is there anything that you see that could be causing a header change, or structural output value change? If there isn't, then maybe re-run CI to make sure that persists, and then if it does I'll take a look locally. |
The order of columns is changing in one of the tables. Would that be the culprit? Component Sizing Summary, Coil:Cooling:Water, the Design Size Design Inlet Air Humidity Ratio is earlier now than before. |
…-Feature---Account-for-fan-heat-is-zone-cooling-coils
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.
Some more questions/comments.
RoutineName, // Routine name | ||
CurrentModuleObject, // Object Type | ||
VarSpeedCoil(DXCoilNum).Name, // Object Name | ||
cAlphaFields(AlfaFieldIncre)); // Field 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.
@Myoldmopar So, seems like Visual Studio is interpreting the clang format differently from linux?
// design fan heat will be added to coil load | ||
Real64 FanCoolLoad = DataAirSystems::calcFanDesignHeatGain(DataFanEnumType, DataFanIndex, VolFlowRate); | ||
// inlet/outlet temp is adjusted after enthalpy is calculcated so fan heat is not double counted | ||
Real64 CpAir = PsyCpAirFnWTdb(MixHumRat, MixTemp); |
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've mentioned this before, I think, but why isn't MixEnth adjusted 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.
Given the test below at line 4082 for MixEnth > SupEnth it looks like MixEnth or SupEnth should have been updated here. However, the comment "The mixed air temp for zone equipment without an OA mixer is 0" implies it doesn't matter because MixEnth will always be greater than SupEnth (unless MixEnth=0 and then if IF at line 4082 would still work as intended).
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 wasn't asking about this particular coil type, but I think this is the general method elsewhere as well, no? Leave the enthalpies, add the load, but adjust the entering or leaving conditions. Just seems it would be cleaner to adjust the entering or leaving conditions first, then the enthalpies would already include the extra fan heat.
@@ -2143,6 +2143,23 @@ namespace WaterCoils { | |||
ShowContinueError("The autosize value for max air volume flow rate 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.
It doesn't hurt anything other than it causes reordering in some of the output.
Ran a few annual simulations just to see what the overall diffs are. Surprising: 5ZoneAirCooled, PackagedTerminalAirConditioner - Some coil sizing diffs, as expected, <1% annual energy, no difference in setpoint not met or not comfortable hours. FanCoilAutoSizen, PackagedTerminalHeatPump - Some coil sizing diffs, as expected, <1% annual energy, small diffs in setpoint not met or not comfortable hours. RefBldgHospitalNew2004_Chicago - no diffs at all - is that expected? All airloops with detailed chilled water coil. |
RefBldgHospitalNew2004_Chicago - air loops already accounted for the fan heat so the diff's would be in the html reporting of entering/leaving air temp. |
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.
At this point, just waiting for linux ci to finish up to check if the doc build errors are gone and reconfirm performance change (last round showed a 1.5% improvement, not sure how that's possible).
Oh - there it is - only the cooling coi leaving wet bulbs and enthalpies. Seems like something more should change, but not pressing this. |
Engineering ref builds locally and looks OK. Gort seems rather far behind, so merging this so we can get going on others that will likely have conflicts after this drops. If there are any doc build problems, we'll fix them later. |
Pull request overview
Overview: Include fan heat when sizing zone cooling coils.
Description: Zone equipment that include use of a cooling coil have sized that equipment using only the zone load. This update includes design fan heat in the cooling coil sizing algorithms. For all coil types the design coil load now includes the zone load plus a term for design fan heat. For water coils, the additional fan heat increases the required water flow rate and the coil inlet air temperature (for blow-thru fans) or decreases the outlet air temperature (for draw-thru fans) used during sizing. Although system cooling coils, those cooling coils in air loops that reside directly on the main branch, have previously included fan heat, this update also adjusts the coil air inlet/outlet temperature (same as implemented with this update for zone coils). Heating coils are not affected by this change and do not currently include fan heat or an adjustment to coil entering or exiting air condition in the sizing algorithms.
Equipment modified by this change include:
Equipment not modified
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.