Add Supplemental Heating Coil to VRF Air Terminal Unit #7252

Nigusse · 2019-04-03T16:01:02Z

Pull request overview

This is a new feature. Modified the VRF terminal units model to support supplemental heating coils. Electric, Fuel, Hot water and Steam coils can be used as supplemental heating coil. The supplemental heating coils are supported in system curve and Fluid_Control VRF systems. Diffs are not expected based on this change. Transition rule has been avoid by making the new input fields optional.

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

Review Checklist

This will not be exhaustively relevant to every PR.

nrel-bot · 2019-04-11T01:32:47Z

@Nigusse it has been 7 days since this pull request was last updated.

nrel-bot · 2019-04-18T00:24:46Z

@Nigusse it has been 7 days since this pull request was last updated.

nrel-bot · 2019-04-25T00:39:27Z

@Nigusse it has been 7 days since this pull request was last updated.

Myoldmopar · 2019-04-29T13:20:48Z

@Nigusse the warning there on the custom_check build is because this PR doesn't have a NewFeature, etc., label on it. At some point when you add that label and then push new commits up, it will turn green. No rush to rebuild it just for that though.

Nigusse · 2019-04-29T20:34:40Z

This branch is ready for review. @Myoldmopar I have addressed your NFP review comments regarding applicability of the model to all VRF system types, and transition requirements on Pull Request Overview section.

rraustad · 2019-04-29T20:45:52Z

src/EnergyPlus/HVACVariableRefrigerantFlow.cc

@@ -3944,6 +3950,174 @@ namespace HVACVariableRefrigerantFlow {
                }
            }

+            // supplemental heating coil
+            if (!lAlphaFieldBlanks(17) && !lAlphaFieldBlanks(18)) {


The protection is good here but what if a users forgets to enter one or the other input? Would we want to warn if one or the other were blank but not both?

I will look into it if it is better to provide a separate warning.

if only one of the two is blank, then I will provide warning message.

rraustad · 2019-04-30T18:53:48Z

src/EnergyPlus/HVACVariableRefrigerantFlow.cc

+        if (VRFTU(VRFTUNum).SuppHeatingCoilPresent) {
+            CompType = VRFTU(VRFTUNum).SuppHeatCoilType;
+            CompName = VRFTU(VRFTUNum).SuppHeatCoilName;
+            SizingMethod = DataHVACGlobals::HeatingCapacitySizing;


Not saying this is wrong since it uses only 1 sizing method (which I like), but there are 2 heating capacity sizing methods. One for water coils (WaterHeatingCapacitySizing) and the other for all other coil types (HeatingCapacitySizing).

@rraustad I missed the two heating coil capacity sizing method flavors. Addressed. I will run my test files if it makes a difference.

rraustad · 2019-04-30T18:58:01Z

src/EnergyPlus/HVACVariableRefrigerantFlow.cc

+        }
+
+        // run supplemental heating coil
+        Real64 SuppPLR = this->SuppHeatPartLoadRatio;


Unless SuppPLR is used somewhere else you could have put this inside the IF block and only executed this line if a supplemental coil was present.

@rraustad You are right. Done.

rraustad · 2019-04-30T19:03:20Z

src/EnergyPlus/HVACVariableRefrigerantFlow.cc

        } else {
            // Algorithm Type: VRF model based on system curve
-            CalcVRF(VRFTUNum, FirstHVACIteration, PartLoadRatio, ActualOutput, OnOffAirFlowRatio);
+            VRFTU(VRFTUNum).CalcVRF(VRFTUNum, FirstHVACIteration, PartLoadRatio, ActualOutput, OnOffAirFlowRatio, SuppHeatCoilLoad);


This looks weird. CalcVRF is a function and this makes it look like a member variable. @Myoldmopar is this standard syntax?

@rraustad This is how you call if it is member function. This is not full OO but it is a step in that direction.

Right, CalcVRF is a member variable...that just happens to be a function. So everyone's right. This is good.

src/EnergyPlus/HVACVariableRefrigerantFlow.cc

rraustad · 2019-04-30T19:06:20Z

src/EnergyPlus/HVACVariableRefrigerantFlow.cc

@@ -10185,7 +10688,25 @@ namespace HVACVariableRefrigerantFlow {

        // Otherwise the coil needs to turn on. Get full load result
        PartLoadRatio = 1.0;
-        this->CalcVRF_FluidTCtrl(VRFTUNum, FirstHVACIteration, PartLoadRatio, FullOutput, OnOffAirFlowRatio);
+        this->CalcVRF_FluidTCtrl(VRFTUNum, FirstHVACIteration, PartLoadRatio, FullOutput, OnOffAirFlowRatio, SuppHeatCoilLoad);


This call looks correct, using this->Calc. Compare to 8265 above.

The reason he couldn't use this-> above is because he wasn't in a member function there, so there's no this-> reference. this-> is only available when you are already in the context of a single instance.

Nigusse · 2019-05-01T15:50:54Z

Local testing showed that supplemental heating coil capacity sizing impacts the the main DX heating coil capacity of the VRF Terminal Unit. But it should not. I will push in revised code.

Myoldmopar · 2019-05-17T01:55:39Z

OK, yeah, hiccup! All is well now. Carry on...

Nigusse · 2019-05-17T19:54:55Z

Merged in develop and resolved conflict. I expect CI machine to run green.

Myoldmopar

I looked the whole thing over, and there is a lot to take in. I didn't see any red flags, and it seems @rraustad has looked it over at least once as well. From a code perspective I'm OK with things.

Myoldmopar · 2019-06-04T12:30:18Z

doc/input-output-reference/src/overview/group-variable-refrigerant-flow-equipment.tex

@@ -2,7 +2,7 @@ \section{Group -- Variable Refrigerant Flow Equipment}\label{group-variable-refr

 This group of EnergyPlus input objects describes the configurations of Variable Refrigerant Flow (VRF, or Variable Refrigerant Volume) air-conditioning systems.

-A VRF system is an air-conditioning system that varies the refrigerant flow rate using variable speed compressor(s) in the outdoor unit, and the electronic expansion valves (EEVs) located in each indoor unit. The system meets the space cooling or heating load requirements by maintaining the zone air temperature at the setpoint. The ability to control the refrigerant mass flow rate according to the cooling and/or heating load enables the use of as many as 60 or more indoor units with differing capacities in conjunction with one single outdoor unit. This unlocks the possibility of having individualized comfort control, simultaneous heating and cooling in different zones, and heat recovery from one zone to another. It may also lead to more efficient operations during part-load conditions.
+A VRF system is an air-conditioning system that varies the refrigerant flow rate using variable speed compressor(s) in the outdoor unit, and the electronic expansion valves (EEVs) located in each indoor unit. The system meets the space cooling or heating load requirements by maintaining the zone air temperature at the setpoint. The ability to control the refrigerant mass flow rate according to the cooling and/or heating load enables the use of as many as 20 or more indoor units with differing capacities in conjunction with one single outdoor unit. This unlocks the possibility of having individualized comfort control, simultaneous heating and cooling in different zones, and heat recovery from one zone to another. It may also lead to more efficient operations during part-load conditions.


Seems like kind of an arbitrary number change, but OK.

Myoldmopar · 2019-06-04T12:32:00Z

src/EnergyPlus/DXCoils.cc

@@ -16388,7 +16389,12 @@ namespace DXCoils {
            EIR = DXCoil(DXCoilNum).RatedEIR(Mode) * EIRTempModFac * EIRFlowModFac;

            // Calculate PLRHeating: modified PartLoadRatio due to defrost ( reverse-cycle defrost only )
-            PLRHeating = min(1.0, (PartLoadRatio + LoadDueToDefrost / TotCap));
+            if (TotCap > 0.0) {


Good divide by zero catch.

Myoldmopar · 2019-06-04T12:34:31Z

src/EnergyPlus/HVACVariableRefrigerantFlow.cc

@@ -4923,6 +5134,9 @@ namespace HVACVariableRefrigerantFlow {
            MyOneTimeFlag = false;
            MyVRFCondFlag = true;

+            MySuppCoilPlantScanFlag.allocate(NumVRFTU);
+            MySuppCoilPlantScanFlag = true;


Seems like you'd want to just add a boolean, defaulted to true, to the VRFTU structure, so that you don't have a separate array to manage. If there are any other changes to make here, I'd like to see that streamlined, or maybe you have a specific reason you needed to do it this way.

No, I did not have any specific reason. I am happy to change it along with your next comment about the nested IF structure. Let me know.

Myoldmopar · 2019-06-04T12:38:57Z

src/EnergyPlus/HVACVariableRefrigerantFlow.cc

+            if ((VRFTU(VRFTUNum).SuppHeatCoilType_Num == DataHVACGlobals::Coil_HeatingWater) ||
+                (VRFTU(VRFTUNum).SuppHeatCoilType_Num == DataHVACGlobals::Coil_HeatingSteam)) {
+
+                if (VRFTU(VRFTUNum).SuppHeatCoilType_Num == DataHVACGlobals::Coil_HeatingWater) {


This nested IF structure is kinda hard to read, and it looks like all but the last line or two are in each of the nested sections. I'd suggest just eliminating the first if (Water Or Steam) block as shown here:

if water or steam if water water-stuff else if steam steam-stuff end a couple common lines end

to

if water water-stuff a couple common lines else if steam steam-stuff a couple common lines end

I realize this code isn't called frequently, so the performance impact of the nested structure will be minimal, and it's not a functional change, so if no other changes are needed, I'll not worry about it.

@Myoldmopar I am making these changes.

Myoldmopar · 2019-06-04T12:40:00Z

src/EnergyPlus/HVACVariableRefrigerantFlow.cc

-                    bool const FirstHVACIteration, // flag for 1st HVAC iteration in the time step
-                    Real64 &PartLoadRatio,         // unit part load ratio
-                    Real64 &OnOffAirFlowRatio      // ratio of compressor ON airflow to AVERAGE airflow over timestep
+    void VRFTerminalUnitEquipment::ControlVRF(int const VRFTUNum,            // Index to VRF terminal unit


👍 👍 👍

Myoldmopar · 2019-06-04T12:41:53Z

src/EnergyPlus/HVACVariableRefrigerantFlow.cc

        } else {
            // Algorithm Type: VRF model based on system curve
-            CalcVRF(VRFTUNum, FirstHVACIteration, PartLoadRatio, ActualOutput, OnOffAirFlowRatio);
+            VRFTU(VRFTUNum).CalcVRF(VRFTUNum, FirstHVACIteration, PartLoadRatio, ActualOutput, OnOffAirFlowRatio, SuppHeatCoilLoad);


Right, CalcVRF is a member variable...that just happens to be a function. So everyone's right. This is good.

Myoldmopar · 2019-06-04T12:43:07Z

src/EnergyPlus/HVACVariableRefrigerantFlow.cc

@@ -10185,7 +10688,25 @@ namespace HVACVariableRefrigerantFlow {

        // Otherwise the coil needs to turn on. Get full load result
        PartLoadRatio = 1.0;
-        this->CalcVRF_FluidTCtrl(VRFTUNum, FirstHVACIteration, PartLoadRatio, FullOutput, OnOffAirFlowRatio);
+        this->CalcVRF_FluidTCtrl(VRFTUNum, FirstHVACIteration, PartLoadRatio, FullOutput, OnOffAirFlowRatio, SuppHeatCoilLoad);


The reason he couldn't use this-> above is because he wasn't in a member function there, so there's no this-> reference. this-> is only available when you are already in the context of a single instance.

Myoldmopar · 2019-06-04T13:11:33Z

CI is happy, and code is generally fine. I ran it locally and I see in the curve-fit VRF file that the supplemental heating coil is coming on for terminal unit 1, but not for the other terminal units. Is that intentional?

Nigusse · 2019-06-04T16:48:12Z

@Myoldmopar I have also seen those instances. When the heating load is relatively small, the DX heating coil provided all heating requirement. Otherwise, all supplemental heating coils types have been tested individually and they work fine.

Nigusse · 2019-06-04T19:53:58Z

@Myoldmopar I just checked my previous run result for example file VRF_5Zone_wSuppHeater.idf, only supplemental heaters of terminal units 2 and 4 were off whereas supplemental heaters of terminal units 1, 3, and 5 do run. Supplemental heaters 2 and 4 were off because, DX heating coil was able to provide the entire heating requirement. For the later two terminal units the DX heating coil run time fraction were less than 1.0.

Nigusse · 2019-06-04T21:09:03Z

Addressed your review comments. @Myoldmopar Thanks.

mjwitte · 2019-06-04T21:23:12Z

idd/Energy+.idd.in

@@ -34671,7 +34671,6 @@ ZoneHVAC:WaterToAirHeatPump,
        \object-list HeatingCoilName
        \note  Needs to match in the supplemental heating coil object
   N11, \field Maximum Supply Air Temperature from Supplemental Heater
-        \required-field


Why is this object being touched here? If you want to make this not required, then it should be \default autosize

I wanted it to be an optional field, no other reason. OK. will add " \required-field" and default to autosize. @mjwitte Thanks.

@mjwitte never mind, I thought this was the VF terminal unit. It must be accidental change. will fix both.

mjwitte · 2019-06-04T21:24:21Z

idd/Energy+.idd.in

+  N11, \field Maximum Supply Air Temperature from Supplemental Heater
+       \type real
+       \units C
+       \autosizable   


This should have \default autosize and that should be mentioned in the I/O Ref (same for the other one).

Updated the IO reference of the WSHP and VRF terminal units.

Myoldmopar · 2019-06-05T02:30:59Z

@Nigusse thanks for addressing my comments.

You've got a couple IDD comments from @mjwitte to address.

Also custom_check noted that your new example file is not UTF8 encoded. I have convert it if you need help doing it, but if you open that IDF in Notepad++ or something similar, you should be able to change the encoding and save it as UTF-8. Something similar should be available in Visual Studio (File -> Advanced Save Settings?)

Once those are taken care of, I think this will be OK to go in.

…oil-To-VRF-TU

Myoldmopar · 2019-06-05T16:55:43Z

@Nigusse thanks for addressing the IDD comments. @mjwitte please verify they are in good shape.

I took the new test file, opened in a text editor, changed encoding to UTF-8, and now custom_check passes locally. I pushed that change along with pulling from develop. If everyone is happy with the next set of results this can be merged.

…oil-To-VRF-TU

Myoldmopar · 2019-06-05T18:19:15Z

No wait, this branch has two integration failures. Rebuilding locally and I'll run them to see what's up with it. I have a suspicion.

Myoldmopar · 2019-06-05T18:35:21Z

Indeed, it was the missing run period names that were causing the two new test files to fail. (They are now required inputs.) I fixed them, and honestly I may just merge this in. Custom-check has now approved the UTF8 change, and I am getting passing test files.

Nigusse added 4 commits March 25, 2019 10:16

Upload NFP

15e41e6

Modified curve-based VRF model to support supplemental heating coil

342b768

Updated NFP

d4575cf

Added supplemental heating coil to VRF Terminal unit Fluid Control Model

556531f

Nigusse added 3 commits April 11, 2019 13:43

Added unit tests

879fcbc

Final NFP and Design Document

d10c985

Merge branch 'develop' into Add-Suppl-HeatingCoil-To-VRF-TU

cc6c33d

Nigusse added 2 commits April 18, 2019 08:49

More code changes and cleanup

aeef807

Initial Documentations Change

00abaef

Merge branch 'develop' into Add-Suppl-HeatingCoil-To-VRF-TU

bfe7234

Nigusse added the NewFeature Includes code to add a new feature to EnergyPlus label Apr 29, 2019

rraustad reviewed Apr 29, 2019

View reviewed changes

rraustad reviewed Apr 30, 2019

View reviewed changes

src/EnergyPlus/HVACVariableRefrigerantFlow.cc Outdated Show resolved Hide resolved

rraustad reviewed Apr 30, 2019

View reviewed changes

NREL deleted a comment from rraustad May 1, 2019

Addressed Review Comments

a15000e

Nigusse added 5 commits May 1, 2019 12:56

Revised Supplemental Heating Coil Sizing

506195e

Merge branch 'develop' into Add-Suppl-HeatingCoil-To-VRF-TU

de0af9d

Added example files.

61462b4

Added new test files to the CMakeLists

7f90de1

Merge branch 'develop' into Add-Suppl-HeatingCoil-To-VRF-TU

3396146

Resolved merge conflict

5d4f92c

mjwitte added the IDDChange Code changes impact the IDD file (cannot be merged after IO freeze) label Jun 3, 2019

mjwitte changed the title ~~NFP - Add Supplemental Heating Coil to VRF Air Terminal Unit~~ Add Supplemental Heating Coil to VRF Air Terminal Unit Jun 3, 2019

Myoldmopar approved these changes Jun 4, 2019

View reviewed changes

Nigusse added 2 commits June 4, 2019 16:10

Merge branch 'develop' into Add-Suppl-HeatingCoil-To-VRF-TU

555e7ae

Addressed review comments

154e233

mjwitte reviewed Jun 4, 2019

View reviewed changes

Nigusse and others added 6 commits June 5, 2019 08:28

updated idd per review comments

fe0f291

updated IO Ref and reverted accidental idd change

d3d561b

Merge branch 'develop' into Add-Suppl-HeatingCoil-To-VRF-TU

a354921

Fixed IO Ref in ZoneHVAC:WaterToAirHeatPump

ba12618

Merge remote-tracking branch 'origin/develop' into Add-Suppl-HeatingC…

8d55f15

…oil-To-VRF-TU

Change encoding to utf8 for test file

db40a35

Merge remote-tracking branch 'origin/develop' into Add-Suppl-HeatingC…

3b79bff

…oil-To-VRF-TU

Add newly required run period names

bceab53

Myoldmopar merged commit 1e8a1da into develop Jun 5, 2019

Myoldmopar deleted the Add-Suppl-HeatingCoil-To-VRF-TU branch June 5, 2019 18:36

jasondegraw mentioned this pull request Jul 3, 2019

Add Supplemental Heating Coil option to VRF Terminal Unit #6605

Closed

jmarrec mentioned this pull request Sep 24, 2019

Add support for supplemental heat to ZoneHVAC:TerminalUnit:VariableRefrigerantFlow NREL/OpenStudio#3687

Closed

TiejunWu mentioned this pull request Oct 25, 2022

VRF Terminal Unit Supplemental Heating Coil Control #9716

Closed

3 tasks

Add Supplemental Heating Coil to VRF Air Terminal Unit #7252

Add Supplemental Heating Coil to VRF Air Terminal Unit #7252

Conversation

Nigusse commented Apr 3, 2019 • edited Loading

Pull request overview

Work Checklist

Review Checklist

nrel-bot commented Apr 11, 2019

nrel-bot commented Apr 18, 2019

nrel-bot commented Apr 25, 2019

Myoldmopar commented Apr 29, 2019

Nigusse commented Apr 29, 2019 • edited Loading

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

Nigusse commented May 1, 2019

Myoldmopar commented May 17, 2019

Nigusse commented May 17, 2019

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

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 4, 2019

Nigusse commented Jun 4, 2019

Nigusse commented Jun 4, 2019

Nigusse commented Jun 4, 2019

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 5, 2019

Myoldmopar commented Jun 5, 2019

Myoldmopar commented Jun 5, 2019

Myoldmopar commented Jun 5, 2019

Nigusse commented Apr 3, 2019 •

edited

Loading

Nigusse commented Apr 29, 2019 •

edited

Loading