Support Electric Heating Coil with ASHRAE90VariableFan Capacity Control #8101

Nigusse · 2020-06-22T20:21:12Z

Pull request overview

Fixes ZoneHVAC:FourPipeFanCoil with electric heating coil and ASHRAE90VariableFan is not working #8087
Added a control logic to support fancoil units with electric heating coil and ASHRAE90VariableFan capacity control method.
Diffs are expected for fan coil units with electric heating coil with ASHRAE90VariableFan capacity control method.

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)
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

Reviewer

This will not be exhaustively relevant to every PR.

Nigusse · 2020-06-22T20:35:35Z

Plots below show zone air temperature and electric heating coil heating rate for develop and the branch for the fix. The branch shows that the zone air temperature during heating and cooling is properly controlled. The electric heating coil heating rate is also modulated to provide the required heating rate.

Nigusse · 2020-06-22T20:41:40Z

Plots below show zone air temperature and heating rates for the branch with hot-water and electric heating coils. The hot-water coil (HWCoil) and electric heating coil (EHCoil) produce "similar" results and the zone air temperature is well controlled in both cases.

Nigusse · 2020-06-23T12:21:43Z

The CI run for this branch shows the unit test "Test_UnitaryHybridAirConditioner_Unittest" failed but when I run this unit test locally it passes. Is there known issue about this unit test? @Myoldmopar

mitchute · 2020-06-23T13:21:44Z

The CI run for this branch shows the unit test "Test_UnitaryHybridAirConditioner_Unittest" failed but when I run this unit test locally it passes. Is there known issue about this unit test? @Myoldmopar

This has been failing intermittently on my machine as well, but I can't get it to repeat consistently so I have never gotten to the bottom of the issue.

Nigusse · 2020-06-23T13:59:41Z

@mitchute Thanks.

Nigusse · 2020-06-23T15:48:57Z

src/EnergyPlus/FanCoilUnits.cc

+                    // load is larger than capacity, thus run the fancoil unit at full capacity
+                    PLR = 1.0;
+                } 
+                Calc4PipeFanCoil(state, FanCoilNum, ControlledZoneNum, FirstHVACIteration, QUnitOut, PLR);
                PowerMet = QUnitOut;


Added else if logic to capture a condition when the heating or cooling load is larger than the maximum equipment capacity and sets the PLR to 1.0 since we calling Cal4PipeFanCoil() function with the optional argument PLR.

Nigusse · 2020-06-23T15:49:56Z

src/EnergyPlus/FanCoilUnits.cc

                PowerMet = QUnitOut;
                AirMassFlow = Node(InletNode).MassFlowRate;
                // CR9155 Remove specific humidity calculations
                SpecHumOut = Node(OutletNode).HumRat;
                SpecHumIn = Node(InletNode).HumRat;
                // Latent rate (kg/s), dehumid = negative
                LatOutputProvided = AirMassFlow * (SpecHumOut - SpecHumIn);
+                FanCoil(FanCoilNum).PLR = PLR;


Needed to set it here for unit test.

This is the one thing that stands out to me from your walkthrough. Fixing it for the unit test just feels odd. Can you clarify what impact this has on the simulation itself?

PLR was not reported/updated for ASHRAE90VariableFan capacity control method. If you look at other capacity control methods PLR value is set to FanCoil.PLR for reporting purpose. In this fix I'm passing PLR as argument to Calc4PipeFanCoil() function here and setting PLR value to 1.0 when the load exceeds the capacity here. There is not other place where a value is set to FanCoil.PLR for this capacity control method.

OK, I'll accept that, sounds good.

Nigusse · 2020-06-23T15:51:07Z

src/EnergyPlus/SZVAVModel.cc

@@ -86,6 +86,7 @@ namespace SZVAVModel {

    // MODULE PARAMETER DEFINITIONS
    static std::string const BlankString;
+    int const HCoil_Water(1);


Added heating coil type variable for identification.

Nigusse · 2020-06-23T15:53:36Z

src/EnergyPlus/SZVAVModel.cc

+                            TempSolveRoot::SolveRoot(state, 0.001, MaxIter, SolFlag, PartLoadRatio, FanCoilUnits::CalcFanCoilWaterFlowResidual, 0.0, 1.0, Par);
+                        } else {
+                            TempSolveRoot::SolveRoot(state, 0.001, MaxIter, SolFlag, PartLoadRatio, FanCoilUnits::CalcFanCoilLoadResidual, 0.0, 1.0, Par);
+                        }
                        outletTemp = DataLoopNode::Node(OutletNode).Temp;


Added SolveRoot() function calling for electric heating coil part-load ratio calculation using a different residual function.

Nigusse · 2020-06-23T15:53:48Z

src/EnergyPlus/SZVAVModel.cc

+                            TempSolveRoot::SolveRoot(state, 0.001, MaxIter, SolFlag, PartLoadRatio, FanCoilUnits::CalcFanCoilAirAndWaterFlowResidual, 0.0, 1.0, Par);
+                            if (SolFlag < 0) {
+                                MessagePrefix = "Step 2b: ";
+                            }


Added SolveRoot() function calling for electric heating coil part-load ratio calculation using a different residual function.

Nigusse · 2020-06-23T15:54:14Z

src/EnergyPlus/SZVAVModel.cc

+                            state, 0.001, MaxIter, SolFlag, PartLoadRatio, FanCoilUnits::CalcFanCoilAirAndWaterFlowResidual, 0.0, 1.0, Par);
+                    } else {
+                        TempSolveRoot::SolveRoot(state, 0.001, MaxIter, SolFlag, PartLoadRatio, FanCoilUnits::CalcFanCoilLoadResidual, 0.0, 1.0, Par);
+                    }


Added SolveRoot() function calling for electric heating coil part-load ratio calculation using a different residual function.

Nigusse · 2020-06-23T15:55:02Z

src/EnergyPlus/SZVAVModel.cc

+                        TempSolveRoot::SolveRoot(state, 0.001, MaxIter, SolFlag, PartLoadRatio, FanCoilUnits::CalcFanCoilWaterFlowResidual, 0.0, 1.0, Par);
+                    } else {
+                        TempSolveRoot::SolveRoot(state, 0.001, MaxIter, SolFlag, PartLoadRatio, FanCoilUnits::CalcFanCoilLoadResidual, 0.0, 1.0, Par);
+                    }
                    if (SolFlag < 0) {


Same change as above new SolveRoot() function calling.

Nigusse · 2020-06-23T15:55:12Z

src/EnergyPlus/SZVAVModel.cc

+                        TempSolveRoot::SolveRoot(state, 0.001, MaxIter, SolFlag, PartLoadRatio, FanCoilUnits::CalcFanCoilAirAndWaterFlowResidual, 0.0, 1.0, Par);
+                    } else {
+                        TempSolveRoot::SolveRoot(state, 0.001, MaxIter, SolFlag, PartLoadRatio, FanCoilUnits::CalcFanCoilLoadResidual, 0.0, 1.0, Par);
+                    }


Same change as above new SolveRoot() function calling.

Nigusse · 2020-06-23T15:56:37Z

src/EnergyPlus/SZVAVModel.hh

@@ -76,7 +76,7 @@ namespace SZVAVModel {
    // parts of the simulation.

    // MODULE PARAMETER DEFINITIONS
-    // na
+    extern int const HCoil_Water;


Added heating coil type variable for identification.

Myoldmopar · 2020-06-30T16:33:54Z

Your results are very encouraging, I am sure this is done or nearly done. I'll do a code review next.

Nigusse · 2020-06-30T16:38:20Z

Yes, this is done and ready for review. I was just updating the branch since it had merge conflict.

Myoldmopar

Code changes are likely fine. I'd like a little clarification on the logic used to switch between residuals. Also, a unit test file appears to have just been emptied? Or something happened?

Myoldmopar · 2020-06-30T17:49:01Z

src/EnergyPlus/FanCoilUnits.cc

@@ -3096,15 +3096,19 @@ namespace FanCoilUnits {
                                                   PLR,
                                                   CompressorOnFlag);
                    }
-                }
-                Calc4PipeFanCoil(state, FanCoilNum, ControlledZoneNum, FirstHVACIteration, QUnitOut);
+                } else if ((CoolingLoad && QUnitOutMax > QZnReq && QZnReq < 0.0) || (HeatingLoad && QUnitOutMax < QZnReq && QZnReq > 0.0)) {


Seems like a straightforward check for calculating the desired operating PLR for the fan coil.

I had to check for condition where the load is greater than the available capacity and set PLR to 1.0. Previously Calc4PipeFanCoil() was run with out the PLR argument and you do have to worry when load exceed the capacity because the default value of PLR is 1.

Myoldmopar · 2020-06-30T17:52:02Z

src/EnergyPlus/SZVAVModel.cc

@@ -772,8 +773,11 @@ namespace SZVAVModel {
                        Par(9) = lowWaterMdot; // minCoilFluidFlow - low fan speed water flow rate > 0 if SAT limited
                        Par(12) = AirMassFlow; // sets air flow rate used when iterating on coil capacity
                        Par(13) = 0.0;         // other than 0 means to iterate on SA temperature
-
-                        TempSolveRoot::SolveRoot(state, 0.001, MaxIter, SolFlag, PartLoadRatio, FanCoilUnits::CalcFanCoilWaterFlowResidual, 0.0, 1.0, Par);
+                        if (SZVAVModel.HCoilType_Num == HCoil_Water || !HeatingLoad) {


The arguments in this IF check are a difficult to fully grok. Anytime there is not a heating load it goes into the original residual function? Could you just clarify?

(Though it's used many times in here so it's probably fine, but still clarifying the logic would help make it digestable)

The residual function FanCoilUnits::CalcFanCoilWaterFlowResidual() varies the water flow rate to meet the heating load for hot water heating coil. The other residual function CalcFanCoilLoadResidual() varies PLR to meet the heating load for electric heating coils. We cannot use the same residual function. The former residual function is used for both cooling and heating modes of operation. Chilled water cooled coil flow rate is varied to meet the cooling load using the same residual function and is captured with "!HeatingLoad" case.

Myoldmopar · 2020-06-30T17:54:58Z

tst/EnergyPlus/unit/FanCoilUnits.unit.cc

@@ -1,4375 +0,0 @@
-// EnergyPlus, Copyright (c) 1996-2020, The Board of Trustees of the University of Illinois,


Any reason why this whole file got deleted? Or something?

I did resolve unit tests conflict, tested locally and every thing was fine. May be I did something inadvertently. I will fix that.

Nigusse · 2020-06-30T18:26:04Z

Recovered the deleted file, tested the unit tests locally, and merged in develop. This should resolve the deleted file issue.

Myoldmopar · 2020-07-02T13:43:56Z

This all looks good. Thank you for the clarifications. CI is happy, your unit tests are good. I'm going to build and run it locally a little but if anyone else has any thoughts just speak up.

Myoldmopar · 2020-07-02T13:45:00Z

Actually, I guess running it locally would mean I'd have to set up my own file. Can you modify an existing IDF, or add a new one, that demonstrates this? You already have it in the unit tests, so I'm guessing it wouldn't take much.

Nigusse · 2020-07-02T13:52:17Z

The defect file has a combination of one electric heating coil and hot water heating coils. One of the FancoilUnit uses electric heating coil and two other FancoilUnits use hot water heating coil. I can clean the defect file and add it. This defect file is also based on existing example file. So, do you prefer electric only heating coil or a combination is fine?

Myoldmopar · 2020-07-02T13:53:41Z

@Nigusse honestly either is fine, as long as the file clearly demonstrates the use of the new feature in at least one unit. I would suggest leaving it as a combination as that is less effort. Thanks!

Nigusse · 2020-07-02T16:28:29Z

@Myoldmopar Added new example with for FanCoilUnit with ASHRAE90VariableFan capacity control method and electric and hot water heating coils.

Myoldmopar · 2020-07-02T17:12:27Z

Thanks @Nigusse! Building and testing now.

Myoldmopar · 2020-07-02T17:27:05Z

@Nigusse the file wasn't running initially because of a case-sensitive file name issue. I addressed that in d14936c and was able to run the test file. Clean error file and results look good. I think I'll risk a tiny bit and go ahead and merge this in without waiting on CI results. Thanks for adding the test file. Good to go.

Myoldmopar · 2020-07-02T21:21:02Z

Well this inadvertently left a tab character in the IDF so develop has a minor issue. I should've waited on CI ... 🙃

nigusse added 5 commits June 22, 2020 09:53

Control logic for ASHRAE90VariableFan with electric heating coil

4111b62

Merge branch 'develop' into 173380574_Issue8087

a1a548f

Modified the fix and cleanup

dc626a5

Added unit tests for issue 8087

2c49377

Merged in develop and fixes issue 8087

ca0fff8

Nigusse added Defect Includes code to repair a defect in EnergyPlus NewFeature Includes code to add a new feature to EnergyPlus labels Jun 22, 2020

Nigusse commented Jun 23, 2020

View reviewed changes

Merge in develop; resolved conflict

f23e8e4

Myoldmopar reviewed Jun 30, 2020

View reviewed changes

nigusse added 2 commits June 30, 2020 14:17

Recovered deleted FanCoulUnit unit test file

60297b3

Merge branch 'develop' into 173380574_Issue8087

8c69c50

nigusse added 2 commits July 2, 2020 12:24

Added new example file FanCoilAutosize_ASHRAE90VariableFan

8201505

Merge branch 'develop' into 173380574_Issue8087

b65cb7b

Myoldmopar added 2 commits July 2, 2020 12:14

Merge remote-tracking branch 'origin/develop' into 173380574_Issue8087

a5d673e

Fix filename casing

d14936c

Myoldmopar merged commit 7a28efb into develop Jul 2, 2020

Myoldmopar deleted the 173380574_Issue8087 branch July 2, 2020 17:27

		@@ -1,4375 +0,0 @@
		// EnergyPlus, Copyright (c) 1996-2020, The Board of Trustees of the University of Illinois,

Support Electric Heating Coil with ASHRAE90VariableFan Capacity Control #8101

Support Electric Heating Coil with ASHRAE90VariableFan Capacity Control #8101

Conversation

Nigusse commented Jun 22, 2020 • edited Loading

Pull request overview

Pull Request Author

Reviewer

Nigusse commented Jun 22, 2020

Nigusse commented Jun 22, 2020

Nigusse commented Jun 23, 2020

mitchute commented Jun 23, 2020

Nigusse commented Jun 23, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Myoldmopar commented Jun 30, 2020

Nigusse commented Jun 30, 2020

Myoldmopar left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Nigusse Jun 30, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Nigusse Jun 30, 2020 • edited Loading

Choose a reason for hiding this comment

Nigusse commented Jun 30, 2020

Myoldmopar commented Jul 2, 2020

Myoldmopar commented Jul 2, 2020

Nigusse commented Jul 2, 2020

Myoldmopar commented Jul 2, 2020

Nigusse commented Jul 2, 2020

Myoldmopar commented Jul 2, 2020

Myoldmopar commented Jul 2, 2020

Myoldmopar commented Jul 2, 2020

Nigusse commented Jun 22, 2020 •

edited

Loading

Nigusse Jun 30, 2020 •

edited

Loading

Nigusse Jun 30, 2020 •

edited

Loading