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

Airflow Network Default Behavior Enhancements #8454

Merged
merged 8 commits into from
Apr 30, 2021
Merged

Airflow Network Default Behavior Enhancements #8454

merged 8 commits into from
Apr 30, 2021

Conversation

lgu1234
Copy link
Contributor

@lgu1234 lgu1234 commented Jan 8, 2021 •
edited
Loading

Pull request overview

Although the issue provides two possible options, I would like to propose the logic as show below:

If (NoExist AirflowNetwork:SimulationControl) && (Exist AirflowNetwork:MultiZone:Zone & AirflowNetwork:MultiZone:Surface) Then

Default all fields of AirflowNetwork::SimulationControl and issue a warning to make the default 
Let the program catches other possible errors

Else

Bypass, because eio provides information on AirflowNetwork Model:Control,NoMultizoneOrDistribution

End If

The justification is that the objects of AirflowNetwork:MultiZone:Zone and AirflowNetwork:MultiZone:Surface are two main objects for AFN model to work. If both exist, the default of AirflowNetwork:SimulationControl makes sense. Otherwise, it assumes no AFN simulations. Therefore, there is no need to generate any warnings. In addition, the eio provide information below:

! <AirflowNetwork Model:Control>, No Multizone or Distribution/Multizone with Distribution/Multizone without Distribution/Multizone with Distribution only during Fan Operation
AirflowNetwork Model:Control,NoMultizoneOrDistribution

The new feature request is GitHub Issue #8010 as AirflowNetwork objects not used without AFN:SimControl object and no error/warning message #8010

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

@lgu1234
Copy link
Contributor Author

lgu1234 commented Jan 8, 2021

This new feature is one of FSEC tasks. Please let me know your comments and concerns.

@lgu1234 lgu1234 added the NewFeature Includes code to add a new feature to EnergyPlus label Jan 8, 2021
@jasondegraw
Copy link
Member

@lgu1234 This makes sense to me.

@lgu1234
Copy link
Contributor Author

lgu1234 commented Jan 25, 2021

@jasondegraw I upload Design Document. Please review. Thanks.

@nrel-bot-3
Copy link

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

@Myoldmopar Myoldmopar added this to the EnergyPlus Future milestone Mar 1, 2021
@nrel-bot-2b
Copy link

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

lgu1234
lgu1234 commented Apr 6, 2021
View reviewed changes
src/EnergyPlus/AirflowNetwork/include/AirflowNetwork/Elements.hpp
@@ -1659,6 +1659,7 @@ struct AirflowNetworkData : BaseGlobalStruct
Real64 ExhaustFanMassFlowRate = 0.0; // Exhaust fan flow rate used in PressureStat
int PressureSetFlag = 0; // PressureSet flag
Real64 ReliefMassFlowRate = 0.0; // OA Mixer relief node flow rate used in PressureStat
bool AFNDefaultControlFlag = false; // Default simulation control flag
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 a bool variable to determine whether AFN default object of AirflowNetwork:SimulationControl is needed or not

src/EnergyPlus/AirflowNetworkBalanceManager.cc
SimAirNetworkKey = "MultizoneWithoutDistribution";
state.dataAirflowNetwork->AirflowNetworkSimu.InitFlag = 1;
ShowWarningError(state, RoutineName + CurrentModuleObject + " object is not found ");
ShowContinueError(state, "..The default behaviour values are assigned. Please see details in Input Output Reference.");
Copy link
Contributor Author

Choose a reason for hiding this comment

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

If there is no AirflowNetwork:SimulationControl object and Number of AirflowNetwork:MultiZone:Zone >= 1 and number of AirflowNetwork:MultiZone:Surface >=2, the default values of AirflowNetwork:SimulationControl are created internally.

src/EnergyPlus/AirflowNetworkBalanceManager.cc
state.dataAirflowNetwork->SimulateAirflowNetwork = AirflowNetworkControlSimple;
print(state.files.eio, Format_110);
print(state.files.eio, Format_120, "NoMultizoneOrDistribution");
return;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Otherwise, the original code is executed.

src/EnergyPlus/AirflowNetworkBalanceManager.cc
"..specified in " + CurrentModuleObject + ' ' + cAlphaFields(1) + " = " +
state.dataAirflowNetwork->AirflowNetworkSimu.AirflowNetworkSimuName);
ErrorsFound = true;
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

If state.dataAirflowNetwork->AFNDefaultControlFlag = false, the original code section to read AirflowNetwork:SimulationControl object is executed.

src/EnergyPlus/AirflowNetworkBalanceManager.cc
UtilityRoutines::SameString(state.dataAirflowNetwork->AirflowNetworkSimu.HeightOption, "OpeningHeight"))) {
ShowSevereError(state, RoutineName + CurrentModuleObject + " object, " + cAlphaFields(4) + " = " + Alphas(4) + " is invalid.");
if (!state.dataAirflowNetwork->AFNDefaultControlFlag) {
if (UtilityRoutines::SameString(state.dataAirflowNetwork->AirflowNetworkSimu.WPCCntr, "Input")) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

If state.dataAirflowNetwork->AFNDefaultControlFlag = false, keep original code execution.

@lgu1234
Copy link
Contributor Author

lgu1234 commented Apr 6, 2021

This file is used as a reference for comparison.
AirflowNetwork3zVentAutoWPCTest.txt
The object of AirflowNetwork:SimulationControl is commented out. This is file is used to test default behavior.
AirflowNetwork3zVentAutoWPCTest_new.txt

Both files should have same csv outputs.

jasondegraw
jasondegraw approved these changes Apr 28, 2021
View reviewed changes
Copy link
Member

@jasondegraw jasondegraw left a comment

Choose a reason for hiding this comment

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

I made a couple of small changes to the documentation, looks ready to go to me.

@Myoldmopar
Copy link
Member

It's a little behind develop. I think it would be all green if develop was merged in. Since it looks otherwise ready, I'll go ahead and do that now.

@Myoldmopar
Copy link
Member

Code changes look good, unit test and documentation in place, approved, and CI system all happy. This is good to go in. Thanks @lgu1234 and @jasondegraw !

@Myoldmopar Myoldmopar merged commit 05b0b39 into develop Apr 30, 2021
@Myoldmopar Myoldmopar deleted the AFN-Default-Behavior-Enhancements branch April 30, 2021 13:37
@matthew-larson matthew-larson mentioned this pull request Jun 10, 2021
Closed
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jasondegraw jasondegraw jasondegraw approved these changes

Assignees
No one assigned
Labels
NewFeature Includes code to add a new feature to EnergyPlus
Projects
None yet
Milestone
EnergyPlus 9.6 Release
Development

Successfully merging this pull request may close these issues.
8 participants
@lgu1234 @jasondegraw @nrel-bot-3 @nrel-bot-2b @Myoldmopar @nrel-bot @nrel-bot-2 @nrel-bot-2c