-
Notifications
You must be signed in to change notification settings - Fork 396
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
Use Standard air density for sizing all cooling coil types (water coils and ideal loads air system already used standard air density) #9431
Conversation
} else if (this->zoneEqUnitVent) { | ||
PeakCoilLoad = max(0.0, (state.dataEnvrn->StdRhoAir * DesVolFlow * (CoilInEnth - CoilOutEnth))); | ||
} | ||
Real64 PeakCoilLoad = max(0.0, (state.dataEnvrn->StdRhoAir * DesVolFlow * (CoilInEnth - CoilOutEnth))); |
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.
All coil types now use the same conversion from volume to mass
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.
👍
EIO diffs (typical of reported):
Stopped here. |
Simple warning is fixed, waiting for comments. |
An interesting realization here is that the zone design cooling mass flow rate is at the supply air moisture content. The air is already dehumidified. So for coil sizing I am wondering if this mass flow rate should be corrected somehow (e.g., by ratio of CpAir,in/CpAir,out)? This change corrects back to zone design cooling mass flow rate which should always be lower than coil inlet mass flow rate. I guess we take one step at a time where all coils will be doing the same thing.
|
Since there is a lot to think about with a change like this, I looked at the parent and child objects to see how they convert volume to mass. All the parent objects use Design Volume * StdRhoAir. The fan models also use this convention. The cooling tower, earth tube and evap cooler use actual air density which seems reasonable since they are calculating a mass flow rate to meet a load. The DX coil model stands out but is using a rating point to calculate the rated mass flow rate (at altitude), and then when used in a parent is operating at the (zone or airloop) design mass flow rate. Parent Objects:
Child Objects:
|
Here's some history on air density. On May 9, 2001, the standard air definition was changed here to address #294, #295, and #296 (which were old CR3573, CR3574, and CR3575). So now The next day, it was changed to use standard barometric pressure here So, yeah, once this was changed to use |
} else if (this->zoneEqUnitVent) { | ||
PeakCoilLoad = max(0.0, (state.dataEnvrn->StdRhoAir * DesVolFlow * (CoilInEnth - CoilOutEnth))); | ||
} | ||
Real64 PeakCoilLoad = max(0.0, (state.dataEnvrn->StdRhoAir * DesVolFlow * (CoilInEnth - CoilOutEnth))); |
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 could you comment on how this relates to #9425? It looks like the centralized sizing routine made this a minimal change, but are there other places that still need this change? |
#9425 focused on the variable speed coil model and to try to line up VS coil sizing with what UnitarySystem used as mass flow rate and coil capacity (the VS coil model caused diffs for PTUnit to UnitarySystem branch). That change was a result of finally realizing that zone design mass flow rate (based on Cp of supply air humidity ratio entering the zone) is directly related to zone design volume flow rate based on StdRhoAir. Some component models convert zone design volume to mass using actual inlet condition (VS coil and other cooling coils) and others use StdRhoAir (e.g., fan). That should mean components in a parent size to different mass flow rates, but we only see volume flow rate in the eio file so we never noticed it. Now, understanding that, do we really want to calculate a cooling coil mass flow rate using the supply air condition (apply the conversion using StdRhoAir) or the actual inlet condition (using coil inlet air density)? That is really the question here. Water coils and Ideal Loads air systems use StdRhoAir. It seemed reasonable to use StdRhoAir everywhere there is a conversion from zone design air volume to zone design air mass. Another question is what does the zone design air volume flow rate represent if it was derived by dividing zone design air mass flow rate (a moist air mass flow rate) by StdRhoAir (a dry air density) in the zone sizing routines? The resulting volume is not the actual volume entering the zone during zone sizing. The only way to get back to the zone design air mass flow rate is to multiply zone design air volume by StdRhoAir. But that is zone supply air mass flow rate (outlet of the cooling coil). By now you should see the dilema. Should E+ size a cooling coil using supply air mass flow rate or inlet air mass flow rate? In E+ they are incorrectly the same, in reality they are not since condensate leaves the coil. This PR expands on that suggestion of always using StdRhoAir, and the sizing routine consolidation where we said we would circle back and clean up some redundancy. If we choose to use StdRhoAir everywhere then that will clean up the sizing routine so that all coils size the same way. This is a big decision because it changes coil sizes so we need to make sure this is the path to take. Neither of these PRs have been merged yet. I also discussed why these changes were suggested on last conference call and it seemed there was agreement. With sizing changes we should always critique that change as many different ways as possible. I can always figure out a different way to do things if this is not the correct path. |
I scanned HeatingCapacitySizing and did not see anything, although that does not mean a parent passed a volume that was calculated based on inlet air conditions. I was focused here on cooling coil air flow and capacity. If we can come to agreement I think finding other places like this will be fairly easy. |
…0-StdRhoAir-used-in-coil-sizing
Diffs for DXCoilSystemAuto.
With increased capacity I would expect the coil PLR to decrease with very little difference, if any, in the coil delivered capacity since the load has not changed. The change in coil sizing is on the order of +3%. And the PLR actually increased which is opposite of what I expected. The fact that the coil SHR decreased may explain the increase in PLR. Zone temp is the same as is the supply air T and Tsetpoint. I also would have expected the coil PLR to be nearer to 1 on the summer design day but that is not happening. There is no Sizing:Parameters object in this file with cooling/heating sizing multipliers and Sizing:Zone sizing factors are set to 0. So sizing factors are assumed to be 1. |
I find it hard to believe that a 0.07% change in SHR could have that impact on PLR. Looking at other data shows that PLR changed by about 3% (updated figure above). The flow rates have not changed (same load). The current density flow rate is a little higher because the supply node density is a little lower, but not by much. The change in latent capacity is about 0.1%, and is lower now, meaning more sensible capacity at the same PLR now. So I am still not sure why PLR changed as much as it did, and increased instead of decreasing as I would expect. |
I had the PLR data reversed, dotted vs solid, in my head, not the graph. The PLR is now lower than before so lines up with what I would expect. The decrease in PLR is about the same as the increase in capacity. |
Trying to wrap up what all this means. The coil now sizes the capacity based on the same mass flow rate as found during zone sizing. That's a good thing, the coil capacity is directly related to zone load. The new coil model, as other DX coil models, calculates a rated mass flow rate during initialization. This is a rating point for the coil, not the operating point.
The TU calculates the same mass flow rate found during zone sizing. So during the simulation, at peak load, the same mass flow rate as found during sizing is used. That's a good thing.
The new coil model, used in DXCoilSystemAuto, also makes an adjustment to the coil bypass factor the same as the old DX coil model. What this adjustment does is correct coil performance based on a difference between the operating and rated mass flow rate. The operating mass flow rate is now the same as found during zone sizing. And the coil rated mass flow rate is about correct but still a little off because the volume flow rate, used to calculate rated mass flow, is calculated using a StdRhoAir basis, not the actual density of the supply air (i.e., the zone design volume flow is not the volume entering the zone during sizing and is different by rhoAir/StdRhoAir, about 3%). All in all it seems this step moved in the right direction. |
Mostly seeing the same diffs as #9425. Have seen a couple of these new warnings. Changing the capacity should not be an issue with finding a coil SHR. The coil inlet condition is reasonable at about 80F / 55% RH and this coil is fully autosized so a lower SHR would bring the outlet condition to the proper side of the saturation curve. This is more of a clue to help correct logic during sizing. Unless there are one or more hard-sized inputs, this warning should rarely happen (i.e., something needs fixing). #9459. Fenestration_RefBldgSmallHotelNew2004_Chicago
|
Almost finished reviewing these diffs with not much to see and then came across this in UnitarySystem_WaterCoils_wMultiSpeedFan. The water coils already use StdRhoAir so what changed to cause this diff? This is a coil on air loop main branch so nothing should have changed. I don't see anything that should have caused this. This is the same change in capacity as if coil inlet density were changed to StdRhoAir, but StdRhoAir was already used? Update: this is a water coil inside a UnitarySystem parent. The UnitarySystem calls over to CoolingCapacitySizing and is not a water coil, so the returned capacity would have changed and then that capacity would be forwarded to the coil. So yes, this is expected with this change.
|
@rraustad alright after the merge of #9425, I pulled develop into this branch. While merging, I also ran the format checker and found that there was a single line that needed formatting in the ConvertInputFormat code. I hijacked your PR here to add that one-line change. I'm building and testing this locally and we'll let CI run on here and see what happens. |
Unit and integration tests all look fine locally, and regressions appear to be similar to what Decent saw the last time around, so this is hopefully at least neutral with what was here before I merged develop. |
All the diffs are related to the increase in cooling coil capacity due to the higher air density used. The most interesting change is the -0.02% performance hit because StdRhoAir is used to calculate capacity. If the execution time of those test files is accurate then there is likely more time step reduction than before. That is a little hard to believe if the target load or set point is maintained where everything I have looked at says that is true. The additional capacity could cause a different rate of change in zone temperature during setback so it is possible that changing capacity could cause longer runtimes. |
I'm ready to move forward on this. There are lots of diffs, but you have presented them well and you have done this on a PR with very very minimal code changes. This is good stuff. I don't think we really need another round of CI, but I'll build and run unit/integration/regression locally to confirm nothing has changed. @mjwitte any thoughts/concerns about this going in? It will start throwing diffs on other PRs until they pull in develop, so I'm happy to time the merge carefully. |
Everything is happy locally and I can reproduce the CI results. This is now queued for merge, time to speak up if anyone wants me to hold off for timing purposes. |
I could merge this later tonight (11 PM ET) if you like. |
Amazing all the repetitive example files with a slight difference in configuration. Diffs are the same over, and over, and over, and over again. Looks like over 700 example files with only a slight difference to exercize some specific model. I think we could be a little better at modifying existing instead of adding new test files. |
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.
[ ] If any defect files are updated to a more recent version, upload new versions here or on DevSupport[ ] If IDD requires transition, transition source, rules, ExpandObjects, and IDFs must be updated, and add IDDChange label[ ] If structural output changes, add to output rules file and add OutputChange label[ ] If adding/removing any LaTeX docs or figures, update that document's CMakeLists file dependenciesReviewer
This will not be exhaustively relevant to every PR.