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

Consolidate code in CorrectZoneHumRat #6415

Merged
merged 6 commits into from
Jan 19, 2018

Conversation

lgu1234
Copy link
Contributor

@lgu1234 lgu1234 commented Dec 11, 2017

Pull request overview

Addresses #6233. Remove a surplus section of code for ZoneMassFlowRate <= 0.0 in moisture balance equation. The coefficient calculations in moisture balance equation are consistent with energy balance equation.

Small differences are found for an example file of GSHP-GLHE-CalcGFunctions.idf in local computer runs.

Work Checklist

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)
  • At least one of the following appropriate labels must be added to this PR to be consumed into the changelog:
    • Defect: This pull request repairs a github defect issue. The github issue should be referenced in the PR description
    • Refactoring: This pull request includes code changes that don't change the functionality of the program, just perform refactoring
    • NewFeature: This pull request includes code to add a new feature to EnergyPlus
    • Performance: This pull request includes code changes that are directed at improving the runtime performance of EnergyPlus
    • DoNoPublish: This pull request includes changes that shouldn't be included in the changelog

Review Checklist

This will not be exhaustively relevant to every PR.

  • Code style (parentheses padding, variable names)
  • Functional code review (it has to work!)
  • n/a If defect, results of running current develop vs this branch should exhibit the fix
  • CI status: all green or justified
  • Performance: CI Linux results include performance check -- verify this
  • Unit Test(s)

@mjwitte mjwitte added the Defect Includes code to repair a defect in EnergyPlus label Dec 11, 2017
@nrel-bot-3
Copy link

@lgu1234 @lgentile it has been 14 days since this pull request was last updated.

Copy link
Contributor

@mjwitte mjwitte left a comment

Choose a reason for hiding this comment

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

@lgu1234 Where is the equivalent section for the energy balance? Do I understand correctly that this is simply a code cleanup and there should be no diffs?
I merged in develop to bring this branch up to date.

A = AirflowNetworkExchangeData( ZoneNum ).SumMHr + AirflowNetworkExchangeData( ZoneNum ).SumMMHr + SumHmARa( ZoneNum );
}
C = RhoAir * Zone( ZoneNum ).Volume * Zone( ZoneNum ).ZoneVolCapMultpMoist / SysTimeStepInSeconds;
ZoneOABal = MDotOA( ZoneNum );
Copy link
Contributor

Choose a reason for hiding this comment

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

What is MDotOA? I only see it used in the moisture prediction/correction, but I don't find it in the temperature calculations.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mjwitte MDotOA is defined as Airbalance MASS FLOW rate in DataHeatBalFanSys, The equivalent item of MDotOA in the temperature calculation is MDotCPOA defined as Airbalance MASS FLOW * AIR SPECIFIC HEAT.

In order to avoid large differences in an example file, the following two lines of correction are used:

	ZoneOABal = MDotOA( ZoneNum );
	if ( ZoneMassFlowRate <= 0.0 ) ZoneOABal = 0.0;

However, the temperature calculation uses MDotCPOA as is without modification. Therefore, in order to be consistent with energy calculation, the correct may be removed.

Copy link
Contributor

Choose a reason for hiding this comment

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

@lgu1234 I still don't understand what "Airbalance mass flow rate" is. Where does it come from?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mjwitte Let me dig deeper to find out "MDotOA".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mjwitte "MDotOA" is calculated with Air Balance Method = Quadrature in the ZoneAirBalance:OutdoorAir. It should be independent of ZoneMassFlowRate and calculated every time step. Therefore, the code in temperature calculation is correct, while the following two lines should be removed in moisture calculation, even though large differences are observed in SingleFamilyHouse_TwoSpeed_ZoneAirBalance.idf:

    ZoneOABal = MDotOA( ZoneNum );
if ( ZoneMassFlowRate <= 0.0 ) ZoneOABal = 0.0; 

Copy link
Contributor

Choose a reason for hiding this comment

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

@lgu1234 Thanks. I forgot about that feature. I would be good to expand the comment for MDotOA where it's declared. Please go ahead and make this correction and remove the unused variables than I'll give it one final review.

@@ -4682,6 +4682,7 @@ namespace ZoneTempPredictorCorrector {
int ADUNum;
int ADUInNode;
int ADUOutNode;
Real64 ZoneOABal;
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like ExhMassFlowRate and TotExitMassFlowRate (a few lines up) are not used for anything. Can these be deleted?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mjwitte If you agree with my changes, I will remove these two variables: ExhMassFlowRate and TotExitMassFlowRate.

I also notice that this issue also happens in CorrectZoneContaminants of the ZoneContaminantPredictorCorrector module.

@mjwitte mjwitte changed the title 153522005 remove a section with zone mass flow rate Consolidate code in CorrectZoneHumRat Jan 10, 2018
@lgu1234
Copy link
Contributor Author

lgu1234 commented Jan 11, 2018

@mjwitte The equivalent section is located in CalcZoneSums. Theoretically, the code after cleanup should not show differences. However, some calculations entered the section with MassFlowRate <=0. This should not happen. Therefore, if we use consistent code in both moisture and temperature, we may see some differences, but the revision is correct.

@lgu1234
Copy link
Contributor Author

lgu1234 commented Jan 16, 2018

@mjwitte I uploaded the branch a while ago. Here is what I have done:

  1. Remove two variables: ExhMassFlowRate and TotExitMassFlowRate
  2. Remove a section to define MDotOA to be consistent with temperature calculation

Test run shows differences in SingleFamilyHouse_TwoSpeed_ZoneAirBalance as expected.

It is ready for final review.

@mjwitte mjwitte merged commit 8346d7a into develop Jan 19, 2018
@mjwitte mjwitte deleted the 153522005-Remove-a-section-with-ZoneMassFlowRate branch January 19, 2018 20:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Defect Includes code to repair a defect in EnergyPlus
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants