Remove AirTerminal:SingleDuct:Uncontrolled #7448

mjwitte · 2019-08-08T20:17:01Z

Pull request overview

In v8.9.0 (March 2018), AirTerminal:SinglDuct:Uncontrolled was marked obsolete, to be replaced with AirTerminal:SingleDuct:ConstantVolume:NoReheat (which requires a ZoneHVAC:AirDistributionUnit object like all of the other air terminal units). This change removes AirTerminal:SinglDuct:Uncontrolled from the idd, removes all related source code, adds transition code to move to the new air terminal (and modify related objects) and eliminates this object from all example files.

IDD
Transition
Update test files
Unit tests - fix existing one
Documentation
ExpandObjects
Fixes ZoneHVAC:AirDistributionUnit output for Zone Air Terminal Sensible Heating/Cooling Rate is summed - should be averaged #7595
Unit tests - add a new one
~~Ferret out small performance downgrade (-0.07%).~~ This is somewhat unavoidable, because this terminal unit uses all of the ZoneHVAC:AirDistributionUnit logic including leakage calculations, so there are extra hoops (ifs) to jump through. Any messing around here to streamline SingleDuct will only obscure the substantive code changes here.

Explanation of Diffs

_SmallOffice_Dulles is configured with no ZoneSplitter, so with the old direct air terminal unit, which only has a single node (acts as inlet and outlet), the zone inlet node is the same node as the AirloopHVAC Demand Side Inlet Node. So, the last time around the loop when the Demand Side Inlet Node gets updated from the Supply Side Outlet Node the zone inlet node got updated even if there wasn't another round of simulating the zone equipment. With the new terminal unit, this doesn't happen, because the terminal unit separates the air loop Demand Side Inlet from the zone inlet. I can reproduce this with the old terminal unit by adding a ZoneSplitter in the loop, then the diffs are tiny. The two charts below illustrate the diffs (reported at detailed frequency with ReportDuringWarmup, the same patterns persist after warmup). The summer design day with more humidity action and higher levels doesn't have this issue.

For _SmallOffice_Dulles , even though there are some significant timestep diffs, the annual site energy changes only slightly from 383811.80 to 383811.74 [kBtu].

The other files with big diffs are AirFlowNetwork files withDistribution. The additional node for the newer terminal unit requires an additional linkage element in the AFN, which introduces diffs.

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:
- Defect: This pull request repairs a github defect issue. The github issue should be referenced in the PR description
- 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
- Performance: This pull request includes code changes that are directed at improving the runtime performance of EnergyPlus
- DoNoPublish: This pull request includes changes that shouldn't be included in the changelog

Review Checklist

This will not be exhaustively relevant to every PR.

…ests

…nsition

…ermSDUncontrolled

mjwitte · 2019-08-09T18:15:57Z

Well, that's a disappointing set of diffs. There's at least one issue that needs to be corrected in the new(er) terminal unit based on these diffs in Sum of Air Terminal Heating Rates.

nrel-bot · 2019-09-10T00:33:56Z

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

nrel-bot-2 · 2019-10-08T02:05:29Z

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

…ermSDUncontrolled

…her cleanup

…olume

…ermSDUncontrolled

mjwitte

This is ready for review. Here's the developer walkthrough of changes.

Significant doc changes across AuxiliaryPrograms, EMS guide, Engineering Ref, and I/O ref.
Transitioned 100s of idfs (performance_tests and testfiles) from old to new - ran transition on the entire set (a few months ago).
Removed all references to old terminal unit throughout the code.
Other highlights noted as line comments.
Given the merge of this transition with other transition code in develop, the reviewer should probably test some random 9.2 files to be sure it's all working correctly. At some point before release we should run a test on the entire testfiles folder for 9.2->9.3 and make sure there no failures. @Myoldmopar @mitchute

mjwitte · 2020-01-29T15:46:45Z

idd/Energy+.idd.in

      \key AirTerminal:DualDuct:ConstantVolume
      \key AirTerminal:DualDuct:VAV
      \key AirTerminal:SingleDuct:ConstantVolume:Reheat
+      \key AirTerminal:SingleDuct:ConstantVolume:NoReheat


This was missing from RoomAir:Node:AirflowNetwork:HVACEquipment.

mjwitte · 2020-01-29T15:47:35Z

idd/Energy+.idd.in

@@ -45649,49 +45648,6 @@ ZoneHVAC:VentilatedSlab:SlabGroup,

 \group Zone HVAC Air Loop Terminal Units

-AirTerminal:SingleDuct:Uncontrolled,


Remove this object from IDD and all references to it.

mjwitte · 2020-01-29T15:49:20Z

performance_tests/PipingSystem_Underground_FHX.idf

@@ -776,8 +776,8 @@
    Zone2WTAHP,              !- Zone Equipment 1 Name
    1,                       !- Zone Equipment 1 Cooling Sequence
    1,                       !- Zone Equipment 1 Heating or No-Load Sequence
-    ,                        !- Zone Equipment 1 Sequential Cooling Load Fraction Schedule Name
-    ;                        !- Zone Equipment 1 Sequential Heating Load Fraction Schedule Name
+    ,                        !- Zone Equipment 1 Sequential Cooling Fraction Schedule Name


Running transition on all testfiles (a few months ago) introduced additional cleanups.

mjwitte · 2020-01-29T15:54:13Z

src/EnergyPlus/MixedAir.cc

@@ -1894,6 +1894,7 @@ namespace MixedAir {
                                                    if (UtilityRoutines::SameString(ZoneEquipList(EquipListNum).EquipName(EquipNum),
                                                                                    AirDistUnit(ADUNum).Name)) {
                                                        if ((AirDistUnit(ADUNum).EquipType_Num(EquipNum) == SingleDuctVAVReheat) ||
+                                                            (AirDistUnit(ADUNum).EquipType_Num(EquipNum) == SingleDuctConstVolNoReheat) ||


This was missing.

mjwitte · 2020-01-29T16:12:57Z

src/EnergyPlus/RoomAirModelManager.cc

@@ -2858,112 +2858,112 @@ namespace RoomAirModelManager {
            }
        }

-        if (TypeNum == 1) { // ZoneHVAC:TerminalUnit : VariableRefrigerantFlow
+        if (TypeNum == DataHVACGlobals::ZoneEquipTypeOf_VariableRefrigerantFlow) { // ZoneHVAC:TerminalUnit : VariableRefrigerantFlow


Cleaning this up to use the parameter variables.

mjwitte · 2020-01-29T16:41:13Z

src/Transition/CreateNewIDFUsingRulesV9_3_0.f90

@@ -610,6 +990,34 @@ SUBROUTINE CreateNewIDFUsingRules(EndOfFile,DiffOnly,InLfn,AskForInput,InputFile

              ! If your original object starts with H, insert the rules here

+              CASE("HEATPUMP:WATERTOWATER:EIR:HEATING")


Just moving these up to their alphabetical location. Nit-picking.

mjwitte · 2020-01-29T16:42:55Z

src/Transition/InputProcessor.f90

    ItemNum=ItemNum+1
-    IF (IDFRecords(ObjNum)%Alphas(1) == ObjName) EXIT
+    IF (SameString(IDFRecords(ObjNum)%Alphas(1), ObjName)) EXIT


Changed these to avoid case-sensitivity problems encountered during this work. Hope that's ok. @mbadams5

mjwitte · 2020-01-29T16:46:40Z

src/Transition/SupportFiles/Report Variables 8-8-0 to 8-9-0.csv

 Heating Coil Total Heating Rate,Heating Coil Heating Rate,
 Heating Coil Total Heating Energy,Heating Coil Heating Energy,
 Heating Coil Air Heating Rate,Heating Coil Heating Rate,
 Heating Coil Air Heating Energy,Heating Coil Heating Energy,
+old variable name,new variable name,-- add variable names (before this line) and leave off units -- <DELETE> to delete


Correcting this in old Report Variables csv file.

mjwitte · 2020-01-29T16:47:48Z

src/Transition/SupportFiles/Report Variables 9-2-0 to 9-3-0.csv

-12,12,These numbers should be the number of report variables in the following list (including deletes). Two columns/numbers.
-old variable name,new variable name,-- add variable names (before this line) and leave off units -- <DELETE> to delete
-Water to Water Heat Pump Load Side Heat Transfer Rate,Heat Pump Load Side Heat Transfer Rate,
-Water to Water Heat Pump Load Side Heat Transfer Energy,EIR Heat Pump Load Side Heat Transfer Energy,


Note these were in the wrong place and this line had a stray "EIR". That's been fixed here in the PR. @mitchute

mjwitte · 2020-01-29T16:49:36Z

tst/EnergyPlus/unit/AirTerminalSingleDuctConstantVolumeNoReheat.unit.cc

@@ -385,6 +387,218 @@ TEST_F(EnergyPlusFixture, AirTerminalSingleDuctCVNoReheat_Sim)
    EXPECT_EQ(SysOutlet(SysNum).AirMassFlowRate, SysInlet(SysNum).AirMassFlowRate);
 }

+TEST_F(EnergyPlusFixture, AirTerminalSingleDuctCVNoReheat_OASpecification)


New unit test covers the OA specification on this terminal unit. Other unit tests updated as needed.

mjwitte · 2020-01-29T17:09:20Z

src/EnergyPlus/ZoneAirLoopEquipmentManager.cc

-                                (Node(AirDistUnit(AirDistUnitNum).OutletNodeNum).Temp - Node(ZoneEquipConfig(ControlledZoneNum).ZoneNode).Temp);
-            // AirDistUnit( AirDistUnitNum ).HeatRate = max( 0.0, SysOutputProvided );
-            // AirDistUnit( AirDistUnitNum ).CoolRate = std::abs( min( 0.0, SysOutputProvided ));
+            if (AirDistUnit(AirDistUnitNum).EquipType_Num(1) == SingleDuctConstVolNoReheat) {


Forgot to note here. The old direct air terminal unit used a different method to calculate sensible delivered. Duplicated that method here just for SingleDuctConstVolNoReheat to minimize diffs. #7728 posted to resolve late.

mjwitte · 2020-01-29T17:40:14Z

So, there's been a legitimate integration test failure for a while that I didn't notice, so not quite ready for review.

mitchute · 2020-01-29T19:45:01Z

@mjwitte I did a quick search of the repo for the old object and found a few stray references in IDF comment lines, and two in the IO-docs. I went ahead and made the changes. I also built the docs and they all look good. I'll wait for you to address the last integration test issue before reviewing it further.

mitchute · 2020-01-29T20:01:51Z

Oh, and it's also still referenced in HVAC-diagram...

mjwitte · 2020-01-29T21:04:14Z

@mitchute Thanks for the cleanup. HVAC-Diagram, hmmm, will check on that, too.

…/EnergyPlus into removeAirTermSDUncontrolled

…ermSDUncontrolled

mjwitte · 2020-01-31T19:25:25Z

src/Transition/SupportFiles/Report Variables 9-2-0 to 9-3-0.csv

-old variable name,new variable name,-- add variable names (before this line) and leave off units -- <DELETE> to delete
+9.2.0,9.3.0,Transition notes - some of these are EMS variable names
+14,14,These numbers should be the number of report variables in the following list (including deletes). Two columns/numbers.
+Water to Water Heat Pump Load Side Heat Transfer Rate,Heat Pump Load Side Heat Transfer Rate,


@mitchute Not sure why github diff shows all of these lines have changed. Local diff does not. Only line 2 was change and lines 15 and 16 were added.

Agreed, that's odd. Everything looks good #7730. Potentially different line endings or encoding, but both seem unlikely.

…ermSDUncontrolled

… stuff

…ermSDUncontrolled

…r consistency

mitchute · 2020-02-13T18:07:39Z

@mjwitte this is looking good. I don't have any issues from a coding perspective. There was one file that got missed in the transition process, so I went ahead and transitioned it up, tested it, and it worked without issue. All unit tests also pass locally. All changes are now pushed up and I think this should be ready as soon as CI takes another pass.

From an outside reviewer perspective, there are plenty of diffs here but they appear to justified.

RDD diffs are due to fixing ZoneHVAC:AirDistributionUnit output for Zone Air Terminal Sensible Heating/Cooling Rate is summed - should be averaged #7595
The majority of the text diffs are due to objects and connections being renamed
One file (_SmallOffice_Dulles.idf) has big table diffs. Upon inspection, the "Annual Building Sensible Heat Gain Components" table has some fields with diffs on the order of 0.01 kBtu.
Some selected results from AirflowNetwork_MultiZone_SmallOffice_HeatRecoveryHXSL.idf are posted below. This is one of the few files that have big math diffs according to local testing and CI.

Results for the winter and summer design days are plotted.

Temperatures are consistent and the relative energy diffs are very minor. Given all that's been done, I consider this acceptable.

@Myoldmopar any final thoughts? If not, I think we can merge this in once CI comes back.

Myoldmopar · 2020-02-13T19:34:15Z

I have no final thoughts, this looks good, and thanks for the analysis of the diffs there, @mitchute.

mitchute · 2020-02-13T19:38:58Z

CI is done and there are addional changes to examine. Merging.

mjwitte added 11 commits August 5, 2019 21:12

Remove AirTerminal:SingleDuct:Uncontrolled - idd and transition

942f6d1

Remove AirTerminal:SingleDuct:Uncontrolled - more idd and transition

eca79f2

Remove AirTerminal:SingleDuct:Uncontrolled - remove from source code

a35b3e3

Remove AirTerminal:SingleDuct:Uncontrolled - more from source code

2763577

Remove AirTerminal:SingleDuct:Uncontrolled - transition unit tests

cad1789

Remove AirTerminal:SingleDuct:Uncontrolled - transition performance_t…

dfbf1a6

…ests

Remove AirTerminal:SingleDuct:Uncontrolled - ems transition

82e11c9

Remove AirTerminal:SingleDuct:Uncontrolled - temporary 9.2 to 9.2 Tra…

de21a57

…nsition

Remove AirTerminal:SingleDuct:Uncontrolled - more transition pieces

d9dde50

Merge remote-tracking branch 'remotes/origin/develop' into removeAirT…

602d0ed

…ermSDUncontrolled

Remove AirTerminal:SingleDuct:Uncontrolled - transition testfiles

9bee1fb

mjwitte added the Defect Includes code to repair a defect in EnergyPlus label Aug 8, 2019

mjwitte added 4 commits August 8, 2019 17:29

Remove AirTerminal:SingleDuct:Uncontrolled - fix build errors

84580e9

Remove AirTerminal:SingleDuct:Uncontrolled - fix expandobjects

93f10b4

Remove AirTerminal:SingleDuct:Uncontrolled - remove from docs

f026830

Remove AirTerminal:SingleDuct:Uncontrolled - idd cleanup

1dd0e9d

mjwitte added this to the EnergyPlus Future milestone Aug 12, 2019

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

mjwitte added 9 commits October 9, 2019 15:51

Merge remote-tracking branch 'remotes/origin/develop' into removeAirT…

9fcaaea

…ermSDUncontrolled

Remove AirTerminal:SingleDuct:Uncontrolled - fix unit tests after merge

0c72c25

Remove AirTerminal:SingleDuct:Uncontrolled - move transition to 9.3

bfa65cd

Remove AirTerminal:SingleDuct:Uncontrolled - fix new testfiles and ot…

7bb6f5b

…her cleanup

Remove AirTerminal:SingleDuct:Uncontrolled - fix system sizing and ot…

748b978

…her cleanup

Remove AirTerminal:SingleDuct:Uncontrolled - fix HVACTemplate const v…

40f3ae7

…olume

Remove AirTerminal:SingleDuct:Uncontrolled - revert CV min frac change

2dca408

Remove AirTerminal:SingleDuct:Uncontrolled - attempt to fix diffs

40f84af

Merge remote-tracking branch 'remotes/origin/develop' into removeAirT…

3151c89

…ermSDUncontrolled

mjwitte commented Jan 29, 2020

View reviewed changes

mjwitte requested review from mitchute and Myoldmopar January 29, 2020 16:55

mjwitte mentioned this pull request Jan 29, 2020

Resolve terminal unit sensible output calculation method differences #7728

Closed

3 tasks

mjwitte commented Jan 29, 2020

View reviewed changes

cleanup stray object names in idfs and docs

0cd9473

mjwitte added 3 commits January 29, 2020 15:21

Remove AirTerminal:SingleDuct:Uncontrolled - fix integr test failure

a9a9eed

Remove AirTerminal:SingleDuct:Uncontrolled - HVAC Diagram cleanup

692155d

Merge branch 'removeAirTermSDUncontrolled' of https://github.com/NREL…

8621cd2

…/EnergyPlus into removeAirTermSDUncontrolled

mjwitte mentioned this pull request Jan 30, 2020

Fix Report Variables CSV Ordering #7730

Merged

20 tasks

Merge remote-tracking branch 'remotes/origin/develop' into removeAirT…

649fa6f

…ermSDUncontrolled

mjwitte commented Jan 31, 2020

View reviewed changes

mitchute mentioned this pull request Jan 31, 2020

Make HeatPump:* Output Variable Names Consistent #7734

Merged

20 tasks

mjwitte added 3 commits February 7, 2020 10:47

Merge remote-tracking branch 'remotes/origin/develop' into removeAirT…

ef87136

…ermSDUncontrolled

Remove AirTerminal:SingleDuct:Uncontrolled - update unit test for eio…

74ca911

… stuff

Merge remote-tracking branch 'remotes/origin/develop' into removeAirT…

34ec21e

…ermSDUncontrolled

mitchute mentioned this pull request Feb 7, 2020

Documentation Fixes - 2018b #7737

Merged

20 tasks

mitchute added 3 commits February 12, 2020 15:12

Merge branch 'develop' into removeAirTermSDUncontrolled

0d8ef4a

Merge branch 'develop' into removeAirTermSDUncontrolled

3cffe8c

transition MultiSpeedHeatPump_DirectSolution.idf, rename two files fo…

343a94e

…r consistency

mitchute merged commit 0eb2edf into develop Feb 13, 2020

mitchute deleted the removeAirTermSDUncontrolled branch February 13, 2020 19:39

mjwitte mentioned this pull request Jul 17, 2020

SingleDuctAirTerminal Heat/Cool Rate/Energy variables are no longer used #8161

Closed

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove AirTerminal:SingleDuct:Uncontrolled #7448

Remove AirTerminal:SingleDuct:Uncontrolled #7448

mjwitte commented Aug 8, 2019 •

edited

Loading

mjwitte commented Aug 9, 2019

nrel-bot commented Sep 10, 2019

nrel-bot-2 commented Oct 8, 2019

mjwitte left a comment

mjwitte Jan 29, 2020

mjwitte Jan 29, 2020

mjwitte Jan 29, 2020

mjwitte Jan 29, 2020

mjwitte Jan 29, 2020

mjwitte Jan 29, 2020

mjwitte Jan 29, 2020

mjwitte Jan 29, 2020

mjwitte Jan 29, 2020

mjwitte Jan 29, 2020

mjwitte Jan 29, 2020 •

edited

Loading

mjwitte commented Jan 29, 2020

mitchute commented Jan 29, 2020

mitchute commented Jan 29, 2020

mjwitte commented Jan 29, 2020

mjwitte Jan 31, 2020

mitchute Jan 31, 2020

mitchute commented Feb 13, 2020

Myoldmopar commented Feb 13, 2020

mitchute commented Feb 13, 2020

		@@ -45649,49 +45648,6 @@ ZoneHVAC:VentilatedSlab:SlabGroup,

		\group Zone HVAC Air Loop Terminal Units

		AirTerminal:SingleDuct:Uncontrolled,

		@@ -610,6 +990,34 @@ SUBROUTINE CreateNewIDFUsingRules(EndOfFile,DiffOnly,InLfn,AskForInput,InputFile

		! If your original object starts with H, insert the rules here

		CASE("HEATPUMP:WATERTOWATER:EIR:HEATING")

Remove AirTerminal:SingleDuct:Uncontrolled #7448

Remove AirTerminal:SingleDuct:Uncontrolled #7448

Conversation

mjwitte commented Aug 8, 2019 • edited Loading

Pull request overview

Explanation of Diffs

Work Checklist

Review Checklist

mjwitte commented Aug 9, 2019

nrel-bot commented Sep 10, 2019

nrel-bot-2 commented Oct 8, 2019

mjwitte 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

mjwitte Jan 29, 2020 • edited Loading

Choose a reason for hiding this comment

mjwitte commented Jan 29, 2020

mitchute commented Jan 29, 2020

mitchute commented Jan 29, 2020

mjwitte commented Jan 29, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mitchute commented Feb 13, 2020

Myoldmopar commented Feb 13, 2020

mitchute commented Feb 13, 2020

mjwitte commented Aug 8, 2019 •

edited

Loading

mjwitte Jan 29, 2020 •

edited

Loading