Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Extend Spaces to Sizing and HVAC #9982

Merged
merged 64 commits into from
Aug 23, 2023
Merged

Extend Spaces to Sizing and HVAC #9982

merged 64 commits into from
Aug 23, 2023

Conversation

mjwitte
Copy link
Contributor

@mjwitte mjwitte commented Apr 26, 2023

Pull request overview

  • Partially address Extend Spaces to Sizing #8927
    • When ZoneAirHeatBalanceAlgorithm "Do Space Heat Balance for Sizing" = Yes, zone sizing is also done for all spaces. The HVAC Sizing Summary table report will include subtables for Space Sensible Cooling and Heating as well as for Zone Sensible Cooling and Heating. Space Sizing will also be reported to the eio output.
    • At this stage, the space sizing results are reported, but not used.
    • Zone sizing remains unchanged, sizing for all spaces in the zone lumped together which is essentially the same as the coincident space load.
  • Partially addresses Extend Spaces to HVAC #8928
    • Adds two new objects SpaceHVAC:EquipmentConnections and SpaceHVAC:ZoneEquipmentSplitter.
    • These objects allow one or more zones to be simulated at the space level for HVAC, leading to independent air temperature and humidity in each space.
  • TODO in this pull request:
    • Review zone vs space calcs carefully.
    • Finalize IDD
  • TODO in future pull requests for v23.2:
    • Add unit tests.
    • Add engineering ref notes.
    • Use the space exhaust and return nodes?
    • Set zone temperature and humidity based on lumping spaces together.
    • Change surface heat balance to use space air conditions instead of zone.
    • For zones without space heat balance set all space conditions to the zone condition.
    • Use space return node fields
    • Add checks and warnings for various constraints (e.g. if one space has SpaceHVAC then all spaces in that zone must also).
  • TODO in future pull requests post-v23.2:
    • Allow zone equipment sizing to be based on coincident or non-coincident space loads. - IDD change

Pull Request Author

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)
  • Label the PR with at least one of: Defect, Refactoring, NewFeature, Performance, and/or DoNoPublish
  • Pull requests that impact EnergyPlus code must also include unit tests to cover enhancement or defect repair
  • Author should provide a "walkthrough" of relevant code changes using a GitHub code review comment process
  • If any diffs are expected, author must demonstrate they are justified using plots and descriptions
  • If changes fix a defect, the fix should be demonstrated in plots and descriptions
  • If any defect files are updated to a more recent version, upload new versions here or on DevSupport
  • If IDD requires transition, transition source, rules, ExpandObjects, and IDFs must be updated, and add IDDChange label
  • If structural output changes, add to output rules file and add OutputChange label
  • If adding/removing any LaTeX docs or figures, update that document's CMakeLists file dependencies

Reviewer

This will not be exhaustively relevant to every PR.

  • Perform a Code Review on GitHub
  • If branch is behind develop, merge develop and build locally to check for side effects of the merge
  • If defect, verify by running develop branch and reproducing defect, then running PR and reproducing fix
  • If feature, test running new feature, try creative ways to break it
  • CI status: all green or justified
  • Check that performance is not impacted (CI Linux results include performance check)
  • Run Unit Test(s) locally
  • Check any new function arguments for performance impacts
  • Verify IDF naming conventions and styles, memos and notes and defaults
  • If new idf included, locally check the err file and other outputs

@mjwitte 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 Apr 26, 2023
@mjwitte mjwitte added this to the EnergyPlus 23.2 IOFreeze milestone Apr 26, 2023
```

### ZoneHVAC:SpaceSplitter
* *New object, splits the air flow from a single piece of zone equipment to one or more Spaces.*
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How does the air loop splitter know how much air goes to each zone without these fraction inputs? The zone air flows create these fractions among a group of zones (i.e., it's not the splitter that does this). And if there is a splitter doesn't there need to be a mixer? My point is that I think you can use the existing splitter/mixer objects just like the air loop. The only question I have is the mechanism to get the zone (space) flow rates back to the outlet nodes of the splitter (space splitter). The answer is buried in the node maxavail data.

Copy link
Contributor Author

@mjwitte mjwitte Apr 26, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good questions. A diagram would be helpful. Currently, let's say we have a 2-zone airloop.

AirloopOutlet --> ZoneSplitter --> Zone1 ATU Inlet --> Zone1 ATU Outlet --> Zone 1
                              |--> Zone2 ATU Inlet --> Zone2 ATU Outlet --> Zone 2                                 

The ATUs control the flow and set maxavail on the ATU Inlet nodes which gets passed back up to the airloop.
The proposed method keeps all of the same nodes as above but adds the SpaceSplitter between the ATU outlet and the Space inlet nodes.

AirloopOutlet --> ZoneSplitter --> Zone1 ATU Inlet --> Zone1 ATU Outlet --> SpaceSplitter1--> Space 1a Supply Inlet --> Space 1a (Zone 1)
                              |                                                          |--> Space 1b Supply Inlet --> Space 1b (Zone 1)
                              |--> Zone2 ATU Inlet --> Zone2 ATU Outlet --> SpaceSplitter2--> Space 2a Supply Inlet (Zone 2)

So, the flow controls for the main airloop would stay the same, and the SpaceSplitter would divide up the flow to the various space diffuser outlets (which are completely passive with no controls on them). If the thermostat is located in a specific space, then the ATU will control total flow to give the controlling space the flow it needs (kind of the way unitary uses the control zone flow fraction to pro-rate the output). I'm sure there are some holes in the scheme yet.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the first diagram, Zone1 and Zone 2 have design flow rates. The air loop, and the splitter, do not know those flow rates, yet the correct amount of air gets to each zone. In the second diagram, Space 1a, Space 1b, and Space 2a all have design flow rates. I don't see what's changed to need a new object. The existing splitter divides up flows without the need for flow fraction inputs. The only difference here is a ganged splitter configuration but flow is still determined from the design flow rates. Maybe in the Splitter code, space splitters are modeled first so that the zone splitters know the same information known now at the zone splitter outlet nodes. I agree you could do this either way with the same result. I think all you would need to do is place the space MassFlowRateMax and MaxAvail on the space inlet node and the existing splitter object works the same way it does now. Then model space splitters first to pass the information up to the zone splitters. Easier said than done?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As proposed, there is no object or field to specify space design flow rates (or user specified flow rates/fractions) for the space inlets, except the SpaceSplitter. If we put a ZoneSplitter there, where would the space-level flow information reside?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The proposed "Do Space Heat Balance for Sizing" tentatively means that given a space load you would pass a supply air T and w (from Sizing:Zone) to the space and calculate the space air flow rate. A Tstat is needed in the space for this method or the single Tstat within the zone could be used, to get the space SPs. I'm not sure if this proposal intends to do that or not but if it does you have the space air flows and therefore the space fractions. If the space air flows are not calculated and the space loads are calculated, the space loads provide the space fractions. This seems tricky for non-coincident sizing since the space peak times may not line up with the zone peak time, and then what would happen? I think here non-coincident applies to zones, not spaces.

I assume the intent is to split the zone air flow into space flows at fixed fractions, and those flows sum to meet the coincident space peak load and air flow that match the zone. If a sizing run is done then you have the information needed to calculate the fractions. If the intent is to allow the user to "test and balance" the space diffusers (likely before they know the zone/space loads) then you would need an input. So it seems you only need these fractions when sizing is not performed. How about the AirLoopHVAC:ZoneSplitter? A single input for a space name/fraction pair list is required if the inlet node connects to a splitter and all outlet nodes connect to space inlet nodes. After doing this I'm not sure this is any better. The last 2 objects are really no better than your original suggestion except that when autosizing the FlowFractionList isn't needed. I'm also not sure what a 3rd party front end would want, use of the same object, or a specific object per use case. Proceed at will. Another thought, for non-autosized, you know the space loads prior to simulating the air systems and there is also an opportunity to calculate the space fractions. But then the space fractions would vary and that would be wierd (but all set points would be met!). Consider this discussion food for thought.

AirLoopHVAC:ZoneSplitter,
  Zone Supply Air Splitter 1,  !- Name
  ,                        !- Space Name and Flow Fraction List
  Zone Eq In Node,         !- Inlet Node Name
  SPACE1-1 Space Splitter, !- Outlet 1 Node Name
  SPACE2-1 ATU In Node,    !- Outlet 2 Node Name
  SPACE3-1 ATU In Node,    !- Outlet 3 Node Name
  SPACE4-1 ATU In Node,    !- Outlet 4 Node Name
  SPACE5-1 ATU In Node;    !- Outlet 5 Node Name

AirLoopHVAC:ZoneSplitter,
  Zone Space1-1 Air Splitter,  !- Name
  SPACE1-1 Flow Fraction List, !- Space Name and Flow Fraction List
  SPACE1-1 Space Splitter,     !- Inlet Node Name
  SPACE1-1a ATU In Node,       !- Outlet 1 Node Name
  SPACE1-1b ATU In Node;       !- Outlet 2 Node Name

ZoneHVAC:FlowFractionList,
  SPACE1-1 Flow Fraction List,  !- Name
  0.4,                          !- Space Fraction for Outlet 1 Node
  0.6;                          !- Space Fraction for Outlet 2 Node

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just looked at the Space object and a flow fraction input would work there as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume the intent is to split the zone air flow into space flows at fixed fractions, and those flows sum to meet the coincident space peak load and air flow that match the zone. If a sizing run is done then you have the information needed to calculate the fractions.

Yes, if autosized.

. If the intent is to allow the user to "test and balance" the space diffusers (likely before they know the zone/space loads) then you would need an input.

Either way, we need an autosizable input somewhere. Same as VAV terminal unit supply flow rate.

\type node
A3 ; \field Space Air Inlet Node or NodeList Name
\type node
```
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or even here is better to add the flow fraction? I assume unconditioned spaces are not allowed in a conditioned zone so each space would have this object.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or even here is better to add the flow fraction? I assume unconditioned spaces are not allowed in a conditioned zone so each space would have this object.

Maybe, but this would not allow different balancing for different equipment. I'll work on expanding the NFP and adding some diagrams to consider some options here.

Also, I started with a space equipment list to allow space-level HVAC equipment, then decide that wasn't necessary, but I probably need to allow that back in, because a fan coil, say, would rarely serve more than one room (space).

As a side note, I always find the reverse maxavail flow resolution in the airloops to be rather opaque (i.e. I can't follow it in the code). So my goal is to keep this as simple and straightforward as possible.

@nrel-bot-2c
Copy link

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

\units dimensionless
\minimum 0.0
\maximum 1.0
\autosizable
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should probably run these extensible fields out to about 10 in the idd when implemented. I can see a hallway with 5 or more spaces on a single zone on each side of the hallway.

\required-field
\type node
A3 ; \field Space Air Inlet Node or NodeList Name
\type node
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did not understand this on the first go around. Why doesn't a space have an exhaust or return outlet node? Will these nodes be implied as connecting to a zone exhaust/return outlet? and how to know which space has which outlet type? If an air loop connects to these spaces then yes, the air flow would sum to a zone return node. If a PTAC is connected to a space then the space would need an exhaust node. Also need a space exhaust node for "space" exhaust fans.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I was hoping to avoid space-level return/exhaust nodes, but that might not work out.

Zone,Sum,Zone Heating Setpoint Not Met While Occupied Time [hr]
Zone,Sum,Zone Cooling Setpoint Not Met Time [hr]
Zone,Sum,Zone Cooling Setpoint Not Met While Occupied Time [hr]
```
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These Setpoint Not Met Time output variables seem important. Seems like a clear method for how the space level results get rolled up to the zone level will be needed, eg. average temperature for zone first and then compare to setpoint, or is zone unmet if any spaces are unmet.

@nrel-bot
Copy link

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

Copy link
Contributor Author

@mjwitte mjwitte left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code walkthru part 1

@@ -453,6 +453,9 @@ namespace DataHeatBalance {
Real64 ExtGrossWallArea = 0.0; // Exterior Wall Area for Zone (Gross)
Real64 ExteriorTotalSurfArea = 0.0; // Total surface area of all exterior surfaces for Zone
int SystemZoneNodeNumber = 0; // This is the zone or space node number for the system for a controlled zone
Real64 FloorArea = 0.0; // Floor area used for this space
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move these to be available for both zone and space under the same field names. Changes space floorArea to ZoneSpace FloorArea which cause a bunch of case changes anywhere space floorArea was used. Same for totOccupants to TotOccupants.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know it's really difficult to predict refactors like this, but it could help to take a quick look through the code you want to touch in your next phase, and quickly do some renaming in a first-pass PR before functional changes. I get that it won't be obvious at first, just saying that would make explaining and digesting the PR changes easier all around.

@@ -314,6 +314,100 @@ void ZoneSizingData::allocateMemberArrays(int const numOfTimeStepInDay)
this->LatentHeatFlowSeq.dimension(numOfTimeStepInDay, 0.0);
}

void TermUnitZoneSizingData::allocateMemberArrays(int const numOfTimeStepInDay)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some new methods for the new struct TermUnitZoneSizingData which is a subset of ZoneSizingData.

@@ -594,15 +594,104 @@ namespace DataSizing {
ZoneSizing zoneSizingMethod = ZoneSizing::Invalid; // load to sizing on: sensible, latent, sensibleandlatent, sensibleonlynolatent
std::string CoolSizingType; // string reported to eio, Cooling or Latent Cooling
std::string HeatSizingType; // string reported to eio, Heating or Latent Heating
std::string CoolPeakDateHrMin; // date:hr:min of cooling peak
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These were loose arrays, moved them into the ZoneSizingData struct.

void zeroMemberData();
void allocateMemberArrays(int numOfTimeStepInDay);
};

// based on ZoneSizingData but only member variables that are actually used by terminal unit sizing
struct TermUnitZoneSizingData
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

New struct, subset of ZoneSizingData.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So does ZoneSizingData inherit this subset class now so that these variables aren't repeated?

Comment on lines +1345 to +1348
Array2D<DataSizing::ZoneSizingData> SpaceSizing; // Data for space sizing (all data, all design)
EPVector<DataSizing::ZoneSizingData> FinalSpaceSizing; // Final data for space sizing including effects
Array2D<DataSizing::ZoneSizingData> CalcSpaceSizing; // Data for space sizing (all data)
EPVector<DataSizing::ZoneSizingData> CalcFinalSpaceSizing; // Final data for space sizing (calculated only)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

New space sizing arrays.

state.dataSize->ZoneSizThermSetPtHi(state.dataSize->CurZoneEqNum));
state.dataSize->TermUnitFinalZoneSizing(state.dataSize->CurTermUnitSizingNum).ZoneSizThermSetPtHi);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moving some of the loose arrays into the TermUnitFinalZoneSizing struct caused a bunch of changes like this all over the HVAC code.

Comment on lines -300 to +301
if (state.dataHeatBal->doSpaceHeatBalanceSizing || state.dataHeatBal->doSpaceHeatBalanceSimulation) {
state.dataHeatBal->spaceAirRpt.allocate(state.dataGlobal->NumOfZones);
if (state.dataHeatBal->doSpaceHeatBalanceSimulation) {
state.dataHeatBal->spaceAirRpt.allocate(state.dataGlobal->numSpaces);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some related cleanup of existing code.

@@ -438,6 +438,26 @@ namespace OutputReportPredefined {

s->pdrSizing = newPreDefReport(state, "HVACSizingSummary", "Size", "HVAC Sizing Summary");

s->pdstSpaceClSize = newPreDefSubTable(state, s->pdrSizing, "Space Sensible Cooling");
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

New space sizing output tables.

@@ -592,180 +583,36 @@ void ManageSizing(EnergyPlusData &state)

// report sizing results to eio file
if (state.dataSize->ZoneSizingRunDone) {
bool isSpace = true;
if (state.dataHeatBal->doSpaceHeatBalanceSizing) {
for (int spaceNum = 1; spaceNum <= state.dataGlobal->numSpaces; ++spaceNum) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Report space sizing, then zone sizing, to both eio and table.

for (int CtrlZoneNum = 1; CtrlZoneNum <= state.dataGlobal->NumOfZones; ++CtrlZoneNum) {
if (!state.dataZoneEquip->ZoneEquipConfig(CtrlZoneNum).IsControlled) continue;
if (state.dataSize->FinalZoneSizing(CtrlZoneNum).DesCoolVolFlow > 0.0) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved a bunch to shared reportZoneSizing function.

Copy link
Contributor Author

@mjwitte mjwitte left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code walkthru part 2 of 2.

@@ -380,6 +380,305 @@ void InitZoneEquipment(EnergyPlusData &state, bool const FirstHVACIteration) //
airLoopFlow.ExcessZoneExhFlow = 0.0;
}
}
void sizeZoneSpaceEquipmentPart1(EnergyPlusData &state,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Split SizeZoneEquipment into pieces that can be re-used for zones and spaces.

Comment on lines +3196 to +3197
if (state.dataHeatBal->doSpaceHeatBalanceSimulation && !state.dataGlobal->DoingSizing) {
for (int spaceNum : state.dataHeatBal->Zone(ControlledZoneNum).spaceIndexes) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Zero some things at the space level prior to simulating zone equipment.

@@ -3461,17 +3624,47 @@ void SimZoneEquipment(EnergyPlusData &state, bool const FirstHVACIteration, bool
}
}

UpdateSystemOutputRequired(state, ControlledZoneNum, SysOutputProvided, LatOutputProvided, EquipTypeNum);
// If SpaceHVAC is active and this equipment has a space splitter, distribute the equipment output and update the spaces
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apply the SpaceHVAC:ZoneEquipmentSplitter for this piece of zone equip.

TotInletAirMassFlowRateMaxAvail += thisNode.MassFlowRateMaxAvail;
TotInletAirMassFlowRateMin += thisNode.MassFlowRateMin;
TotInletAirMassFlowRateMinAvail += thisNode.MassFlowRateMinAvail;
// SpaceHVAC TODO: For now, ZoneMassBalance happens at the zone level, not space
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note

@@ -4365,12 +4560,6 @@ void CalcZoneMassBalance(EnergyPlusData &state, bool const FirstHVACIteration)
CalcZoneMixingFlowRateOfReceivingZone(state, ZoneNum, ZoneMixingAirMassFlowRate);
ZoneMixingNetAirMassFlowRate = massConservation.MixingMassFlowRate - massConservation.MixingSourceMassFlowRate;
}
auto &zoneNode = Node(zoneEquipConfig.ZoneNode);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved to shared function.

if (state.dataHeatBal->doSpaceHeatBalanceSizing || state.dataHeatBal->doSpaceHeatBalanceSimulation) {
if (state.dataHeatBal->doSpaceHeatBalanceSimulation) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some more related cleanup of existing code.

@Myoldmopar
Copy link
Member

@mjwitte nice job getting clean CI! EIO diffs in the two space example files appear to be the only meaningful changes, which I would expect to see.

Thanks for the code walk-through, that's super helpful! I'll take a look now.

@Myoldmopar
Copy link
Member

  • Partially address Extend Spaces to Sizing #8927

    • When ZoneAirHeatBalanceAlgorithm "Do Space Heat Balance for Sizing" = Yes, zone sizing is also done for all spaces. The HVAC Sizing Summary table report will include subtables for Space Sensible Cooling and Heating as well as for Zone Sensible Cooling and Heating. Space Sizing will also be reported to the eio output.
    • At this stage, the space sizing results are reported, but not used.
    • Zone sizing remains unchanged, sizing for all spaces in the zone lumped together which is essentially the same as the coincident space load.
  • Partially addresses Extend Spaces to HVAC #8928

OK, this makes sense, and is a big reason why we are getting clean CI.

  • Adds two new objects SpaceHVAC:EquipmentConnections and SpaceHVAC:ZoneEquipmentSplitter.
  • These objects allow one or more zones to be simulated at the space level for HVAC, leading to independent air temperature and humidity in each space.

Nice.

Copy link
Member

@Myoldmopar Myoldmopar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well. This is big 😆

You've obviously done a ton of work here, and nothing raises a red flag. It's tough to scrutinize any one specific change because so many of the changes had to be spread around. I'm encouraged that CI is happy -- you didn't break anything in the current code. As @rraustad would say, we shouldn't hold this up arbitrarily and we probably need to get this out and available to users and refine it over time.

@@ -453,6 +453,9 @@ namespace DataHeatBalance {
Real64 ExtGrossWallArea = 0.0; // Exterior Wall Area for Zone (Gross)
Real64 ExteriorTotalSurfArea = 0.0; // Total surface area of all exterior surfaces for Zone
int SystemZoneNodeNumber = 0; // This is the zone or space node number for the system for a controlled zone
Real64 FloorArea = 0.0; // Floor area used for this space
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know it's really difficult to predict refactors like this, but it could help to take a quick look through the code you want to touch in your next phase, and quickly do some renaming in a first-pass PR before functional changes. I get that it won't be obvious at first, just saying that would make explaining and digesting the PR changes easier all around.

void zeroMemberData();
void allocateMemberArrays(int numOfTimeStepInDay);
};

// based on ZoneSizingData but only member variables that are actually used by terminal unit sizing
struct TermUnitZoneSizingData
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So does ZoneSizingData inherit this subset class now so that these variables aren't repeated?

int zoneEquipInletNodeNum = 0;
DataZoneEquipment::SpaceEquipSizingBasis spaceSizingBasis = DataZoneEquipment::SpaceEquipSizingBasis::Invalid;
std::vector<ZoneEquipSplitterMixerSpace> spaces;
};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice clean little structs 👍 👍

this->columnTag.deallocate();
this->tableEntry.deallocate();
this->CompSizeTableEntry.deallocate();
this->ShadowRelate.deallocate();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh my gawd. 👍

Copy link
Contributor

@rraustad rraustad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review of IO Ref and IDD.

\subsection{SpaceHVAC:EquipmentConnections}\label{spacehvacequipmentconnections}

If HVAC equipment is modeled at the Space level for one or more zones, the SpaceHVAC:EquipmentConnections statement defines the node connections to a space. Any space modeled with SpaceHVAC:EquipmentConnections will have its own air temperature and humidity ratio, and surfaces and internal gains in the zone will interact with the space conditions.
Note that all nodes mentioned in the SpaceHVAC:EquipmentConnections input must be unique. That is, all nodes in all the SpaceHVAC:EquipmentConnections statements referenced by the ``Space Air Inlet Nodes'', ``Space Air Exhaust Nodes'', ``Space Air Node Name'' and ``Space Return Air Node Name'' cannot have any node name appearing more than once. Unlike ZoneHVAC:EquipmentConnections, there is no equipment list. Equipment is connected to spaces using \hyperref[spacehvaczoneequipmentsplitter]{SpaceHVAC:ZoneEquipmentSplitter}. If any space in a zone has a SpaceHVAC:EquipmentConnections object, then all spaces in that zone must have one, even if they are not served by any HVAC equipment. SpaceHVAC is only used when \hyperref[zoneairheatbalancealgorithm]{ZoneAirHeatBalanceAlgorithm} ``Do Space Heat Balance for Simulation'' is Yes.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I checked that the minimum fraction was indeed 0. But it can also be autosized. I guess if the user selects autosize then this space IS actually served by the HVAC equipment. There is no way to know if this space is served by the HVAC system other than a hard 0 as the fraction. Not requesting changes, just stating an observation.

N1, \field Space 1 Fraction
   \note Fraction of this zone equipment output or airflow distributed to this space.
   \default autosize
   \units dimensionless
   \minimum 0.0


\paragraph{Field: Space Return Air Node 1 Flow Rate Fraction Schedule Name}\label{field-space-return-air-flow-rate-fraction-schedule-name}

The name of a schedule to specify the return air flow rate for the first return air node as a fraction of the base return air. If the next field is blank, then the return air flow rate is the total supply inlet flow rate to the space less the total exhaust node flow rate from the space multiplied by this schedule name. If this field is left blank, the schedule defaults to 1.0 at all times.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So if this schedule fraction is 0.5, assuming no exhaust, then the return flow is 1/2 of the space total supply inlet flow (the sum of all the space inlets). Where does the rest of the air go? I have to applaud the space concept effort here but boy is it hard to grasp all the connections/interactions.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the same as the existing Zone equip connection object. If there's a second airloop serving the zone or space excess air will go there otherwise it's exfiltration


\paragraph{Field: Space Return Air Node 1 Flow Rate Basis Node or NodeList Name}\label{field-space-return-air-flow-rate-basis-node-or-nodelist-name}

The name of a node or list of nodes (\hyperref[nodelist]{NodeList}) that is used to calculate the return air flow rate for the first return air node in this space. The sum of the current flow rates for this node(s) multiplied by the Space Return Air Node 1 Flow Rate Fraction Schedule determines the return air flow rate. If this field is blank, then the base return air flow rate is the total supply inlet flow rate to the space less the total exhaust node flow rate from the space in the case of a single air loop serving this space. If there are multiple air loops serving this space, the base return air flow rate is governed by the corresponding supply inlet flow rate and the AirloopHVAC Design Return Air Flow Fraction of Supply Air Flow.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume this node name is the name of a space inlet node? When would this node not be a space inlet node?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably but not limited to that.

SPACE3-1 Out Node, !- Space Return Air Node or NodeList Name
, !- Space Return Air Node 1 Flow Rate Fraction Schedule Name
; !- Space Return Air Node 1 Flow Rate Basis Node or NodeList Name
\end{lstlisting}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There can be multiple space inlets, exhausts, and returns. But there is only 1 fraction schedule. It seems like spaces are currently limited to a single return node? or Return Air Node 1 is the only node that can have a fraction applied? I feel the urge to see "extensible" here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe someday

Zone 5-Remainder, !- Space 3 Name
0.2, !- Space 3 Fraction {dimensionless}
Zone 5-Remainder In Node; !- Space 3 Node Name
\end{lstlisting}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any connection between Space 1 Fraction in the mixer and the splitter? I think you told me that the splitter and mixer would NOT be used in unison (therefore the mixer and splitter fractions have no relationship) and are used based on the equipment type (e.g., PTAC vs ZoneExhaust).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No connection. For example, a fan coil could draw from one space and supply 2 or more spaces (once the mass balance code is implemented).

\key Ideal
\default SingleSpace
A7, \field Control Space Name
\note Used with SingleSpace thermostat control method
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

\note This field is only used when Thermostat Control Method = SingleSpace.

\begin-extensible
\required-field
\type object-list
\object-list SpaceNames
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this object need a min-fields? I'm trying to figure out where I can stop providing inputs for this object.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, minfilelds up to last field for space 1

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, min-fields is already there and includes up to N1 since A10 is not required for non-airflow equipment.

A11, \field Space 2 Name
\required-field
\type object-list
\object-list SpaceNames
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And I see that Space 2 Name is a required-field, as is Space 3 Name. I think that's a mistake. There can technically be only 1 space in a zone so requiring any fields after Space 1 might be troublesome?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Required is a mistake. If you want to make these idd changes please go ahead.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will make the few changes to the idd. (notes, required-field, min-fields)

\required-field
\type node
A7, \field Space 2 Name
\required-field
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment on required-field here after Space 1.

@@ -212,14 +212,18 @@ \subsubsection{Inputs}\label{inputs-1-052}
\begin{lstlisting}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The figure above should use actual object names so that users better understand the connections.

@rraustad
Copy link
Contributor

The only diffs here are with the 2 5ZoneAirCooledWithSpace* example files. I think this effort is ready to merge. I'm happy to do that later today just to not create a moving develop while CI is running.

Copy link
Contributor

@rraustad rraustad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed code, commented and discussed, reviewed diffs.

@rraustad
Copy link
Contributor

I just pulled develop and unit tests run successfully.

@rraustad
Copy link
Contributor

rraustad commented Aug 23, 2023

5ZoneAirCooledWithSpacesHVAC.idf:

It appears that all the numbers are matching in these tables. Probably need to eventually make these more similar (e.g., include space volume, etc,). Thinking about the windows, does sunlight hit the space next to the window, or is distributed to all spaces?

image

And space type summary seems to make sense given the input in the Space object. The total row could eventually be filled in.

Space,
  Space 5 Office,          !- Name
  Zone 5,                  !- Zone Name
  autocalculate,           !- Ceiling Height {m}
  autocalculate,           !- Volume {m3}
  ,                        !- Floor Area {m2}
  Office,                  !- Space Type
  Std 62.1 Office Space,   !- Tag 1
  Std 90.1 Office-Open Plan;  !- Tag 2

image

@rraustad
Copy link
Contributor

rraustad commented Aug 23, 2023

And Space 3* is peaking at different times, that's a good sign.

image

Comparing to zone cooling loads:

image

And Space 3 loads double when a zone multiplier of 2 is used:

image

@rraustad
Copy link
Contributor

rraustad commented Aug 23, 2023

@Myoldmopar this is ready to merge. Let me know if you have other tests you want to run, otherwise I can merge this. My review is now complete.

@Myoldmopar
Copy link
Member

I don't need anything else, merge away!

@rraustad
Copy link
Contributor

Nice expansion of what spaces can do. Also, a 1.7% speed improvement.

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
IDDChange Code changes impact the IDD file (cannot be merged after IO freeze) NewFeature Includes code to add a new feature to EnergyPlus
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants