Unitary system now reports warning when cycling supply fan schedule is used with setpoint control type #8323

matthew-larson · 2020-09-29T18:59:46Z

Pull request overview

Fixes Near zero single speed cooling coil electric power for unitary system with cycling supply fan #8285
This pull request corrects a check for the unitary system based on the Supply Air Fan Operating Mode Schedule when the Control Type is set to Setpoint. Previously, the fan operation mode schedule was not being checked, which was keeping the cooling coil from being enabled.

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

Reviewer

This will not be exhaustively relevant to every PR.

…tary-clgcoil-cycling

rraustad · 2020-09-29T20:17:20Z

src/EnergyPlus/UnitarySystem.cc

+                        DataHVACGlobals::OnOffFanPartLoadFraction = 1.0;
+                    }
+                }
+


Well, this works but now this same code is in 2 places, here and in initLoadBasedControl. Shouldn't these get moved to initUnitarySystems inside the FirstHVACIteration test (line 1084) so that both control types execute this, it's only done once per time step and there is only 1 copy of these lines of code.

That makes sense, I'll make this adjustment once the CI finishes. Thanks

…tary-clgcoil-cycling

matthew-larson · 2020-10-15T16:21:01Z

@rraustad I moved the fix to initUnitarySystems as recommended to eliminate the duplicated check. If you could make sure I got it in the right spot, it's ready for another review.

rraustad · 2020-10-15T17:18:25Z

tst/EnergyPlus/unit/UnitarySystem.unit.cc

+    EXPECT_NEAR(DataLoopNode::Node(2).Temp, DataLoopNode::Node(2).TempSetPoint, 0.001);
+    // cooling coil air inlet node temp is greater than cooling coil air outlet node temp
+    EXPECT_GT(DataLoopNode::Node(3).Temp, DataLoopNode::Node(2).Temp);
+}


Shoot, this was ready to merge. Line feed needed at end of unit test.

rraustad

Revision to code location so load based and set point based control use same logic. This looks good. Unit test still needs mod to last line.

rraustad · 2020-10-15T17:25:55Z

src/EnergyPlus/UnitarySystem.cc

@@ -991,6 +991,15 @@ namespace UnitarySystems {
            DataLoopNode::Node(this->m_HeatRecoveryInletNodeNum).MassFlowRate = mdotHR;
        }

+        if (this->m_FanOpModeSchedPtr > 0) {


Well, I actually suggested this be moved to line 1006, or better yet 1010, below since it needs only be done once per time step. Might as well make that change while you correct the unit test.

…it test.

rraustad · 2020-10-15T17:47:15Z

Perfect, I think I'll run the defect file before merging this. Also, check off the pull request author items in the pull request description. Think about the PR title from the users perspective.

rraustad · 2020-10-15T18:49:50Z

Thanks @matthew-larson. You are done with this one unless something else unexpectedly pops up. I'll get to testing the defect file when I can.

matthew-larson · 2020-10-15T18:58:25Z

Sounds good, thanks for your help and walking me through the code, it was very valuable. I'll keep tabs on this to make sure everything comes back clean.

…tary-clgcoil-cycling

…oint control.

…rol.

matthew-larson · 2020-10-29T19:42:11Z

@rraustad I updated the pull request for the new fix of making sure the correct error is caught when a cycling fan is used with Setpoint control. I also updated the unit test to check for the error. Let me know if you have any additional comments on this.

rraustad · 2020-10-29T20:11:40Z

tst/EnergyPlus/unit/UnitarySystem.unit.cc

+    DataZoneEquipment::ZoneEquipInputsFilled = true;                                    // indicate zone data is available
+    thisSys->getUnitarySystemInputData(state, compName, zoneEquipment, 0, ErrorsFound); // get UnitarySystem input from object above
+    std::string const error_string = delimited_string({
+        "   ** Severe  ** AirLoopHVAC:UnitarySystem = UNITARY SYSTEM MODEL\n   **   ~~~   ** For FAN:ONOFF = SUPPLY FAN 1\n   **   ~~~   ** Fan operating mode must be continuous (fan operating mode schedule values > 0).\n   **   ~~~   ** Error found in Supply Air Fan Operating Mode Schedule Name ALWAYSZERO\n   **   ~~~   ** ...schedule values must be (>0., <=1.)"


@matthew-larson are you clang formatting as you go? It seems this line would have wrapped. Not sure if it's worth another push if everything else is OK.

I have clang formatting ON in CLion. I can double check that it's working right.

rraustad · 2020-11-02T16:53:19Z

This seems correct now.

The DOAS unit uses setpoint control and FanOpMode schedule value = 0

** Severe  ** AirLoopHVAC:UnitarySystem = DOAS UNITARY
**   ~~~   ** For FAN:ONOFF = DOAS UNITARY SUPPLY FAN
**   ~~~   ** Fan operating mode must be continuous (fan operating mode schedule values > 0).
**   ~~~   ** Error found in Supply Air Fan Operating Mode Schedule Name DOAS UNITARY FAN MODE SCHEDULE
**   ~~~   ** ...schedule values must be (>0., <=1.)
**  Fatal  ** getUnitarySystemInputData: previous errors cause termination. Check inputs

If value for FanOpMode schedule is changed to 1 or that input is left blank then there are no fan op mode schedule warnings and the coil turns on:

The check of the FanOpMode schedule for both set point control and load or SZVAV control is not necessary so I'm glad that code was not ultimately moved as discussed. For set point control, we know the fan op mode must be constant so only checking once that the schedule value is > 0 is the only thing needed.

rraustad · 2020-11-02T17:44:44Z

Running unit tests locally multiple times to get a clean pass. Keeps stalling at LoggingSubStep4stepsPerHour. That unit test should not be related to this change.

mitchute · 2020-11-02T17:57:33Z

Keeps stalling at LoggingSubStep4stepsPerHour

@rraustad, FWIW, that test usually takes a few minutes to complete on my machine.

rraustad · 2020-11-02T18:27:48Z

Yeah, that unit test finally finished after 15 minutes, using a release build.

rraustad

Simple changes made to catch issues with fan operating mode schedule when set point control is used.

mitchute · 2020-11-02T19:15:29Z

15 minutes... 😮

matthew-larson added 6 commits September 17, 2020 14:08

Add if statement for SolveRoot method and PartLoadRatio calculation.

0ed4d98

Revert code change and add unit test for issue.

0e3b305

Merge branch 'develop' of https://github.com/nrel/energyplus into uni…

2be3bd9

…tary-clgcoil-cycling

Modify fix location and unit test.

4f7867b

Merge branch 'develop' of https://github.com/nrel/energyplus into uni…

0720d0b

…tary-clgcoil-cycling

Merge branch 'develop' of https://github.com/nrel/energyplus into uni…

103f793

…tary-clgcoil-cycling

matthew-larson added Defect Includes code to repair a defect in EnergyPlus NotIDDChange Code does not impact IDD (can be merged after IO freeze) labels Sep 29, 2020

matthew-larson requested a review from nealkruis September 29, 2020 18:59

matthew-larson self-assigned this Sep 29, 2020

rraustad reviewed Sep 29, 2020

View reviewed changes

matthew-larson added 2 commits September 30, 2020 10:15

Merge branch 'develop' of https://github.com/nrel/energyplus into uni…

f44e97a

…tary-clgcoil-cycling

Merge fan operating mode if statement into InitUnitarySystems.

cfbb9fc

matthew-larson marked this pull request as ready for review September 30, 2020 18:55

Merge branch 'develop' of https://github.com/nrel/energyplus into uni…

ab1959d

…tary-clgcoil-cycling

rraustad reviewed Oct 15, 2020

View reviewed changes

rraustad requested changes Oct 15, 2020

View reviewed changes

rraustad reviewed Oct 15, 2020

View reviewed changes

Move code fix to first iteration only and add line feed and end of un…

66cbd81

…it test.

matthew-larson changed the title ~~Single speed cooling coil electric power for unitary system with cycling supply fan~~ Fix enabling of single speed cooling coil for unitary system with cycling supply fan and setpoint control type Oct 15, 2020

Fix unit test from develop changes.

5bb0670

matthew-larson added 3 commits October 28, 2020 06:46

Merge branch 'develop' of https://github.com/nrel/energyplus into uni…

1b8e318

…tary-clgcoil-cycling

Change fix to provide correct error message for cycling fan with setp…

9d2ed69

…oint control.

Update unit test for error checking of cycling fan with setpoint cont…

2138fb1

…rol.

rraustad reviewed Oct 29, 2020

View reviewed changes

matthew-larson requested a review from rraustad November 2, 2020 15:30

rraustad approved these changes Nov 2, 2020

View reviewed changes

rraustad changed the title ~~Fix enabling of single speed cooling coil for unitary system with cycling supply fan and setpoint control type~~ Unitary system now reports warning when cycling supply fan schedule is used with setpoint control type Nov 2, 2020

rraustad merged commit ddd1a11 into NREL:develop Nov 2, 2020

matthew-larson deleted the unitary-clgcoil-cycling branch November 2, 2020 18:53

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Unitary system now reports warning when cycling supply fan schedule is used with setpoint control type #8323

Unitary system now reports warning when cycling supply fan schedule is used with setpoint control type #8323

matthew-larson commented Sep 29, 2020 •

edited by rraustad

Loading

rraustad Sep 29, 2020

matthew-larson Sep 29, 2020

matthew-larson commented Oct 15, 2020

rraustad Oct 15, 2020

rraustad left a comment

rraustad Oct 15, 2020

rraustad commented Oct 15, 2020

rraustad commented Oct 15, 2020

matthew-larson commented Oct 15, 2020

matthew-larson commented Oct 29, 2020

rraustad Oct 29, 2020

matthew-larson Oct 29, 2020

rraustad commented Nov 2, 2020

rraustad commented Nov 2, 2020

mitchute commented Nov 2, 2020

rraustad commented Nov 2, 2020

rraustad left a comment

mitchute commented Nov 2, 2020

Unitary system now reports warning when cycling supply fan schedule is used with setpoint control type #8323

Unitary system now reports warning when cycling supply fan schedule is used with setpoint control type #8323

Conversation

matthew-larson commented Sep 29, 2020 • edited by rraustad Loading

Pull request overview

Pull Request Author

Reviewer

rraustad Sep 29, 2020

Choose a reason for hiding this comment

matthew-larson Sep 29, 2020

Choose a reason for hiding this comment

matthew-larson commented Oct 15, 2020

rraustad Oct 15, 2020

Choose a reason for hiding this comment

rraustad left a comment

Choose a reason for hiding this comment

rraustad Oct 15, 2020

Choose a reason for hiding this comment

rraustad commented Oct 15, 2020

rraustad commented Oct 15, 2020

matthew-larson commented Oct 15, 2020

matthew-larson commented Oct 29, 2020

rraustad Oct 29, 2020

Choose a reason for hiding this comment

matthew-larson Oct 29, 2020

Choose a reason for hiding this comment

rraustad commented Nov 2, 2020

rraustad commented Nov 2, 2020

mitchute commented Nov 2, 2020

rraustad commented Nov 2, 2020

rraustad left a comment

Choose a reason for hiding this comment

mitchute commented Nov 2, 2020

matthew-larson commented Sep 29, 2020 •

edited by rraustad

Loading