Fix space sizing output (spsz) when there is no space HVAC equipment #10947

mjwitte · 2025-02-20T23:44:02Z

Pull request overview

Fixes Space sizing spsz output file is not populated when space sizing is active but there is no space HVAC equipment #10946

Description of the purpose of this PR

Write the spsz output file anytime "Do Space Heat Balance for Sizing" is Yes. Previously it was only written if the idf also included SpaceHVAC:* objects.
Avoid opening and leaving an empty spsz output file if it will not be filled.
~~Update some related IDD \memo lines.~~ (held for v25.2)
Add check with fatal error if SpaceHVAC:EquipmentConnections object is used for a space in an uncontrolled zone.
Some code cleanup in GetZoneEquipmentData.
Some I/O Ref cleanups

Pull Request Author

Reviewer

mjwitte · 2025-02-21T03:44:43Z

tst/EnergyPlus/unit/SizingManager.unit.cc

              OutputReportPredefined::RetrievePreDefTableEntry(*state, state->dataOutRptPredefined->pdchZnClDesDay, "ZONE 1"));
    EXPECT_EQ("7/21 16:00:00", OutputReportPredefined::RetrievePreDefTableEntry(*state, state->dataOutRptPredefined->pdchZnClPkTime, "ZONE 1"));
+    // Expect output to spsz file
+    EXPECT_FALSE(state->files.spsz.get_output().empty());


This doesn't work. Tried several different ways to check the contents of this file.

mjwitte · 2025-02-21T03:46:29Z

Tested with example files:
5ZoneAirCooledWithSpaces - no spsz output before and after fix
5ZoneAirCooledWithSpaceHeatBalance - spsz output is now fully populated
5ZoneAirCooledWithSpacesHVAC - spsz output fully populated before and after fix

rraustad · 2025-02-21T14:10:23Z

This probably doesn't need any attention but I noticed that, for 5ZoneAirCooledWithSpaceHeatBalance which has auto generated spaces, the header for epluszsz and eplusspsz look the same. Reported as Zone/Space Name ":" Design Day Name ":" Variable Name, so am just noting it here.

Then I checked eplusout.csv and the spaces have their own report names which allows proper reporting (i.e., if they had the same report name they would be duplicates and excel would likely not show the duplicate, they would be in the csv but excel might? get confused). Again, probably nothing to do here.

UPDATE: I have noticed this "duplicate" issue before but it happened when the cooling design day and heating design day date were the same (e.g., 1/21 for cooling and heating), so the header should report accurately). It was the rows that did not show up correctly, not the columns.

rraustad · 2025-02-21T14:29:27Z

Tested 5ZoneAirCooledWithSpaceHeatBalance before and after this fix. This is eplusspsz.csv:

rraustad · 2025-02-21T14:36:28Z

Also tested 5ZoneAirCooledWithSpaceHeatBalance with Do Space Heat Balance for Sizing set as Yes and No. eplusspsz.csv is blank when set to No so this works as expected.

ZoneAirHeatBalanceAlgorithm,
  ThirdOrderBackwardDifference,  !- Algorithm
  No,                      !- Do Space Heat Balance for Sizing
  Yes;                      !- Do Space Heat Balance for Simulation

rraustad · 2025-02-21T15:49:08Z

Just out of curiosity of why the unit test didn't work, at the end of writeZszSpsz the spsz file is closed. Just before it's closed os = unique_ptr, after it's closed os = empty. It's the os pointer that is getting tested in state->files.spsz.get_output().empty()

rraustad · 2025-02-21T19:06:30Z

src/EnergyPlus/ZoneEquipmentManager.cc

+        if (forSpaces) {
+            zoneNum = state.dataHeatBal->space(i).zoneNum;
+        }
+        if (!state.dataHeatBal->Zone(zoneNum).IsControlled) continue;


Only if you have any other changes.

int zoneNum = (forSpaces) ? state.dataHeatBal->space(i).zoneNum : i;

Yes, of course. That construct still is not my first thought, or second, or ...

To add on, the parentheses around forSpaces is not needed unless you need it for order of operations. (Ternary has a very low operation precedence, so most things will be evaluated before it is applied.)

rraustad

These changes correct the issue.

mjwitte

Walkthru

mjwitte · 2025-02-21T20:14:38Z

src/EnergyPlus/SizingManager.cc

        state.dataGlobal->ZoneSizingCalc = true;
        Available = true;

-        if (state.dataSize->SizingFileColSep == CharComma) {


Move to ZoneEquipmentManager to avoid creating an empty spsz output file. Can't make that decision here yet, because the project data (with Do Space Heat Balance flags) has not been read yet.

mjwitte · 2025-02-21T20:15:08Z

src/EnergyPlus/ZoneEquipmentManager.cc

 void writeZszSpsz(EnergyPlusData &state,
                  EnergyPlus::InputOutputFile &outputFile,
                  int const numSpacesOrZones,
-                  Array1D<DataZoneEquipment::EquipConfiguration> const &zsEquipConfig,


This is no longer needed.

mjwitte · 2025-02-21T20:16:40Z

src/EnergyPlus/ZoneEquipmentManager.cc

-        if (!zsEquipConfig(i).IsControlled) continue;
+        int zoneNum = (forSpaces) ? state.dataHeatBal->space(i).zoneNum : i;
+        if (!state.dataHeatBal->Zone(zoneNum).IsControlled) continue;


For sizing output, check if the parent zone is controlled instead of checking zsEquipConfig(i).IsControlled (which is only true if there are SpaceHVAC:* objects).

And if there are SpaceHVAC objects, that implies that the parent zone IsControlled flag will be set to true, right?

Well, it should be. SpaceHVAC objects don't work without corresponding ZoneHVAC objects for the parent zone. I see a check for that in some of the SpaceHVAC objects, but not for SpaceHVAC:EquipmentConnections. I'll add that and remove the IDD changes and stash them (somewhere) for later.

How does this code in SizeZoneEquipment know the space is controlled? and if this isn't working as expected for spaces then uncontrolled spaces will be held to the Tstat temp and not float?

auto &zoneEquipConfig = state.dataZoneEquip->ZoneEquipConfig(ControlledZoneNum); if (!zoneEquipConfig.IsControlled) continue; sizeZoneSpaceEquipmentPart2( state, zoneEquipConfig, state.dataSize->CalcZoneSizing(state.dataSize->CurOverallSimDay, ControlledZoneNum), ControlledZoneNum); if (state.dataHeatBal->doSpaceHeatBalance) { for (int spaceNum : state.dataHeatBal->Zone(ControlledZoneNum).spaceIndexes) { sizeZoneSpaceEquipmentPart2( state, zoneEquipConfig, state.dataSize->CalcSpaceSizing(state.dataSize->CurOverallSimDay, spaceNum), ControlledZoneNum, spaceNum); } }

I just checked 5ZoneAirCooledWithSpacesHVAC and ZONE 5-REMAINDER has a heating and cooling load showing in eplusspsz.csv. This is a different issue than the original and is scope creeping.

UPDATE: ZONE 5-REMAINDER does have an equipment connections object so it should show loads. I suspect an uncontrolled space would also show loads but that's a different issue than this.

SpaceHVAC:EquipmentConnections, Zone 5-Remainder, !- Space Name

For sizing, the assumption is that all spaces in a zone are controlled to the zone thermostat setpoint. That's intentional, I'll have to check and see if the docs are clear (or cloudy, or dark as night) on that.

The Engineering Ref seems clear:

\subsection{Space Sizing}\label{space-sizing} When ZoneAirHeatBalanceAlgorithm ``Do Space Heat Balance for Sizing'' is ``Yes'', the same sizing calculations described above will be performed for each space that is part of a controlled zone, using the same thermostat setpoints as the parent zone. The space sizing results will be reported the same as zone sizing results (eio, table, and spsz outputs). \subsection{NonCoincident Zone Sizing}\label{noncoincident-zone-sizing} Sizing:Zone has an option for ``Type of Space Sum to Use'', ``Coincident'' or ``NonCoincident''. Coincident zone sizing (the default) is always calculated first, with all spaces in the zone lumped together. For ``NonCoincident'' zone sizing, if the zone contains more than one space, the zone sizing results will be overwritten using the sums and averages of the space sizing results. If all spaces for a given load type (heating or cooling) peak on the same design day, then that day will be reported as the zone peak day, otherwise the zone design day will be "N/A". The zone peak time will be determined by scanning the peak zone sequential loads which are calculated by summing the space peak day sequential loads.

But some I/O Ref changes are needed.

mjwitte · 2025-02-21T20:18:13Z

src/EnergyPlus/ZoneEquipmentManager.cc

+                } else {
+                    state.files.spsz.filePath = state.files.outputSpszTxtFilePath;
+                }
+                state.files.spsz.ensure_open(state, "UpdateZoneSizing", state.files.outputControl.spsz);


Wait until here to open zsz and spsz files. At this point we know if doSpaceHeatBalanceSizing is active.

Actually glad you fixed this. I saw the 0 kB files but wasn't sure how to avoid that. This is a perfect fix.

Myoldmopar

This looks fine. I've just got to decide what to do about the IDD changes. They are tiny, memo only changes. But they still change the IDD, and we've been pretty strict in the past about that.

Myoldmopar · 2025-02-26T14:34:11Z

idd/Energy+.idd.in

       \memo space air node, air inlet nodes, air exhaust nodes, and the air return node.
       \memo If any space in a zone has a SpaceHVAC:EquipmentConnections object, then all spaces in the zone must have one.
-       \memo Used only when ZoneAirHeatBalanceAlgorithm "Do Space Heat Balance for Sizing"is Yes.
+       \memo Used only when ZoneAirHeatBalanceAlgorithm "Do Space Heat Balance for Simulation" is Yes.


How strict shall I be regarding IO changes... need to decide...

Myoldmopar · 2025-02-26T14:34:27Z

src/EnergyPlus/SizingManager.cc

        state.dataGlobal->ZoneSizingCalc = true;
        Available = true;

-        if (state.dataSize->SizingFileColSep == CharComma) {


Myoldmopar · 2025-02-26T14:37:06Z

src/EnergyPlus/ZoneEquipmentManager.cc

-        if (!zsEquipConfig(i).IsControlled) continue;
+        int zoneNum = (forSpaces) ? state.dataHeatBal->space(i).zoneNum : i;
+        if (!state.dataHeatBal->Zone(zoneNum).IsControlled) continue;


And if there are SpaceHVAC objects, that implies that the parent zone IsControlled flag will be set to true, right?

Myoldmopar · 2025-02-26T14:40:41Z

src/EnergyPlus/ZoneEquipmentManager.cc

+        if (forSpaces) {
+            zoneNum = state.dataHeatBal->space(i).zoneNum;
+        }
+        if (!state.dataHeatBal->Zone(zoneNum).IsControlled) continue;


To add on, the parentheses around forSpaces is not needed unless you need it for order of operations. (Ternary has a very low operation precedence, so most things will be evaluated before it is applied.)

…Sizing

Myoldmopar · 2025-02-26T17:26:42Z

Thanks for cleaning up the IDD stuff. I think it's ready to go now. Will let CI confirm and then be ready to merge in a bit.

mjwitte · 2025-02-26T17:30:25Z

src/EnergyPlus/DataZoneEquipment.cc

+        if (!state.dataHeatBal->Zone(zoneNum).IsControlled) {
+            ShowSevereError(state, format("{}{}=\"{}\"", RoutineName, CurrentModuleObject, thisSpace.Name));
+            ShowContinueError(state,
+                              format("..Zone Name={} is not a controlled zone. A ZoneHVAC:EquipmentConnections object is required for this zone.",


Made a modified version of 5ZoneAirCooledWithSpacesHVAC that has only SpaceHVAC:EquipmentConnections objects for the spaces in zone 5 (remove the ZoneHVAC:EquipmentConnections object and the other types of SpaceHVAC:* objects). This file crashes with develop, and fatals with this branch:

* Severe ** GetZoneEquipmentData: SpaceHVAC:EquipmentConnections="SPACE 5 OFFICE" ** ~~~ ** ..Zone Name=ZONE 5 is not a controlled zone. A ZoneHVAC:EquipmentConnections object is required for this zone. ** Severe ** GetZoneEquipmentData: SpaceHVAC:EquipmentConnections="SPACE 5 CONFERENCE" ** ~~~ ** ..Zone Name=ZONE 5 is not a controlled zone. A ZoneHVAC:EquipmentConnections object is required for this zone. ** Severe ** GetZoneEquipmentData: SpaceHVAC:EquipmentConnections="ZONE 5-REMAINDER" ** ~~~ ** ..Zone Name=ZONE 5 is not a controlled zone. A ZoneHVAC:EquipmentConnections object is required for this zone. ** Warning ** GetZoneEquipmentData: SpaceHVAC:EquipmentConnections, duplicate items NOT CHECKED due to previous errors. ** Fatal ** GetZoneEquipmentData: Errors found in getting Zone Equipment input.

5ZoneAirCooledWithSpacesHVAC-TestAZoneNotControlled2.idf.txt

Myoldmopar · 2025-02-26T19:21:04Z

Good to go, thanks @mjwitte

mjwitte added 2 commits February 20, 2025 17:29

Update SpaceHVAC idd notes

cccac75

Unit test for spsz output

21a94a3

mjwitte added Defect Includes code to repair a defect in EnergyPlus IDDChange Code changes impact the IDD file (cannot be merged after IO freeze) labels Feb 20, 2025

mjwitte added this to the EnergyPlus 25.1 IO Freeze milestone Feb 20, 2025

Fix spsz output

7d3f762

mjwitte marked this pull request as ready for review February 21, 2025 03:43

mjwitte commented Feb 21, 2025

View reviewed changes

cleanup

b3919bb

rraustad reviewed Feb 21, 2025

View reviewed changes

rraustad approved these changes Feb 21, 2025

View reviewed changes

Use conditional assignment and avoid empty spsz output

1ffab86

mjwitte commented Feb 21, 2025

View reviewed changes

mjwitte modified the milestones: EnergyPlus 25.1 IO Freeze, EnergyPlus 25.1 Feb 24, 2025

Myoldmopar reviewed Feb 26, 2025

View reviewed changes

mjwitte added 4 commits February 26, 2025 09:45

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

ab205cd

…Sizing

Revert idd changes

3378c6f

Clean up ZoneHVAC/SpaceHVAC EquipmentConnections input processing

40c76ba

Doc updates

fc99428

mjwitte removed the IDDChange Code changes impact the IDD file (cannot be merged after IO freeze) label Feb 26, 2025

mjwitte commented Feb 26, 2025

View reviewed changes

Myoldmopar merged commit d2c044e into develop Feb 26, 2025
10 checks passed

Myoldmopar deleted the 10946SpaceSizing branch February 26, 2025 19:21

Fix space sizing output (spsz) when there is no space HVAC equipment #10947

Fix space sizing output (spsz) when there is no space HVAC equipment #10947

Uh oh!

Conversation

mjwitte commented Feb 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull request overview

Description of the purpose of this PR

Pull Request Author

Reviewer

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mjwitte commented Feb 21, 2025

Uh oh!

rraustad commented Feb 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rraustad commented Feb 21, 2025

Uh oh!

rraustad commented Feb 21, 2025

Uh oh!

rraustad commented Feb 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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!

rraustad left a comment

Choose a reason for hiding this comment

Uh oh!

mjwitte 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!

rraustad Feb 26, 2025 • 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!

mjwitte Feb 26, 2025 • 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!

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!

Myoldmopar commented Feb 26, 2025

Uh oh!

mjwitte Feb 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

mjwitte commented Feb 20, 2025 •

edited

Loading

rraustad commented Feb 21, 2025 •

edited

Loading

rraustad commented Feb 21, 2025 •

edited

Loading

rraustad Feb 26, 2025 •

edited

Loading

mjwitte Feb 26, 2025 •

edited

Loading

mjwitte Feb 26, 2025 •

edited

Loading