Separate design and control variables by jmythms · Pull Request #8530 · NatLabRockies/EnergyPlus

jmythms · 2021-02-12T05:37:14Z

Pull request overview

Split design and control variables for VarFlow and CFlow LowTempRadiant, and hot water and steam baseboard objects.
Design document link
33 test files + Unit tests were transitioned for the regression tests. The big table diffs are from differences in the number of IDF input fields.

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.

…b.com/NREL/EnergyPlus into Separate-Design-and-Control-Variables Pulling to update local.

…b.com/NREL/EnergyPlus into Separate-Design-and-Control-Variables Updating with online changes

…b.com/NREL/EnergyPlus into Separate-Design-and-Control-Variables

dareumnam

Built and tested a few testfiles locally, runs well.

dareumnam · 2021-02-16T18:05:53Z

testfiles/5ZoneCoolingPanelBaseboard.idf

-    SB45,                    !- Surface 3 Name
-    0.1;                     !- Fraction of Radiant Energy to Surface 3
+    SPACE4-1 Baseboard, !- Name
+    SPACE4-1 Baseboard Design, !- Design_Object_Name


SPACE4-1 Baseboard Design is the same as SPACE2-1 Baseboard Design, I think we can use one design object for both baseboards. I think this is one of the main points of this new feature.

@dareumnam Thanks so much for the review! Will take off the redundant design objects.

dareumnam · 2021-02-16T18:07:09Z

testfiles/5ZoneCoolingPanelBaseboardAuto.idf

-    SB45,                    !- Surface 3 Name
-    0.1;                     !- Fraction of Radiant Energy to Surface 3
+    SPACE4-1 Baseboard, !- Name
+    SPACE4-1 Baseboard Design, !- Design_Object_Name


Same here, and idfs below.

Requested changes were made, thank you! In some IDFs, the design objects were different and were not merged together.

…o clear warning

Myoldmopar

@jmythms this is looking really good! I made just a few comments that should be addressed before we merge, but they are pretty minor. Let me know if you have any questions about them. Once they are addressed I'll mark it approved, do a little more testing, and then it can merge.

Myoldmopar · 2021-02-17T18:10:19Z

src/Transition/CreateNewIDFUsingRulesV9_5_0.f90

+                POutArgs(14:15) = InArgs(23:24)
+                POutArgs(16:19) = InArgs(28:31)
+                POutArgs(20) = InArgs(34)
+                CALL WriteOutIDFLines(DifLfn,'ZONEHVAC:LOWTEMPERATURERADIANT:VARIABLEFLOW:DESIGN',20,POutArgs,FldNames,FldUnits)


@jmythms After transitioning, the new object name looks a bit funny in all caps. I'd change this string, and the similar ones in each block, to just being PascalCase:ObjectName:Style.

Myoldmopar · 2021-02-17T18:11:25Z

src/Transition/CreateNewIDFUsingRulesV9_5_0.f90

+                POutArgs(3:4) = InArgs(7:8)
+                POutArgs(5:7) = InArgs(11:13)
+                CALL WriteOutIDFLines(DifLfn,'ZONEHVAC:BASEBOARD:RADIANTCONVECTIVE:STEAM:DESIGN',7,POutArgs,FldNames,FldUnits)
+                nodiff=.false.


But after transitioning a baseboard file, I was able to run the file successfully with a clean error file. I did not test every permutation of object, but I'll continue testing a couple more.

Myoldmopar · 2021-02-17T18:16:41Z

idd/Energy+.idd.in

+       \type real
+       \minimum 0.0
+       \default 1.0
+       \note Enter the fraction of auto - sized heating design capacity.Required field when capacity the


I see you just copied this note from the previous object, but if you can fix up the spacing it would be nice. Change it to be just autosized, and then a space after the period. Then go through the other notes briefly and see if you spot any other weird spacing or punctuation.

It's weird that this file/these lines do not show up as outdated.

Myoldmopar · 2021-02-17T18:18:23Z

src/EnergyPlus/HWBaseboardRadiator.hh

    extern Array1D<HWBaseboardParams> HWBaseboard;
+    extern Array1D<HWBaseboardDesignData> HWBaseboardDesignObject;
    extern Array1D<HWBaseboardNumericFieldData> HWBaseboardNumericFields;
+    extern Array1D<HWBaseboardDesignNumericFieldData> HWBaseboardDesignNumericFields;


We should follow this up quickly with a PR that moves this to the deglobalized state, but not for today obviously.

Myoldmopar · 2021-02-17T18:30:52Z

src/EnergyPlus/SteamBaseboardRadiator.hh

+
+        // Default Constructor
+        SteamBaseboardDesignData()
+                : HeatingCapMethod(0), DesignScaledHeatingCapacity(0.0), Offset(0.0), FracRadiant(0.0), FracDistribPerson(0.0)


I know this is how most default constructors are written in EnergyPlus, so I don't want you to change anything for now, but I wanted to point out that I think we should move to initializing right at the declaration. In other words, instead of having to write this initializer list, just set heating method to 0 right where it is declared: int HeatingCapMethod = 0. The compiler is quite smart, and at even just level 1 optimization, gcc will end up with the same assembly code either way. And not that it matters, but in debug builds, the code is much more efficient if you are able to initialize the way I am suggesting. If someone wants to correct me here, I'm open to discussion, this is just what I'm seeing on CompilerExplorer.

Thanks for this.

Myoldmopar · 2021-02-17T18:33:38Z

src/Transition/InputRulesFiles/Rules9-4-0-to-9-5-0.md

+
+Field 6, previous Field 12 -  Fraction Radiant
+
+Field 7, previous Field 13 -  Fraction of Radiant Energy Incident on People


Very detailed, would it make it more readable to make each of these

into

a

markdown

list?

Myoldmopar · 2021-02-17T18:36:33Z

testfiles/CMakeFiles/CMakeDirectoryInformation.cmake

+set(CMAKE_C_INCLUDE_REGEX_SCAN "^.*$")
+set(CMAKE_C_INCLUDE_REGEX_COMPLAIN "^$")
+set(CMAKE_CXX_INCLUDE_REGEX_SCAN ${CMAKE_C_INCLUDE_REGEX_SCAN})
+set(CMAKE_CXX_INCLUDE_REGEX_COMPLAIN ${CMAKE_C_INCLUDE_REGEX_COMPLAIN})


Hmmm, something went awry here @jmythms . Looks like you accidentally pointed a build folder to the testfiles folder at some point and CMake created these files. The files that need to be plucked back off this branch include:

testfiles/CMakeFiles/CMakeDirectoryInformation.cmake

testfiles/CMakeFiles/progress.marks

testfiles/CTestTestfile.cmake

testfiles/Makefile

testfiles/cmake_install.cmake

Myoldmopar · 2021-02-17T18:38:01Z

doc/input-output-reference/src/overview/group-radiative-convective-units.tex

@@ -47,103 +52,121 @@ \subsubsection{Inputs}\label{inputs-038}

 This field is the name of the hot water outlet node for the baseboard heater.

-\paragraph{Field: Rated AverageWaterTemperature}\label{field-rated-averagewatertemperature}
+\paragraph{Field: Rated Average Water Temperature}\label{field-rated-averagewatertemperature}


Nice cleanup

Myoldmopar · 2021-02-17T18:38:21Z

doc/input-output-reference/src/overview/group-radiative-convective-units.tex

-      0.1,                     !- fraction of radiant energy from heater distributed to surface 5
-      WR-1,                    !- surface 6 Name
-      0.1;                     !- fraction of radiant energy from heater distributed to surface 6
+ZoneHVAC:Baseboard:RadiantConvective:Water:Design,


jmythms · 2021-02-17T18:49:33Z

@jmythms this is looking really good! I made just a few comments that should be addressed before we merge, but they are pretty minor. Let me know if you have any questions about them. Once they are addressed I'll mark it approved, do a little more testing, and then it can merge.

@Myoldmopar Thank you very much! I will let you know as soon as it is done.

jmythms · 2021-02-18T12:17:49Z

idd/Energy+.idd.in

@@ -40712,25 +40712,72 @@ ZoneHVAC:TerminalUnit:VariableRefrigerantFlow,

 \group Zone HVAC Radiative/Convective Units

+ZoneHVAC:Baseboard:RadiantConvective:Water:Design,
+       \min-fields 7


I was not sure of the assignment for min-fields.

My logic for the current assignment was the (alpha+num) fields - just one field in the ConstantFlow and VariableFlow that I thought was optional.

jmythms · 2021-02-18T12:19:41Z

src/Transition/CreateNewIDFUsingRulesV9_5_0.f90

+                END IF
+                IF (InArgs(34) == Blank) THEN
+                  CurVarIterator = 19
+                END IF


Added logic for removing the last line in the Design objects, if it is blank.

next time:

CurVarIterator = 19 IF (InArgs(34) /= Blank) THEN CurVarIterator = 20 POutArgs(CurVarIterator) = InArgs(34) END IF

jmythms · 2021-02-18T12:55:44Z

@Myoldmopar Could you take one more look, when you get the time, please?

Myoldmopar

Thanks for the final cleanups! This will merge soon. Great work @jmythms

Myoldmopar · 2021-02-18T14:24:11Z

src/Transition/CreateNewIDFUsingRulesV9_5_0.f90

+                END IF
+                IF (InArgs(34) == Blank) THEN
+                  CurVarIterator = 19
+                END IF


next time:

CurVarIterator = 19 IF (InArgs(34) /= Blank) THEN CurVarIterator = 20 POutArgs(CurVarIterator) = InArgs(34) END IF

Myoldmopar · 2021-02-18T14:52:28Z

Alright, this one is dropping in first since it needs no other changes. Thanks @jmythms

jmythms · 2021-02-18T14:58:52Z

Alright, this one is dropping in first since it needs no other changes. Thanks @jmythms

Thank you so much!

jmythms and others added 30 commits November 30, 2020 09:08

Merge branch 'develop' into Separate-Design-and-Control-Variables

95503ff

Changes to IDD

bf828f4

Missed the design keywords.

2a7f89c

Removed proposed

ec2081e

Updated changes to IDD file

7eb4896

move everything to a directory

b2ac926

Merge branch 'Separate-Design-and-Control-Variables' of https://githu…

2e686df

…b.com/NREL/EnergyPlus into Separate-Design-and-Control-Variables Pulling to update local.

deletions

9be318f

Fix image

5c110b1

Updated conversations

97857cc

updated figure 1)

8735e17

Merge branch 'Separate-Design-and-Control-Variables' of https://githu…

124b610

…b.com/NREL/EnergyPlus into Separate-Design-and-Control-Variables Updating with online changes

updated breakdown images

2dcfa6f

Updated images.

b04f7c0

Updated IOREF - LTRad

6d3efcd

Updated IOREF - LTRadConvFlow

ba8a620

Update IOREF - LTRadConvFlow fields

8b35e78

Update IOREF - LTBaseboardWater

9ea3d90

Updated IOREF - LTBaseboardSteam

07d2dd1

Updated next steps

dfc588c

Update changes to IDD files

c035886

Removed approach

a6c9a32

Final polishing

91cdb04

Cleanup

646b86b

reordered idd

19deb29

Left the date open.

5f56b62

Marker: Suggestions from Matt

e6de91f

first commit, dunno how FindItemInList works

42d1413

Merge branch 'Separate-Design-and-Control-Variables' of https://githu…

74a018b

…b.com/NREL/EnergyPlus into Separate-Design-and-Control-Variables

Add test IDF

442dd13

jmythms requested review from Myoldmopar and dareumnam February 16, 2021 14:49

dareumnam reviewed Feb 16, 2021

View reviewed changes

jmythms added 10 commits February 16, 2021 12:57

Remove redundant design obj in IDFs

dff2cc0

Remove redundant design obj in IDFs 2

6dce325

Remove redundant design obj in IDFs 3 + changed documentation label t…

e295c14

…o clear warning

Transition CONSTFLOW

15a1134

Transition VFLOW

bf30fb6

Transition HWBB + VFLOW fix

364dc33

Transition SteamBB

071f3d0

InputRules Fix

4ee3041

DesignDoc fix

625bce2

Fix last line for Cflo + VarFlow

7123623

Myoldmopar requested changes Feb 17, 2021

View reviewed changes

jmythms added 6 commits February 18, 2021 01:00

Added CurVarIterator to fix last line

80b2a37

Removed nodiff

ff05129

IDD Note Edit+ Change to PascalCase

5eb244f

markdown lists

4a10ee1

Deleted unnecessary makefiles

916049f

Fixed double spacing in IDD

85cdfb6

jmythms commented Feb 18, 2021

View reviewed changes

Myoldmopar approved these changes Feb 18, 2021

View reviewed changes

Myoldmopar merged commit 6a40bc3 into develop Feb 18, 2021

Myoldmopar deleted the Separate-Design-and-Control-Variables branch February 18, 2021 14:52

Myoldmopar mentioned this pull request Feb 18, 2021

Refactor internal source inputs #8442

Merged

17 tasks


		Field 6, previous Field 12 - Fraction Radiant

		Field 7, previous Field 13 - Fraction of Radiant Energy Incident on People

Conversation

jmythms commented Feb 12, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull request overview

Pull Request Author

Reviewer

Uh oh!

dareumnam left a comment

Choose a reason for hiding this comment

Uh oh!

dareumnam Feb 16, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Myoldmopar left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jmythms commented Feb 17, 2021

Uh oh!

jmythms Feb 18, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jmythms Feb 18, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jmythms commented Feb 18, 2021

Uh oh!

Myoldmopar left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Myoldmopar commented Feb 18, 2021

Uh oh!

jmythms commented Feb 18, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

jmythms commented Feb 12, 2021 •

edited

Loading

dareumnam Feb 16, 2021 •

edited

Loading

jmythms Feb 18, 2021 •

edited

Loading

jmythms Feb 18, 2021 •

edited

Loading