Support Space for Infiltration and Ventilation by mjwitte · Pull Request #9564 · NatLabRockies/EnergyPlus

mjwitte · 2022-07-26T03:29:06Z

Pull request overview

Addresses #9448

Allow ZoneInfiltration and ZoneVentilation to be specified by Zone or Space.
Simulate ZoneInfiltration and ZoneVentilation at the space level (similar to what's already being done for Lights and Equipment).
Add new fields to Space (Volume and Ceiling Height). Addresses Add volume property to space object #9362
Move GlobalInternalGainMiscObject struct and related variables from DataHeatBalance to InternalHeatGains.hh. Nearly all arrays that used GlobalInternalGainMiscObject and related Num... variables are now local to their respective getInput functions. Lights and ElectricEquipment are needed for the demand manager, so they're now in state.InternalHeatGains.
Needs some unit tests.

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.

amirroth · 2022-07-26T12:05:34Z

src/EnergyPlus/DataHeatBalance.hh

    };

-    struct ZoneData
+    struct ZoneData : ZoneSpaceData


Is ZoneData really a subclass of ZoneSpaceData or is ZoneSpaceData a "sub-struct" of ZoneData? Is there any usage in which ZoneSpaceData can be used instead of ZoneData?

Is ZoneData really a subclass of ZoneSpaceData or is ZoneSpaceData a "sub-struct" of ZoneData?

Well, no. ZoneData and SpaceData are peers with very similar structs (many of the same field names), but then each one has some additional fields that only apply to one or the other.

Is there any usage in which ZoneSpaceData can be used instead of ZoneData?

Possibly. For calculating things like Volume, FloorArea, CeilingHeight, etc. I was thinking those could be member functions of ZoneSpaceData.

Actually, they aren't peers. Spaces are contained within Zones but they do have many similar attributes.

Okay, so this is kind of a "tweener" idiom. Not exactly an "is-a" (child struct), not exactly a "has-a" (included struct). Both work for now, can keep it this way until/unless a more obvious parent-child relationship reveals itself.

amirroth · 2022-07-26T12:06:45Z

src/EnergyPlus/DataHeatBalance.hh

+        int ListMultiplier = 1; // For Zone Group object:  used in reporting and systems calculations
+        int ListGroup = 0;      // used only in Zone Group verification.  and for error message.
+        Real64 RelNorth = 0.0;  // Relative North (to building north) [Degrees]
+        Real64 OriginX = 0.0;   // X origin  [m]


Vector3<Real64>

amirroth · 2022-07-26T12:07:22Z

src/EnergyPlus/DataHeatBalance.hh

+        Real64 OriginX = 0.0;   // X origin  [m]
+        Real64 OriginY = 0.0;   // Y origin  [m]
+        Real64 OriginZ = 0.0;   // Z origin  [m]
+        int OfType = 1;         // 1=Standard Zone, Not yet used:


Make this an enumeration and please don't use Of in variable/type names.

amirroth · 2022-07-26T12:08:48Z

src/EnergyPlus/DataHeatBalance.hh

-    int W5GapMatEQL = 0;                // Window5 Equivalent Layer Single-Gas Materials
-    int TotZoneAirBalance = 0;          // Total Zone Air Balance Statements in input
-    int TotFrameDivider = 0;            // Total number of window frame/divider objects
+    int NumOfZoneLists = 0;            // Total number of zone lists


When is something NumOf vs. Tot vs. nothing?

amirroth · 2022-07-26T12:10:56Z

src/EnergyPlus/HeatBalanceAirManager.cc

+                            if (zoneVolume > 0.0) {
+                                spaceFrac = thisSpace.Volume / zoneVolume;
+                            } else {
+                                ShowSevereError(state, std::string(RoutineName) + "Zone volume is zero when allocating Infiltration to Spaces.");


I understand that string + literal string is a special case, but this is still at best one reallocation more than format.

amirroth · 2022-07-26T12:13:33Z

src/EnergyPlus/HeatBalanceAirManager.cc

-                                    "The Minimum Indoor Temperature value and schedule are provided. The scheduled temperature will be used in the " +
-                                    cCurrentModuleObject + " object = " + cAlphaArgs(1));
+                            ShowWarningError(state,
+                                             std::string{RoutineName} +


If you find yourself doing std::string{RoutineName}, turn around and use format for the whole thing.

amirroth · 2022-07-26T12:16:09Z

src/EnergyPlus/SurfaceGeometry.cc

        for (ZoneNum = 1; ZoneNum <= state.dataGlobal->NumOfZones; ++ZoneNum) {
-
+            auto &thisZone = state.dataHeatBal->Zone(ZoneNum);
            if (!state.dataHeatBal->Zone(ZoneNum).HasFloor) {


amirroth · 2022-07-26T12:17:27Z

src/EnergyPlus/SurfaceGeometry.cc

@@ -12039,7 +12044,7 @@ namespace SurfaceGeometry {

        int countNotFullyEnclosedZones = 0;
        for (ZoneNum = 1; ZoneNum <= state.dataGlobal->NumOfZones; ++ZoneNum) {


If you're not going to use ZoneNum for anything other than find thisZone, you may as well define this loop as (thisZone : state.dataHeatBal->Zone). I can't believe I just suggested that. What's happening to me?

…owsInfVentMix

testfiles/1ZoneUncontrolled.idf

    Test 352a,               !- Name
    None,                    !- Fuel Type
-    ZONE ONE,                !- Zone or ZoneList Name
+    ZONE ONE,                !- Zone or ZoneList or Space or SpaceList Name


rraustad · 2022-08-03T23:30:06Z

testfiles/1ZoneUncontrolledUTF8.idf

    Test 352a,               !- Name
    None,                    !- Fuel Type
-    试验,                  !- Zone or ZoneList Name
+    试验,                  !- Zone or ZoneList or Space or SpaceList Name


I do not speak this language! Could be right, could be wrong.

rraustad · 2022-08-03T23:30:47Z

testfiles/1ZoneUncontrolled_Win_ASH55_Thermal_Comfort.idf

  People,
    ZONE ONE People,         !- Name
-    ZONE ONE,                !- Zone or ZoneList Name
+    ZONE ONE,                !- Zone or ZoneList or Space or SpaceList Name


This field name change needs to be described in Rules22-1-0-to-22-2-0.md

Same for People, Lights, and all *Equipment object. The field names changed before, this just brings the example files up to date, because I ran the entire set through transition so it cleaned up all the field names.

OK, I see. So these would show up in a previous change log.

OK, I see. So these would show up in a previous change log.

Correct. https://github.com/NREL/EnergyPlus/blob/develop/src/Transition/InputRulesFiles/Rules9-5-0-to-9-6-0.md

testfiles/1ZoneUncontrolled_Win_ASH55_Thermal_Comfort.idf

  Lights,
    SPACE1-1 Lights 1,       !- Name
-    ZONE ONE,                !- Zone or ZoneList Name
+    ZONE ONE,                !- Zone or ZoneList or Space or SpaceList Name


rraustad · 2022-08-04T00:07:28Z

I reviewed everything. Very good changes and a good learning experience in review. I think for this type of change we need to rely on example file results moreso than code review for results accuracy. I tried several variations of input changes to try and break the code. A few numerical differences that should be explained and some editorial work on the docs. Other than that, this feature looks good for a first pass.

Local unit tests pass
only diff is 5ZoneAirCooledWithSpaces due to input changes to test functionality in this branch, no diffs otherwise
~~unexplained numerical diffs could be posted as issue(s) since they may have been present prior to this branch~~
~~biggest editorial request is with Rules22-1-0-to-22-2-0.md~~
a unit test will iron out some of these numerical diffs
performance shows a 0.12% improvement

Overall nice job.

rraustad · 2022-08-04T14:07:33Z

I ran original 5ZoneAirCooled example file with the same ventilation object applied to Space1-1. The annual ventilation sum matches what is shown in the table above. So there is likely no issue with the table reports as first thought. I still do not understand what is wrong with the hand calc.

…owsInfVentMix

…ames

…owsInfVentMix

mjwitte · 2022-08-05T20:03:44Z

From #9564 (comment)

The answer here said that mechanical ventilation changed because sizing changed. OK, perfectly reasonble answer. However, when I look at the facility electricity, neither of these 2 tests with infiltration and ventilation changed electricity use compared to the original example file. Wouldn't you think that if you added outdoor air to the spaces that the facility electricity use would change?

Looking at the ABUPS report, electricity stays the same, but natural gas increases from 28.34 GJ to 28.55. So, if the OA fraction changes but the overall airflow stays the same, this would make sense. These values are for the file with 2 run periods, Jan 1-31, and July 1-31 (1488.00 hrs).

However, the annual values in the Energy Meters table output are nowhere near the values in ABUPS (even in develop).
Energy Meters Report Annual Electricity:Facility 23.53 GJ, ABUPS 36.25 GJ ?! Similar for Natural Gas.
I'll post a new issue for that. (Interesting, these values agree for 5ZoneAirCooled which has a single full annual RunPeriod. A clue.)

mjwitte · 2022-08-05T20:42:36Z

@rraustad Regarding hand calc comment #9564 (comment).
Hand calc was 417,484.8 m3, but the table result is 448,310.29 for a full year simulation.
The reason for the difference is that the ZoneVentilation volume flow rate input is applied to the outdoor air density, but the Outdoor Air Details report is in Standard Density. Even the "Zone Ventilation Current Density Volume Flow Rate" output is the current zone air density. So, there is no way to output the original specified volume flow rate. Thus #4422.

mjwitte · 2022-08-05T20:59:34Z

@Myoldmopar I believe that all of @rraustad 's comments have been addressed either in code or in comments. Assuming CI comes back all happy, this will be ready for your final review.

rraustad · 2022-08-05T21:02:36Z

@Myoldmopar I agree with @mjwitte assessment. This should be ready to go.

rraustad · 2022-08-05T21:24:21Z

tst/EnergyPlus/unit/HeatBalanceAirManager.unit.cc

+        EXPECT_EQ(thisInfiltration.DesignLevel, flows[itemNum]);
+        EXPECT_TRUE(UtilityRoutines::SameString(ventNames[itemNum], thisVentilation.Name));
+        EXPECT_EQ(thisVentilation.DesignLevel, flows[itemNum]);
+    }


This unit test that shows inputs to infiltration and ventilation are correctly interpreted/managed at the space level.

rraustad

This is a good addition to E+. Nice job!

Myoldmopar · 2022-08-06T16:15:33Z

Let's merge this in and perhaps the folks wanting to leverage this space stuff (@shorowit?) could play with it in their own workflows/tools, leaving us plenty of time to address anything before release. Thanks all!

mjwitte added 3 commits July 23, 2022 14:51

ZoneAirFlows by Space part 1 [decent_ci_skip]

0a2a0e1

ZoneAirFlows by Space part 3

393af09

ZoneAirFlows by Space transition

28068ed

mjwitte added NewFeature Includes code to add a new feature to EnergyPlus IDDChange Code changes impact the IDD file (cannot be merged after IO freeze) labels Jul 26, 2022

shorowit linked an issue Jul 26, 2022 that may be closed by this pull request

Allow Space-level infiltration/ventilation #9448

Closed

3 tasks

Fix infiltration input processing

a259497

amirroth reviewed Jul 26, 2022

View reviewed changes

mjwitte added 3 commits July 26, 2022 07:38

ZoneAirFlows by Space transition idfs

45d5ccc

ZoneAirFlows by Space transition unit test

e678219

ZoneAirFlows by Space transition unit test

62bdb4b

mjwitte added this to the EnergyPlus 22.2 IOFreeze milestone Jul 26, 2022

mjwitte added the OutputChange Code changes output in such a way that it cannot be merged after IO freeze label Jul 26, 2022

mjwitte added 10 commits July 26, 2022 11:24

ZoneAirFlows by Space repairs

1ed8deb

ZoneAirFlows by Space transition idfs again

e851824

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

ba96c29

…owsInfVentMix

Single space volume equal zone volume

ce0756f

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

a0df204

…owsInfVentMix

ZoneAirFlows move Quadrature flag to zone plus cleanups

bd7fe2d

ZoneAirFlows by Space - docs

65353e8

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

5a1638d

…owsInfVentMix

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

65bf351

…owsInfVentMix

Fix Zone Air Balance

b8e8a20

rraustad reviewed Aug 3, 2022

View reviewed changes

testfiles/1ZoneUncontrolled.idf

Test 352a, !- Name

None, !- Fuel Type

ZONE ONE, !- Zone or ZoneList Name

ZONE ONE, !- Zone or ZoneList or Space or SpaceList Name

This comment was marked as resolved.

Sign in to view

rraustad reviewed Aug 3, 2022

View reviewed changes

testfiles/1ZoneUncontrolled_Win_ASH55_Thermal_Comfort.idf

Lights,

SPACE1-1 Lights 1, !- Name

ZONE ONE, !- Zone or ZoneList Name

ZONE ONE, !- Zone or ZoneList or Space or SpaceList Name

This comment was marked as resolved.

Sign in to view

mjwitte self-assigned this Aug 4, 2022

mjwitte added 2 commits August 4, 2022 16:08

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

40d0a32

…owsInfVentMix

Address doc comments

40171bf

mjwitte mentioned this pull request Aug 4, 2022

Space for Infiltration Ventilation and Mixing followup comments #9587

Open

7 tasks

mjwitte added 9 commits August 4, 2022 17:33

Address some comments [decent_ci_skip]

5628d37

Address some comments [decent_ci_skip]

ddcdcd7

Unrequire Schedule for infiltration and vent and fix clean up field n…

cc5cb2c

…ames

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

d6ba993

…owsInfVentMix

Infiltration and Ventilation getinput unit test

4827ff1

repairs found via unit test and other cleanup

1c0d8ac

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

f41c2ad

…owsInfVentMix

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

fd2de53

…owsInfVentMix

Unit test cleanup

2159080

rraustad reviewed Aug 5, 2022

View reviewed changes

rraustad approved these changes Aug 5, 2022

View reviewed changes

Myoldmopar merged commit 569967d into develop Aug 6, 2022

Myoldmopar deleted the spaceAirFlowsInfVentMix branch August 6, 2022 16:15

shorowit mentioned this pull request Aug 8, 2022

Update FT for space-level infiltration/ventilation objects NatLabRockies/OpenStudio#4645

Closed

mjwitte mentioned this pull request Aug 31, 2022

Add volume property to space object #9362

Closed

		@@ -12039,7 +12044,7 @@ namespace SurfaceGeometry {

		int countNotFullyEnclosedZones = 0;
		for (ZoneNum = 1; ZoneNum <= state.dataGlobal->NumOfZones; ++ZoneNum) {

Comments

Conversation

mjwitte commented Jul 26, 2022 • 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!

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!

This comment was marked as resolved.

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!

This comment was marked as resolved.

Uh oh!

rraustad commented Aug 4, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rraustad commented Aug 4, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mjwitte commented Aug 5, 2022

Uh oh!

mjwitte commented Aug 5, 2022

Uh oh!

mjwitte commented Aug 5, 2022

Uh oh!

rraustad commented Aug 5, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rraustad left a comment

Choose a reason for hiding this comment

Uh oh!

Myoldmopar commented Aug 6, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

mjwitte commented Jul 26, 2022 •

edited

Loading

rraustad commented Aug 4, 2022 •

edited

Loading

rraustad commented Aug 4, 2022 •

edited

Loading