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.

Sign up for GitHub

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

Jump to bottom

Add object-level output variables for ZoneInfiltration #8858

Merged
merged 28 commits into from
Aug 18, 2021
Merged

Add object-level output variables for ZoneInfiltration #8858

merged 28 commits into from
Aug 18, 2021

Conversation

yzhou601
Copy link
Contributor

@yzhou601 yzhou601 commented Jul 1, 2021 •
edited
Loading

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

@yzhou601 yzhou601 changed the title Infiltration reporting Report Infiltration at object level Jul 1, 2021
@yzhou601 yzhou601 added NewFeature Includes code to add a new feature to EnergyPlus OutputChange Code changes output in such a way that it cannot be merged after IO freeze labels Jul 1, 2021
@yzhou601 yzhou601 self-assigned this Jul 1, 2021
@mjwitte
Copy link
Contributor

mjwitte commented Jul 1, 2021

Also, while you are coding this it might be good to do the same for the Ventilation objects.

@yzhou601 yzhou601 added this to the EnergyPlus 9.6 IOFreeze milestone Jul 21, 2021
@yzhou601 yzhou601 changed the title Report Infiltration at object level Add object-level output variables for ZoneInfiltration Jul 21, 2021
yzhou601
yzhou601 commented Jul 29, 2021
View reviewed changes
src/EnergyPlus/DataHeatBalance.hh
Real64 InfilVdotStdDensity; // Volume flow rate of Air {m3/s} due to infiltration standard density (adjusted elevation)
Real64 InfilMdot; // Mass flow rate {kg/s} due to infiltration for reporting
Real64 InfilMass; // Mass of Air {kg} due to infiltration
Real64 InfilAirChangeRate; // Infiltration air change rate {ach}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Add output variables in the data struct.

src/EnergyPlus/HVACManager.cc
@@ -2429,6 +2430,7 @@ void ReportAirHeatBalance(EnergyPlusData &state)
state.dataHVACMgr->ReportAirHeatBalanceFirstTimeFlag = false;
}

ReportInfiltrations(state);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Update object-level reporting variables right before zone level reporting, to ensure consistency

src/EnergyPlus/HVACManager.cc
ZnAirRpt(ZoneLoop).InfilMdot = (state.dataHeatBalFanSys->MCPI(ZoneLoop) / CpAir) * ADSCorrectionFactor;
ZnAirRpt(ZoneLoop).VentilMass =
(state.dataHeatBalFanSys->MCPV(ZoneLoop) / CpAir) * TimeStepSys * DataGlobalConstants::SecInHour * ADSCorrectionFactor;
ZnAirRpt(ZoneLoop).InfilMass = ZnAirRpt(ZoneLoop).InfilMdot * TimeStepSys * DataGlobalConstants::SecInHour;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Minor cleanups.

src/EnergyPlus/ZoneEquipmentManager.cc
@@ -6084,6 +6084,7 @@ void CalcAirFlowSimple(EnergyPlusData &state,
if (state.dataHeatBal->Infiltration(j).QuadratureSum) {
state.dataHeatBal->ZoneAirBalance(state.dataHeatBal->Infiltration(j).OABalancePtr).InfMassFlowRate += MCpI_temp / CpAir;
} else {
state.dataHeatBal->Infiltration(j).MCpI_temp = MCpI_temp;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Store MCpI_temp to object-level data struct for later reporting.

src/EnergyPlus/ZoneEquipmentManager.cc Outdated
@@ -6337,6 +6338,100 @@ void AutoCalcDOASControlStrategy(EnergyPlusData &state)
}
}

void ReportInfiltrations(EnergyPlusData &state)
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 function is created to update object-level reporting data, it follows the same equations used in zone reporting.

@yzhou601 yzhou601 marked this pull request as ready for review July 29, 2021 01:06
@yzhou601
Copy link
Contributor Author

@mjwitte @rraustad This is ready for review. It passes my local tests: 1. multiple infiltration objects serving the same zone; 2. One infiltration object serving a zone list; 3. Infiltration flow rate actuated by an EMS program. Summing up object-level outputs that serve the same zones give the same number as zone-level outputs.

image
image

Wait until CI gives a pass. The AUD and RDD diffs are expected by adding new output vars.

@yzhou601 yzhou601 requested a review from mjwitte July 30, 2021 20:43
jcyuan2020
jcyuan2020 reviewed Aug 6, 2021
View reviewed changes
Copy link
Contributor

@jcyuan2020 jcyuan2020 left a comment

Choose a reason for hiding this comment

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

I think this is a nice addition to the current capability of reporting infiltration outputs at individual level. A few comments and suggestions were added there. But I think the implementation was done quite nicely and it is in a good shape overall.

src/EnergyPlus/ZoneEquipmentManager.cc Outdated Show resolved Hide resolved
src/EnergyPlus/ZoneEquipmentManager.cc Outdated Show resolved Hide resolved
src/EnergyPlus/ZoneEquipmentManager.cc Outdated Show resolved Hide resolved
src/EnergyPlus/ZoneEquipmentManager.cc Outdated Show resolved Hide resolved
src/EnergyPlus/ZoneEquipmentManager.cc Outdated Show resolved Hide resolved
doc/input-output-reference/src/overview/group-airflow.tex Show resolved Hide resolved
tst/EnergyPlus/unit/ZoneEquipmentManager.unit.cc Outdated Show resolved Hide resolved
src/EnergyPlus/ZoneEquipmentManager.cc Outdated Show resolved Hide resolved
src/EnergyPlus/HeatBalanceAirManager.cc Show resolved Hide resolved
@Myoldmopar
Copy link
Member

I pulled this branch and merged develop and it tests just fine. The Windows and Linux CI bot has gone down and I'll need to reboot it, but Mac is now testing fine with the build warning gone. I would be OK with this going in right now, is there any reason to hold up?

@jcyuan2020
Copy link
Contributor

jcyuan2020 commented Aug 18, 2021 •
edited
Loading

I took a final look and if I am correct, the IORef TeX might need a finishing touch---the newly added output variables' description should change from "Zone" to "HVAC" according to the change? @Myoldmopar @yzhou601

@Myoldmopar
Copy link
Member

Pulled develop one more time and built and ran tests fine. I verified the new IO ref changes match the output I see in the eplusout.rdd file for an input file with infiltration. I think this is good to go in. Thanks @yzhou601! Thanks @jcyuan2020 for the reviewing.

@Myoldmopar Myoldmopar merged commit f0ba83b into develop Aug 18, 2021
@Myoldmopar Myoldmopar deleted the infiltration-reporting branch August 18, 2021 19:35
@yzhou601 yzhou601 mentioned this pull request Sep 2, 2021
Closed
10 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rraustad rraustad rraustad left review comments

@jcyuan2020 jcyuan2020 jcyuan2020 left review comments

@mjwitte mjwitte Awaiting requested review from mjwitte

Assignees

@yzhou601 yzhou601

Labels
NewFeature Includes code to add a new feature to EnergyPlus OutputChange Code changes output in such a way that it cannot be merged after IO freeze
Projects
None yet
Milestone
EnergyPlus 9.6 IOFreeze
Development

Successfully merging this pull request may close these issues.

Add object-specific Output Variables for ZoneInfiltration objects
10 participants
@yzhou601 @mjwitte @Myoldmopar @jcyuan2020 @rraustad @nrel-bot @nrel-bot-2 @nrel-bot-3 @nrel-bot-2b @nrel-bot-2c