Apply Direct solution in new dx coil #7899

lgu1234 · 2020-04-02T17:49:30Z

Pull request overview

This PR will apply DirectSolution to Coil:Cooling:DX with single speed and multiple speeds.
issue #8216

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.

Reviewer

This will not be exhaustively relevant to every PR.

…new-DX-coil

lgu1234 · 2020-05-18T17:53:02Z

Add 3 test files by adding an object:
PerformancePrecisionTradeoffs,
Yes; !- Use Coil Direct Solutions
UnitarySystem_MultiSpeedDX_DirectSolution.idf.txt
UnitarySystem_SingleSpeedDX_DirectSolution.idf.txt
UnitarySystem_SubcoolReheatDX_DirectSolution.idf.txt

The original 3 examples files are
UnitarySystem_MultiSpeedDX.idf
UnitarySystem_SingleSpeedDX.idf
UnitarySystem_SubcoolReheatDX.idf

Myoldmopar · 2020-05-20T14:26:43Z

Well, CI is perfectly green, and the branch isn't conflicted, so we are off to a good review start. I'll look at the code changes now.

Myoldmopar

No major issues, a couple typos and then I'd like you to confirm my understanding of the implementation and comment on the last section so I know its purpose.

Myoldmopar · 2020-05-20T14:32:21Z

doc/input-output-reference/src/overview/group-simulation-parameters.tex

@@ -1045,6 +1045,13 @@ \subsection{PerformancePrecisionTradeoffs}\label{performanceprecisiontradeoffs}
 \end{longtable}

 Note: The choice of Load in the Control Type of the AirLoopHVAC:UnitarySystem object is required for all coils listed in the above table.
+In addition, when Coil:Cooling:DX is specified under AirLoopHVAC:UnitarySystem, the following coil configurations are supported for Direct Solution:
+
+	Single speed mode at Norminal Speed Number = 1 in the Coil:Cooling:DX:CurveFit:OperatingMode


Typo: Norminal

@Myoldmopar Corrected at Nominal. Thanks

Myoldmopar · 2020-05-20T15:48:09Z

src/EnergyPlus/UnitarySystem.cc

@@ -4542,6 +4542,10 @@ namespace UnitarySystems {
                                }
                            }

+                            if (DataGlobals::DoCoilDirectSolutions && thisSys.m_NumOfSpeedCooling > 1) {
+                                thisSys.FullOutput.resize(thisSys.m_NumOfSpeedCooling + 1);
+                            }


OK, so we start by allocating the full output array to the number of cooling speeds + 1.

Myoldmopar · 2020-05-20T15:48:35Z

src/EnergyPlus/UnitarySystem.cc

@@ -6678,7 +6682,7 @@ namespace UnitarySystems {
                            thisSys.m_HeatMassFlowRate.resize(thisSys.m_NumOfSpeedHeating + 1);
                            thisSys.m_HeatVolumeFlowRate.resize(thisSys.m_NumOfSpeedHeating + 1);
                            thisSys.m_MSHeatingSpeedRatio.resize(thisSys.m_NumOfSpeedHeating + 1);
-                            if (DataGlobals::DoCoilDirectSolutions) {
+                            if (DataGlobals::DoCoilDirectSolutions && thisSys.m_NumOfSpeedCooling < thisSys.m_NumOfSpeedHeating) {
                                thisSys.FullOutput.resize(thisSys.m_NumOfSpeedHeating + 1);
                            }


Then we just increase the size in case heating is larger.

@Myoldmopar Yes.

@lgu1234 I see that you are resizing twice for CoolingCoilType CoilDX_SubcoolReheat here and here. Because the later is always executed whether the cooling type coil is SubCoolReheat or NOT. Is this intentional?

Myoldmopar · 2020-05-20T15:50:42Z

src/EnergyPlus/UnitarySystem.cc

-                    if (DataGlobals::DoCoilDirectSolutions && this->m_CoolingCoilType_Num == DataHVACGlobals::CoilDX_MultiSpeedCooling) {
+                    if (DataGlobals::DoCoilDirectSolutions &&
+                        (this->m_CoolingCoilType_Num == DataHVACGlobals::CoilDX_MultiSpeedCooling ||
+                         (this->m_CoolingCoilType_Num == DataHVACGlobals::CoilDX_Cooling && this->m_NumOfSpeedCooling > 1))) {


This is just adding the new coil to this section.

@Myoldmopar Yes.

This is just filling this->FullOutput[SpeedNum] variable for the new DX cooling coil.

Myoldmopar · 2020-05-20T15:59:25Z

src/EnergyPlus/UnitarySystem.cc

+                                                      HXUnitOn,
+                                                      HeatCoilLoad,
+                                                      SupHeaterLoad,
+                                                      CompressorONFlag);


OK, so first you call to run with cooling PLR of 1.0 to get the full output.

@Myoldmopar Yes, in order to get full outputs at PLR = 1.0.

Myoldmopar · 2020-05-20T16:00:29Z

src/EnergyPlus/UnitarySystem.cc

+                                                      HeatCoilLoad,
+                                                      SupHeaterLoad,
+                                                      CompressorONFlag);
+                        CoolPLR = (ZoneLoad - SensOutputOff) / (SensOutputOn - SensOutputOff);


Then calculate a new cooling PLR using a simple ratio.

@Myoldmopar Yes.

Myoldmopar · 2020-05-20T16:03:36Z

src/EnergyPlus/UnitarySystem.cc

+                                                      HXUnitOn,
+                                                      HeatCoilLoad,
+                                                      SupHeaterLoad,
+                                                      CompressorONFlag);


Then just recalculate the system performance at that PLR, and take what it gets.

@Myoldmopar Yes.

Myoldmopar · 2020-05-20T16:03:58Z

src/EnergyPlus/UnitarySystem.cc

+                                                      HeatCoilLoad,
+                                                      SupHeaterLoad,
+                                                      CompressorONFlag);
+                        PartLoadRatio = CoolPLR;


Of course handle the single speed case here too.

@Myoldmopar Yes for a single speed coil.

Myoldmopar · 2020-05-20T16:04:31Z

src/EnergyPlus/UnitarySystem.cc

+                                                      HeatCoilLoad,
+                                                      SupHeaterLoad,
+                                                      CompressorONFlag);
+                        PartLoadRatio = CoolPLR;


How does this extra section fit into the grand scheme?

@Myoldmopar The extra code is used for a multiple speed coil to recalculate output based on cycleratio (SpeedNumber = 1) or SpeedRatio (SpeedNumber>1) using Direct solution results.

@lgu1234 This is not part of this change but wanted to understand that CoolingCycRatio and CoolingSpeedRatio are passed as CoolPLR to calcUnitarySystemToLoad() function. So, the calcUnitarySystemToLoad() handles the difference between the two variables?

nrel-bot-2 · 2020-06-18T02:12:31Z

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

…EL/EnergyPlus into DirectSolution-in-new-DX-coil

nrel-bot · 2020-07-25T00:39:59Z

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

Nigusse · 2020-08-18T20:25:09Z

Built and ran with develop pulled into the branch. All unit tests passed. @lgu1234 If you reply to my comment regarding FullOutput array resizing, this branch will be ready for merging.

lgu1234 · 2020-08-18T20:37:32Z

@Nigusse Where is your comment of FullOutput? I am not able to locate it.

Nigusse · 2020-08-18T20:44:37Z

@lgu1234 I see that you are resizing twice for CoolingCoilType CoilDX_SubcoolReheat here and here. Because the later is always executed whether the cooling type coil is SubCoolReheat or NOT. Is this intentional?

Nigusse

This branch is ready for merging pending @lgu1234 addressing the review comments about array re-sizing.

Nigusse · 2020-08-18T17:39:10Z

doc/input-output-reference/src/overview/group-simulation-parameters.tex

@@ -1056,7 +1056,7 @@ \subsection{PerformancePrecisionTradeoffs}\label{performanceprecisiontradeoffs}
 \midrule
 \endfirsthead

-AirLoopHVAC:UnitarySystem & Coil:Cooling:DX:SingleSpeed, Coil:Heating:DX:SingleSpeed, Coil:Heating:Electric, Coil:Heating:Fuel,  Coil:Heating:DX:MultiSpeed\tabularnewline
+AirLoopHVAC:UnitarySystem & Coil:Cooling:DX:SingleSpeed, Coil:Heating:DX:SingleSpeed, Coil:Heating:Electric, Coil:Heating:Fuel,  Coil:Heating:DX:MultiSpeed, Coil:Cooling:DX\tabularnewline


Updated I/O ref documentation; added the new DX cooling coil type Coil:Cooling:DX.

Nigusse · 2020-08-18T17:46:15Z

doc/input-output-reference/src/overview/group-simulation-parameters.tex

+
+	Multi speed mode at Nominal Speed Number > 1 in the Coil:Cooling:DX:CurveFit:OperatingMode
+
+	Single speed SubcoolReheat mode. The SubcoolReheat mode requires all inputs of 3 fields in the Coil:Cooling:DX:CurveFit:Performance: Base Operating Mode, Alternative Operating Mode 1, and Alternative Operating Mode 2.


Updated I/O ref documentation; Coil:Cooling:DX coil model in AirLoopHVAC:UnitarySystem supports Direct Solution for load based control of Single Speed, Multi Speed and Single Speed SubcoolReheat operating modes.

Nigusse · 2020-08-18T18:00:22Z

src/EnergyPlus/UnitarySystem.cc

-                                    ShowContinueError(
-                                        "Setpoint control is not available for SubcoolReheat cooling coil. Load control is forced. Simulation continues.");
+                                    ShowContinueError("Setpoint control is not available for SubcoolReheat cooling coil. Load control is forced. "
+                                                      "Simulation continues.");


If the Coil:Cooling:DX coil type operating mode specified is SubcoolReheat operating mode and the UnitarySystem is SetPoint control, then the UnitarySystem control type is reset to Load. Because SubcoolReheat operating mode is supported in Load based control only.

Nigusse · 2020-08-18T18:56:32Z

src/EnergyPlus/UnitarySystem.cc

@@ -6678,7 +6682,7 @@ namespace UnitarySystems {
                            thisSys.m_HeatMassFlowRate.resize(thisSys.m_NumOfSpeedHeating + 1);
                            thisSys.m_HeatVolumeFlowRate.resize(thisSys.m_NumOfSpeedHeating + 1);
                            thisSys.m_MSHeatingSpeedRatio.resize(thisSys.m_NumOfSpeedHeating + 1);
-                            if (DataGlobals::DoCoilDirectSolutions) {
+                            if (DataGlobals::DoCoilDirectSolutions && thisSys.m_NumOfSpeedCooling < thisSys.m_NumOfSpeedHeating) {
                                thisSys.FullOutput.resize(thisSys.m_NumOfSpeedHeating + 1);
                            }


@lgu1234 I see that you are resizing twice for CoolingCoilType CoilDX_SubcoolReheat here and here. Because the later is always executed whether the cooling type coil is SubCoolReheat or NOT. Is this intentional?

Nigusse · 2020-08-18T19:01:21Z

src/EnergyPlus/UnitarySystem.cc

-                    if (DataGlobals::DoCoilDirectSolutions && this->m_CoolingCoilType_Num == DataHVACGlobals::CoilDX_MultiSpeedCooling) {
+                    if (DataGlobals::DoCoilDirectSolutions &&
+                        (this->m_CoolingCoilType_Num == DataHVACGlobals::CoilDX_MultiSpeedCooling ||
+                         (this->m_CoolingCoilType_Num == DataHVACGlobals::CoilDX_Cooling && this->m_NumOfSpeedCooling > 1))) {


This is just filling this->FullOutput[SpeedNum] variable for the new DX cooling coil.

Nigusse · 2020-08-18T19:22:32Z

src/EnergyPlus/UnitarySystem.cc

+                                                      HeatCoilLoad,
+                                                      SupHeaterLoad,
+                                                      CompressorONFlag);
+                        PartLoadRatio = CoolPLR;


@lgu1234 This is not part of this change but wanted to understand that CoolingCycRatio and CoolingSpeedRatio are passed as CoolPLR to calcUnitarySystemToLoad() function. So, the calcUnitarySystemToLoad() handles the difference between the two variables?

Nigusse · 2020-08-18T20:15:17Z

tst/EnergyPlus/unit/UnitarySystem.unit.cc

+                      latOut);
+     EXPECT_NEAR(thisSys->m_CycRatio, 1.000, 0.001);
+     EXPECT_NEAR(thisSys->m_SpeedRatio, 0.238771, 0.02);
+     EXPECT_NEAR(sensOut, -11998.0, 210.0);


The unit tests Direct and Iterative solution methods results are identical. The unit test demonstrates the new method very well.

Nigusse · 2020-08-19T13:46:47Z

@Myoldmopar @lgu1234 I ran the UnitarySystem_MultiSpeedDX.idf and UnitarySystem_MultiSpeedDX_DirectSolution.idf example files provided. The only difference between these two files is that the later has PerformancePrecisionTradeoffs object set to Yes. And I see diffs based on two days of RunPeriods weather file simulation. See below the minimum and maximum diffs observed for a selected variables. Is this acceptable diffs?

Myoldmopar · 2020-08-19T13:54:34Z

Most of them look acceptable, but the sys 5 heating rate is the obvious outlier. Can you show a profile of the difference in that variable over the course of the simulation? Or show the annual energy difference?

lgu1234 · 2020-08-19T13:56:13Z

@Nigusse Yes. You are correct. The only difference in these files is Yes or No. I compared zone air temperature. If energy use is compared, it is better to compare sum, instead of timestep comparison.

Nigusse · 2020-08-19T14:06:35Z

Ok. I will do the profile comparison of system 5 and annual energy use comparison. In the meantime see below the zone air temperature comparisons and both methods show almost identical profile.

Nigusse · 2020-08-19T14:20:30Z

See below plots of SYS 5 FURNACE DX COOL HEATING COIL:Heating Coil Heating Rate [W][Hourly] report variables and diffs profiles.

Nigusse · 2020-08-19T15:02:20Z

Annual energy use differences and minimum and maximum hourly energy use differences for a selected variables.

Also see the annual energy use and differences for system 5 for the two solution techniques:

Myoldmopar · 2020-09-04T14:08:15Z

The Linux diffs appear to be because the branch was out of date when that ran. I will pull develop into here and look it over.

…new-DX-coil

Myoldmopar · 2020-09-04T14:42:40Z

Built and ran fine with develop pulled in locally, we'll let CI verify. I'll also take a final pass over the code now.

Myoldmopar · 2020-09-04T14:45:10Z

Code changes look fine to me. If CI is also happy I assume this will drop in shortly. If anyone wants to look it over, have at it.

Myoldmopar · 2020-09-04T16:03:36Z

Regressions are all gone, this is clean and good to go. I'll give it a few minutes and wait for the unit test and windows to finish but then merge. Thanks @lgu1234

Myoldmopar · 2020-09-04T16:48:38Z

All good, merging.

lgu1234 added 2 commits April 2, 2020 13:43

Initial check-in

85aa9e9

Merge branch 'develop' into DirectSolution-in-new-DX-coil

df16ae5

Myoldmopar added this to the EnergyPlus 9.4.0 milestone Apr 14, 2020

lgu1234 and others added 7 commits April 27, 2020 10:57

Apply DirectSolution to SubcoolReheat type

2775213

Merge branch 'develop' into DirectSolution-in-new-DX-coil

0a08123

Update IO Ref

705b5b3

Merge branch 'develop' into DirectSolution-in-new-DX-coil

cde3647

Repair compile failure

fcbd14e

Merge branch 'develop' into DirectSolution-in-new-DX-coil

98e72e1

Merge remote-tracking branch 'origin/develop' into DirectSolution-in-…

f1101a9

…new-DX-coil

Myoldmopar added the Performance Includes code changes that are directed at improving the runtime performance of EnergyPlus label May 18, 2020

Myoldmopar reviewed May 20, 2020

View reviewed changes

lgu1234 added 3 commits June 26, 2020 13:47

Address document comments of IO Ref

668f865

Merge branch 'DirectSolution-in-new-DX-coil' of https://github.com/NR…

31b73fb

…EL/EnergyPlus into DirectSolution-in-new-DX-coil

Merge branch 'develop' into DirectSolution-in-new-DX-coil

def13bd

lgu1234 added 4 commits August 6, 2020 09:00

Upload a unit test

7df55b6

Merge branch 'develop' into DirectSolution-in-new-DX-coil

0eabb5c

Remove an unused variable

c141c9d

Merge branch 'develop' into DirectSolution-in-new-DX-coil

679e9ac

Nigusse self-requested a review August 18, 2020 17:30

Merge branch 'develop' into DirectSolution-in-new-DX-coil

56804cc

Nigusse approved these changes Aug 18, 2020

View reviewed changes

Merge remote-tracking branch 'origin/develop' into DirectSolution-in-…

2d71845

…new-DX-coil

Myoldmopar approved these changes Sep 4, 2020

View reviewed changes

Myoldmopar merged commit 8f6f2b3 into develop Sep 4, 2020

Myoldmopar deleted the DirectSolution-in-new-DX-coil branch September 4, 2020 16:48


		Multi speed mode at Nominal Speed Number > 1 in the Coil:Cooling:DX:CurveFit:OperatingMode

		Single speed SubcoolReheat mode. The SubcoolReheat mode requires all inputs of 3 fields in the Coil:Cooling:DX:CurveFit:Performance: Base Operating Mode, Alternative Operating Mode 1, and Alternative Operating Mode 2.

Apply Direct solution in new dx coil #7899

Apply Direct solution in new dx coil #7899

Conversation

lgu1234 commented Apr 2, 2020 • edited by Nigusse Loading

Pull request overview

Pull Request Author

Reviewer

lgu1234 commented May 18, 2020

Myoldmopar commented May 20, 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

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

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

nrel-bot-2 commented Jun 18, 2020

nrel-bot commented Jul 25, 2020

Nigusse commented Aug 18, 2020

lgu1234 commented Aug 18, 2020

Nigusse commented Aug 18, 2020

Nigusse 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

Nigusse Aug 18, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Nigusse commented Aug 19, 2020

Myoldmopar commented Aug 19, 2020

lgu1234 commented Aug 19, 2020

Nigusse commented Aug 19, 2020 • edited Loading

Nigusse commented Aug 19, 2020

Nigusse commented Aug 19, 2020 • edited Loading

Myoldmopar commented Sep 4, 2020

Myoldmopar commented Sep 4, 2020

Myoldmopar commented Sep 4, 2020

Myoldmopar commented Sep 4, 2020

Myoldmopar commented Sep 4, 2020

lgu1234 commented Apr 2, 2020 •

edited by Nigusse

Loading

Nigusse Aug 18, 2020 •

edited

Loading

Nigusse commented Aug 19, 2020 •

edited

Loading

Nigusse commented Aug 19, 2020 •

edited

Loading