Skip to content
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

NFP-Zone Air Mass Flow Balance Improvement #8460

Merged
merged 49 commits into from
Feb 19, 2021

Conversation

Nigusse
Copy link
Contributor

@Nigusse Nigusse commented Jan 11, 2021

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.

  • Title of PR should be user-synopsis style (clearly understandable in a standalone changelog context)
  • Label the PR with at least one of: Defect, Refactoring, NewFeature, Performance, and/or DoNoPublish
  • Pull requests that impact EnergyPlus code must also include unit tests to cover enhancement or defect repair
  • Author should provide a "walkthrough" of relevant code changes using a GitHub code review comment process
  • If any diffs are expected, author must demonstrate they are justified using plots and descriptions
  • If changes fix a defect, the fix should be demonstrated in plots and descriptions
  • 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 dependencies

Reviewer

This will not be exhaustively relevant to every PR.

  • Perform a Code Review on GitHub
  • If branch is behind develop, merge develop and build locally to check for side effects of the merge
  • If defect, verify by running develop branch and reproducing defect, then running PR and reproducing fix
  • If feature, test running new feature, try creative ways to break it
  • CI status: all green or justified
  • Check that performance is not impacted (CI Linux results include performance check)
  • Run Unit Test(s) locally
  • Check any new function arguments for performance impacts
  • Verify IDF naming conventions and styles, memos and notes and defaults
  • If new idf included, locally check the err file and other outputs

@Nigusse Nigusse added the NewFeature Includes code to add a new feature to EnergyPlus label Jan 11, 2021
@Nigusse Nigusse added the IDDChange Code changes impact the IDD file (cannot be merged after IO freeze) label Jan 19, 2021
@Nigusse
Copy link
Contributor Author

Nigusse commented Jan 20, 2021

This NFP is ready for review.

The zone is under negative pressure and requires additional supply air flow to balance the zone air flow. In this case the return air mass flow rate will be reset to zero and the supply air flow rate is increased proportionally.
[m_{ret} = 0.0;]
[m_{sup} += m_{ret};]

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How are constant volume HVAC systems going to respond to these 2 cases? I assume OA will make up the difference between supply and return flow rates. What happens when there is no OA system? In that case the supply and return flows should match.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am assuming that the two cases should work for constant volume and VAV systems and have to confirmed through testing.

Copy link
Contributor Author

@Nigusse Nigusse Jan 21, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Regarding "What happens when there is no OA system?", I have not thought enough if it needs a special treatment. One issue is that supply and return flows may not match for zones that has exhaust fan flow, ZoneMixing and Infiltration flows. You have to adjust either the supply, infiltration, or the return flows as needed.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. For systems with no outdoor air, the existing return flow calculations force the return to match the corresponding supply flow for a given zone. This balance must be maintained.
  2. For constant volume systems, are you suggesting that the system flow could be increased by this? That's going to run up against the terminal unit max flow rate.
  3. For VAV systems, increasing the supply flow could cause temperature control problems.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suspected I may run into control problem when the supply air flow rates is altered. In some cases return air can be zero and we may not have other variable to change unless infiltration air flow rate is adjusted instead of supply air. In fact my original NFP draft was to adjust return air and infiltration to balance the flow while keeping the ZoneMixing object flow as specified and letting the control system deal with supply air flow rate.

I would rather adjust return and infiltration air flow rates only to balance the system and let the control system determine the supply air flow rate. What do you think? @mjwitte

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That seems safer, but that question should be answered by those who are requesting this feature and the specific use cases that it will be applied to.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, I will seek input from those requested the feature.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Regarding Case II here.
For VAV terminals, it does seem possible to consider exhaust fan flows when determining damper position, but that is beyond scope I think.

I think when return air wants to go negative there is a sizing or input consistency problem. The user should have set the ventilation requirement in DesignSpecification:OutdoorAir object high enough to supply make up air for exhaust fan and mixed air transfers. For Case II, I would say don't do anything beyond setting return air to zero. The existing unbalanced flow warning should catch this okay.

Comment on lines 49 to 51
Case I: [m_{ret} >= 0.0]

The zone is under positive pressure and requires return air mass flow rate to balance the zone air flow. The return air flow rate will be adjusted within the bounds of 0 and maximum return air flow rate to balance the zone air mass flow.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What determines "maximum return air flow rate"?
How will this work when there are multiple return nodes (more than one AHU serving a zone)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am thinking the maximum will be determined based on the the Node's maximum Mass Flow Rate.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am thinking to use the same logic for all return nodes connected to the same zone. Not sure if this causes any problem.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think return nodes have a defined maximum mass flow rate, but I may be wrong. The only value that might be used would be the overall airloop design flow rate, which will defeat the purpose of this feature when it wants to set return flow > supply.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My thought is it can set zone return flow > zone supply flow but the AirLoop overall return cannot be set > the overall airloop design flow rate.

The zone is under negative pressure and requires additional supply air flow to balance the zone air flow. In this case the return air mass flow rate will be reset to zero and the supply air flow rate is increased proportionally.
[m_{ret} = 0.0;]
[m_{sup} += m_{ret};]

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. For systems with no outdoor air, the existing return flow calculations force the return to match the corresponding supply flow for a given zone. This balance must be maintained.
  2. For constant volume systems, are you suggesting that the system flow could be increased by this? That's going to run up against the terminal unit max flow rate.
  3. For VAV systems, increasing the supply flow could cause temperature control problems.


## Justification for New Feature ##

ZoneAirMassFlowConservation is intended to balance airflows but current design is that the procedure adjusts zone mixing and/or infiltration rates. User that have complex air transfer models like grocery stores, restaurants, and some big box retailers need the ability to model actual air transfer and account for ventilation movement between spaces. In these cases, users want the infiltration, zone mixing, and exhaust fan flow rates to stay fixed at user specified flows, instead adjust the system return air to balance zone air flow.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How does the proposed method model or give user control over transfer air?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Users will specify transfer air using ZoneMixing objects and these flow rates remain the same.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, but with this method, the ZoneMixing flows will occur no matter what the system flows are. It will be up to the user to schedule the ZoneMixing flows to match the HVAC system operation.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that is my plan.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will update the NFP based on the feedback and seek input from those requested the feature. @mjwitte and @rraustad Thank you for your review and feedback.

m_{zmreceiving} = Zone Mixing Receiving Air Mass Flow Rate, [kg/s]
m_{zmsource} = Zone Mixing Source Air Mass Flow Rate, [kg/s]
m_{inf} = Zone Infiltration Air Mass Flow Rate, [kg/s]

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My view is that zone Infiltration is modeled as balanced infiltration and exfiltration and it does not necessarily always add mass to the zone balance. (Exfiltration is ignored in zone heat balance because it doesn't affect the energy balance since leaving the control volume.) But when needed for air balancing it can add mass by zeroing out the exfiltration. So the term m_{inf} needs a logic modifier, if in Case I (below), term doesn't count (x 0.0), but if needed to stay out of Case II (below) then some part [0.0 ..1.0] of the infiltration term can add mass thru exfiltration balancing.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Infiltration is assumed self-balanced only if the "ZoneAirMassFlowConservation" is not active. If the ZoneAirMassFlowConservation is ON, current zone mass flow balance calculation assumes exfiltration is always zero.

OK, but with this method, the ZoneMixing flows will occur no matter what the system flows are. It will be up to the user to schedule the ZoneMixing flows to match the HVAC system operation.
@Nigusse, Response:
Yes, that is my plan.

Copy link
Contributor

@EnergyArchmage EnergyArchmage Jan 28, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe an entirely different way to go would be to add a new version of ZoneMixing that works as system air with input for two air system nodes. one inlet from the exhaust node of the source zone, and one outlet which is a zone inlet node for the receiving zone. Users that have 90.1 air transfers that need to be balanced with system use the new approach. Legacy ZoneMixing can still be used for load calcs with no HVAC.

@Myoldmopar
Copy link
Member

Let me know if you'd still like a meeting organized on this.

@EnergyArchmage @mjwitte @EnergyArchmage @rraustad @nagappanchidambaram

@mjwitte
Copy link
Contributor

mjwitte commented Feb 5, 2021

@Nigusse This has no milestone. Is it intended for v9.5?

@Nigusse Nigusse added this to the EnergyPlus 9.5.0 milestone Feb 8, 2021
@Nigusse
Copy link
Contributor Author

Nigusse commented Feb 9, 2021

The two example files that have the ZoneAirMassFlowConservation object throw the following EIO warning error message:

- Zone Air Mass Flow Balance Simulation, Yes,Yes,AddInfiltrationFlow,MixingSourceZonesOnly
+ Zone Air Mass Flow Balance Simulation, Yes,AdjustZoneMixingFlow,AddInfiltrationFlow,MixingSourceZonesOnly

This is because of change in the IDD and is expected.

@mjwitte mjwitte removed this from the EnergyPlus 9.5.0 milestone Feb 9, 2021
Copy link
Contributor Author

@Nigusse Nigusse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Provided a walk throug of the code changes.

\key AdjustMixingThenReturn
\key AdjustReturnThenMixing
\key None
\default None
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is modified ZoneAirMassFlowConservation object that allows to a different options of enforcing zone air flow balance. Please check the PR for details of comments. We have added three new options now.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tab characters that I will fix right before merging.

int const AdjustReturnOnly(2);
int const AdjustMixingThenReturn(3);
int const AdjustReturnThenMixing(4);
int const NoAdjustReturnAndMixing(5);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Defined five parameters for the different zone air flow balancing enforcing options. The default is NoAdjustReturnAndMixing and will be initialized to "0" instead of "5". I have corrected it and will push the branch later.

@@ -1358,13 +1365,14 @@ namespace DataHeatBalance {
{
// Members
bool EnforceZoneMassBalance; // flag to enforce zone air mass conservation
bool BalanceMixing; // flag to allow mixing to be adjusted for zone mass balance
int ZoneFlowAdjustment; // specifies how zone air flow balance is determined (AdjustMixingOnly, AdjustReturnOnly, AdjustMixingThenReturn, AdjustReturnThenMixing, None)
int InfiltrationTreatment; // determines how infiltration is treated for zone mass balance
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added integer member variable that the maps to the different enforcing options. The existing method "BalanceMixing" is replaced with "AdjustMixingOnly".

int InfiltrationTreatment; // determines how infiltration is treated for zone mass balance
int InfiltrationZoneType; // specifies which types of zones allow infiltration to be changed
// Note, unique global object

// Default Constructor
ZoneAirMassFlowConservation() : EnforceZoneMassBalance(false), BalanceMixing(false), InfiltrationTreatment(0), InfiltrationZoneType(0)
ZoneAirMassFlowConservation()
: EnforceZoneMassBalance(false), ZoneFlowAdjustment(1), InfiltrationTreatment(0), InfiltrationZoneType(0)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated the struct using the new member variable that maps to the different options and deleted the existing method. The default method initialization will be corrected to "0".

@@ -316,7 +316,7 @@ namespace HeatBalanceAirManager {

// flow

if (ZoneAirMassFlow.EnforceZoneMassBalance && ZoneAirMassFlow.BalanceMixing) {
if (ZoneAirMassFlow.EnforceZoneMassBalance && ZoneAirMassFlow.ZoneFlowAdjustment != DataHeatBalance::NoAdjustReturnAndMixing) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This initialization now applies to all four methods except the "None" option.

@@ -604,7 +604,7 @@ TEST_F(EnergyPlusFixture, HeatBalanceManager_ZoneAirMassFlowConservationData3)
GetProjectControlData(*state, ErrorsFound); // returns ErrorsFound false, ZoneAirMassFlowConservation never sets it
EXPECT_FALSE(ErrorsFound);
EXPECT_FALSE(ZoneAirMassFlow.EnforceZoneMassBalance);
EXPECT_FALSE(ZoneAirMassFlow.BalanceMixing);
EXPECT_EQ(ZoneAirMassFlow.ZoneFlowAdjustment, NoAdjustReturnAndMixing);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

transitioned a unit test.

@@ -624,7 +624,7 @@ TEST_F(EnergyPlusFixture, HeatBalanceManager_ZoneAirMassFlowConservationReportVa
"25, !- Maximum Number of Warmup Days",
"6; !- Minimum Number of Warmup Days",
"ZoneAirMassFlowConservation,",
"Yes, !- Adjust Zone Mixing For Zone Air Mass Flow Balance",
"AdjustMixingOnly, !- Adjust Zone Mixing and Return For Air Mass Flow Balance",
"AdjustInfiltrationFlow, !- Infiltration Balancing Method",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

transitioned a unit test.

@@ -1938,7 +1938,7 @@ TEST_F(EnergyPlusFixture, HeatBalanceManager_HVACSystemRootFindingAlgorithmBisec
"25, !- Maximum Number of Warmup Days",
"6; !- Minimum Number of Warmup Days",
"ZoneAirMassFlowConservation,",
"No, !- Adjust Zone Mixing For Zone Air Mass Flow Balance",
"None, !- Adjust Zone Mixing and Return For Air Mass Flow Balance",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

transitioned a unit tests.

\textbf{AdjustMixingThenReturn:} first adjusts the zone mixing air mass flow rates, then adjusts the zone total return air mass flow rate to enforce zone air mass flow balance. Infiltration air flow can also adjusted if required depending on user preference as specified in section \textit{Infiltration Flow rates Adjustments}. For adjusting the mixing mass flow rates the set of equations for \textit{AdjustMixingOnly} method described above are used and for adjusting the zone total return air mass flow rate the equation for \textit{AdjustReturnOnly} method is used.

\textbf{AdjustReturnThenMixing:} first adjusts the zone total return air mass flow rate, then adjusts the zone mixing mass flow rates to enforce zone air mass flow balance. Infiltration air flow can also adjusted if required depending on user preference as specified in section \textit{Infiltration Flow rates Adjustments}. For adjusting the zone total return air mass flow rates the equations for \textit{AdjustReturnOnly} method described above is used and for adjusting the zone mixing mass flow rates the set of equations for \textit{AdjustMixingOnly} method are used.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated the engineering reference to reflect the changes as applicable for each of the zone air flow conservation enforcing methods.

@@ -175,7 +229,7 @@ \subsection{Infiltration Flow Rate Adjustments}\label{infiltration-flow-rate-adj
If a zone is included in the infiltration adjustment, the infiltration air mass flow rate required to balance the zone is determined as follows:

\begin{equation}
{\dot m_{Inf-required}} = MAX\left( {0.0,\,{{\dot m}_{XS}} + {{\dot m}_{EX}} + {{\dot m}_{R}} - {{\dot m}_S}} \right)
{\dot m_{Inf-required}} = MAX\left( {0.0,\,{{\dot m}_{XS}} + {{\dot m}_{EX}} + {{\dot m}_{R}} - {{\dot m}_S} - {\dot m_{XR,\,new}}} \right)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a missing terms in the air air infiltration adjusting or adding calculation option. This correction allows infiltration treatment for receiving zone as well.

@Myoldmopar
Copy link
Member

This has conflicts now from the merge of #8498. I don't have the bandwidth to address them right now, so please take care of that. @Nigusse

@Nigusse
Copy link
Contributor Author

Nigusse commented Feb 18, 2021

OK. I am going to start resolving the conflict now.

@Nigusse
Copy link
Contributor Author

Nigusse commented Feb 18, 2021

Merged in develop, resolved conflicts, and ran unit tests and all passed.

@Myoldmopar
Copy link
Member

Thanks for getting this all fixed up @Nigusse ! Pulling this and testing it now.

@Nigusse
Copy link
Contributor Author

Nigusse commented Feb 19, 2021

@Myoldmopar The EIO and tabular column heading differences are expected and are due to IDD changes.

@Myoldmopar
Copy link
Member

Yeah, this looks good and tests well locally. This will merge now. Thanks @Nigusse, please open a follow-up issue if needed and we can still chat about this on the technical call next week.

Copy link
Member

@Myoldmopar Myoldmopar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, well I assumed it was all ready but it looks like it still needs transition rules to be written. I won't be doing that tonight. I can tomorrow.

\key AdjustMixingThenReturn
\key AdjustReturnThenMixing
\key None
\default None
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tab characters that I will fix right before merging.

extern int const AdjustReturnOnly;
extern int const AdjustMixingThenReturn;
extern int const AdjustReturnThenMixing;
extern int const NoAdjustReturnAndMixing;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmmm I wish this had been an enumeration rather than more global integers hanging out. Oh well.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will change to enumeration along with other follow up changes.

- Now there are five choice keys.
Fields 2-3 remain the same.

See [pull request 8460] (https://github.com/NREL/EnergyPlus/pull/8460)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wait, this doesn't have the Fortran transition implemented yet?!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, I have not done the transition rule FORTRAN code change.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added the FORTRAN transition rules, tested locally and pushed the updated branch.

@Myoldmopar
Copy link
Member

@Nigusse thanks! I was just going to sit down and write the transition rules. I'll build them and get it tested out.

@Myoldmopar
Copy link
Member

CI diffs are OK. Locally with latest develop, unit tests are running fine. Transitioned up a 9.4 file with no problem, and ran it with a fully clean error file. This is great. I'm calling this done. Thanks @Nigusse !

@Myoldmopar Myoldmopar merged commit faa0a4b into develop Feb 19, 2021
@Myoldmopar Myoldmopar deleted the 176230951_AirTransferModel branch February 19, 2021 19:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
IDDChange Code changes impact the IDD file (cannot be merged after IO freeze) NewFeature Includes code to add a new feature to EnergyPlus
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants