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

Fix #6050 - Return Fan:VariableVolume Fan Power Minimum Flow Fraction should not impose air loop flow #8401

Merged
merged 3 commits into from
Dec 11, 2020

Conversation

jmarrec
Copy link
Contributor

@jmarrec jmarrec commented Dec 1, 2020

Pull request overview

The original defect file is on EnergyPlusDevSupport and upgraded to v9.4. I also created a MCVE that exhibits the same behavior. Please see #6050 (comment), but for convenience the AirLoopHVAC in question is here:

https://user-images.githubusercontent.com/5479063/100435961-fcd95680-309e-11eb-991e-491f9c761438.png

Here is a graph showing three cases:

  • Before fix, where the Return Fan's Fan Power Minimum Flow Fraction is 0.0
  • Before fix, where the Return Fan's Fan Power Minimum Flow Fraction is 0.3
  • After fix, where the Return Fan's Fan Power Minimum Flow Fraction is 0.0

You can see that the Before-Frac03 is forcing the flow.

image

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

@jmarrec jmarrec added Defect Includes code to repair a defect in EnergyPlus NotIDDChange Code does not impact IDD (can be merged after IO freeze) labels Dec 1, 2020
@jmarrec jmarrec self-assigned this Dec 1, 2020
@jmarrec jmarrec changed the title Fix #6050 - Do not pass Fan MinAirMassFlowRate to the outlet node's MassFlowRateMin Fix #6050 - Return Fan:VariableVolume Fan Power Minimum Flow Fraction should not impose air loop flow Dec 1, 2020
Comment on lines +1575 to +1587
// Test for #6050 - Return Fan 'Fan Power Minimum Flow Fraction' enforces loop flow, while it should not
// I am simulating a very simple shoebox building with 1 zone.
// There is an airLoopHVAC with a Return Fan:VariableVolume, with a 'Fan Power Minimum Flow Fraction' = 0.3
// The Supply fan has a fraction of zero
//
// The supply side of the loop is has follows: only has a Return Fan, a Mixing box, and a SupplyFan
// o o
// | |
// --- (Return Fan) ---- o-/-o ---- (SupplyFan) ----
// | |
// o o
//
// The Demand side has only one ATU:VAV:NoReheat with a Minimum flow Fraction of 0 so that it doesn't drive the airlfow either
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Gist of the unit test./ This is basically exactly the MCVE I created on EnergyPlusDevSupport.

Comment on lines +1855 to +1863
// NOTE: Terminal has a min flow of 0
"AirTerminal:SingleDuct:VAV:NoReheat,",
" VAV No Rht, !- Name",
" Always On Discrete, !- Availability Schedule Name",
" Zone1 ATU Inlet Node, !- Air Outlet Node Name",
" ATU VAV No Reheat Inlet Node, !- Air Inlet Node Name",
" 1.0, !- Maximum Air Flow Rate {m3/s}",
" Constant, !- Zone Minimum Air Flow Input Method",
" 0; !- Constant Minimum Air Flow Fraction", // IMPORTANT
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Important to set the ATU Min Air Flow Fraction to zero, so that it's not the one driving the flow (I got bit by this initially)

Comment on lines +1918 to +1926
// NOTE: The supply has as a min flow fraction of of 0.3
"Fan:VariableVolume,",
" VSD Return Fan, !- Name",
" Always On Discrete, !- Availability Schedule Name",
" 0.6045, !- Fan Total Efficiency",
" 1017.592, !- Pressure Rise {Pa}",
" 1.0, !- Maximum Flow Rate {m3/s}",
" Fraction, !- Fan Power Minimum Flow Rate Input Method", // IMPORTANT
" 0.3, !- Fan Power Minimum Flow Fraction", // IMPORTANT
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Return Fan with a Fan Power Minimum Flow Fraction of 0.3

Comment on lines +2092 to +2095
EXPECT_EQ(0, DataLoopNode::Node(returnFanNode).MassFlowRateMin);
EXPECT_EQ(0, DataLoopNode::Node(supplyOutletNode).MassFlowRateMin);
EXPECT_EQ(0, DataLoopNode::Node(returnFanNode).MassFlowRate);
EXPECT_EQ(0, DataLoopNode::Node(supplyOutletNode).MassFlowRate);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ensures that the flow rate is indeed zero, and not corresponding to 0.3 Min flow frac

Comment on lines +1110 to +1113
// According to the IO Ref guide:
// "Note that this field is only used to calculate the fan power.
// This field does not enforce the system air flow rate during simulation"
// Node(OutNode).MassFlowRateMin = Fan(FanNum).MinAirMassFlowRate;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fix is here.

@jmarrec
Copy link
Contributor Author

jmarrec commented Dec 2, 2020

regression.5ZoneGeometryTransform

This is a warning about small diffs in EIO. It seems there is a very minimal change in supply airflow rates.

--- 
+++ 
@@ -204,7 +204,7 @@
  Component Sizing Information, Coil:Cooling:DX:TwoSpeed, MAIN COOLING COIL 1, User-Specified Low Speed Gross Rated Sensible Heat Ratio, 0.80000
  Component Sizing Information, Fan:VariableVolume, SUPPLY FAN 1, Design Size Maximum Flow Rate [m3/s], 2.55461
 ! <VAV DX Cooling Coil Standard Rating Information>, DX Coil Type, DX Coil Name, Fan Type, Fan Name, Standard Net Cooling Capacity {W}, Standard Net Cooling Capacity {Btu/h}, IEER {Btu/W-h}, COP 100% Capacity {W/W}, COP 75% Capacity {W/W}, COP 50% Capacity {W/W}, COP 25% Capacity {W/W}, EER 100% Capacity {Btu/W-h}, EER 75% Capacity {Btu/W-h}, EER 50% Capacity {Btu/W-h}, EER 25% Capacity {Btu/W-h}, Supply Air Flow 100% {kg/s}, Supply Air Flow 75% {kg/s},Supply Air Flow 50% {kg/s},Supply Air Flow 25% {kg/s}
- VAV DX Cooling Coil Standard Rating Information, Coil:Cooling:DX:TwoSpeed,MAIN COOLING COIL 1,Fan:VariableVolume,SUPPLY FAN 1,41609.72,141978.25,13.58,2.60,3.51,4.55,5.44,8.89,11.97,15.54,18.55,2.8853,2.2646,1.6075,0.7192,
+ VAV DX Cooling Coil Standard Rating Information, Coil:Cooling:DX:TwoSpeed,MAIN COOLING COIL 1,Fan:VariableVolume,SUPPLY FAN 1,41609.72,141978.25,13.58,2.60,3.51,4.55,5.43,8.89,11.97,15.54,18.54,2.8853,2.2648,1.6076,0.7197,
  Component Sizing Information, Coil:Heating:Fuel, MAIN HEATING COIL 1, Design Size Nominal Capacity [W], 19315.62546
 ! <Environment>,Environment Name,Environment Type, Start Date, End Date, Start DayOfWeek, Duration {#days}, Source:Start DayOfWeek,  Use Daylight Saving, Use Holidays, Apply Weekend Holiday Rule,  Use Rain Values, Use Snow Values, Sky Temperature Model
 ! <Environment:Special Days>, Special Day Name, Special Day Type, Source, Start Date, Duration {#days}

… Flow Rate) to the outlet node's MassFlowRateMin
@jmarrec jmarrec force-pushed the 6050_ReturnFan_MinFlowFrac branch from d45e74f to 89b2d4f Compare December 2, 2020 12:32
@jmarrec
Copy link
Contributor Author

jmarrec commented Dec 2, 2020

The UnitarySystem regression tests that use a Fan:VariableVolume are affected by this change 5Zone_Unitary_HXAssistedCoil, 5Zone_Unitary_VSDesuperheater and 5Zone_Unitary_VSDesuperheatWaterHeater.

5Zone_Unitary_HXAssistedCoil

The Unitary System has a Fan:VariableVolume in it, with does have a non zero minimum flow fraction

  Fan:VariableVolume,
    Supply Fan 1,            !- Name
    FanAvailSched,           !- Availability Schedule Name
    0.7,                     !- Fan Total Efficiency
    600.0,                   !- Pressure Rise {Pa}
    autosize,                !- Maximum Flow Rate {m3/s}
    Fraction,                !- Fan Power Minimum Flow Rate Input Method
    0.25,                    !- Fan Power Minimum Flow Fraction
    ,                        !- Fan Power Minimum Air Flow Rate {m3/s}
    0.9,                     !- Motor Efficiency
    1.0,                     !- Motor In Airstream Fraction
    0.0015302446,            !- Fan Power Coefficient 1
    0.0052080574,            !- Fan Power Coefficient 2
    1.1086242,               !- Fan Power Coefficient 3
    -0.11635563,             !- Fan Power Coefficient 4
    0.000,                   !- Fan Power Coefficient 5
    Mixed Air Node 1,        !- Air Inlet Node Name
    Main Cooling Coil 1 Inlet Node;  !- Air Outlet Node Name

The Loop is serving 5 AirTerminal:SingleDuct:VAV:Reheat which also have a Constant MinimumAirFlowFraction set to 0.1

image

Confirming that the deviation is a result of the Fan Min Flow no longer passed to outlet node

I have taken the file, and produced 8 cases, by varying two variables and the executable used

  • The Fan:VariableVolume 'Fan Power Minimum Flow Fraction': either 0.0 or 0.25
  • The ATU:VAV:Rehead Minimum Airflow Fraction: either 0.1 or 0.0
  • Either with 9.4.0 ('Ori'), or with this branch ('New')

As can be seen in the graphs, the deviation is indeed from the changes here.

image

@mitchute
Copy link
Collaborator

The defect and fix are confirmed. Results look good here. Merging.

@mitchute mitchute merged commit 1cdc5c5 into develop Dec 11, 2020
@mitchute mitchute deleted the 6050_ReturnFan_MinFlowFrac branch December 11, 2020 18:09
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 NotIDDChange Code does not impact IDD (can be merged after IO freeze)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Return Fan:VariableVolume Fan Power Minimum Flow Fraction imposes air loop flow
7 participants