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

Add Space Concept to EnergyPlus Zone Structure, Part 1 #8394

Merged
merged 121 commits into from
Aug 21, 2021
Merged

Conversation

mjwitte
Copy link
Contributor

@mjwitte mjwitte commented Nov 25, 2020

Pull request overview

  • Add Space which is a <= Zone. See NFP for more details.
  • New example file 5ZoneAirCooledWithSpaces.idf

Space:

  • Defines a space (room) in the building.
  • Space is an optional input.
  • All Spaces are part of a Zone.
  • Every Zone contains one or more spaces.
  • If a Zone has no Space(s) specified in input then a default Space named <Zone Name> will be created.
  • If some surfaces in a Zone are assigned to a space and some are not, then a default Space named <Zone Name>-Remainder will be created.
  • Input references to Space Names must have a matching Space object (default space names may not be referenced except in output variable keys).

Key Input Changes (see Rules9-5-0-to-9-6-0 for more details:

  • New optional Space object with Space Type tag and other tags.

  • Surfaces always reference a Zone Name (no change).

  • Surfaces may reference a Space Name (optional)

  • Two space geometry options:

    • Floor surface(s): Area based on floor surface(s) attached to the Space
    • Full geometry: Define the Space with surfaces on all sides (air boundaries are supported).
  • New SpaceList object (equivalent to ZoneList, but for Spaces).

  • Internal gains reference Zone or ZoneList or Space or Spacelist.

    • Gains specified at the Zone level will be applied proportionally by floor area to all Spaces in the Zone
    • Gains specified at the Space level will be applied to that Space only (in addition to any gains specified at the Zone level)
  • Daylighting controls and Reference Points reference a Zone or Space. (Daylighting reference points see all windows in an enclosure). (in the next PR)

  • ZoneProperty:UserViewFactors:BySurfaceName references Space or SpaceList Name instead of Zone or ZoneList Name.

  • New DesignSpecification:OutdoorAir:List object to combine OA requirements by Space.

  • Sizing:Zone references a DesignSpecification:OutdoorAir or DesignSpecification:OutdoorAir:List object.

  • ZoneInfiltration:* and ZoneVentilation:* remain as-is referencing Zone or ZoneList (no change).

  • Thermostats remain as-is referencing Zone or ZoneList.

  • ZoneHVAC remains as-is and continues to reference a single Zone.

Key Output Changes:

  • New table outputs by Space and Space Type (see OutputChanges9-5-0-to-9-6-0 for more details.
  • New columns in Lighting Summary
  • New Space output variables for internal gains (same as the existing Zone-level outputs, see rdd output).
  • New meters for Space and SpaceType (see mdd and mtd output).
  • Fixes meters by end-use sub-category.

Diffs

ToDo List - see #9002 for further progress

  • Daylighting really needs to move to the Space level, or some other clever trick, because enclosures are 1 or more Spaces, and Zones won't necessarily align with that.
    • Fun fact Daylighting:Controls are stored in ZoneDaylight(ZoneNum) and current GetDaylightingControls happily lets you have more than one Daylighting:Controls assigned to the same zone with no complaint. Problem is, only the last one does anything, the others are lost, or possibly you get some kind of merged input if the later one doesn't use the same fields as the first one. Soooo I'm thinking with Space in the mix,
    • Related, in DataHeatBalance.hh: int ZoneFirstSpaceSolEnclosure; // TODO: For daylighting, this is a punt, it's the solar enclosure number of the first space in the zone
    • DaylightingManager.cc:4395 // TODO: This enclosure/zone mapping is a mess, punting for now// ToDo: For now, use min and max enclosure numbers associated with this zone, this could include unrelated enclosures`
    • SurfaceGeometry.cc:14914: // TODO: For daylighting, set the zone solar enclosure number to the first space's number
    • DaylightingManager.cc:4243: // TODO MJW: Reference points will be double-counted for zone with more than one space
  • ZoneToResimulate doesn't play well with Space. HeatBalanceIntRadExchange.cc:193: `
    • SurfaceGeomertry.cc:14903: // ToDo: For now, set the max and min enclosure numbers for each zone to be used in CalcInteriorRadExchange with ZoneToResimulate
    • When a Space is defined by just it's floor surface - now what happens? It makes a space that's in the same enclosure as other similar spaces in the same zone.
  • Table outputs
  • Propagate People changes in InternalHeatGains.cc to all other internal gains - figure out a way to do some code re-use and break up the long functions.
  • Remove requirement that every space have at least one surface? See CreateMissingSpaces:2856
  • Find the sweet spot for allocating internal gains or just keep if !.allocated blocks? See HeatBalancemanager::AllocateZoneHeatBalArrays and InternalHeatGains::GetInternalheatGainsInput
  • Confirm that Space.Surfaces only includes HT surfs, ref. HeatBalanceIntRadExchange.cc:511 & 818
    Yes, see SurfaceGeometry::CreateMissingSpaces
  • Documentation, of course
  • Make sure there's no performance degradation (about 0.7% slowdown as of 7/25)
  • For ZoneProperty:UserViewFactors:BySurfaceName - accept ZoneList name if no corresponding SpaceList exists in input - with warning?
  • Replace direct calls to DataZoneEquipment::CalcDesignSpecificationOutdoorAir in the air terminal units etc with DSOA method so they can reference a list object or a simple one. And the base CalcDesignSpecificationOutdoorAir should probably move into there.

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 Nov 25, 2020

ZoneVentilation and ZoneInfiltration
Name,
Zone or ZoneList Name, (or should these more to the Space level and be renamed?)
Copy link
Contributor

Choose a reason for hiding this comment

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

I haven't thought about this before. I have in the past included a part of a hallway with the adjacent office space as a single zone, just to save time on data entry. The hallway in a commercial building would have a different infiltration rate than other spaces since it would be connected to external doors, and would typically not have a thermostat. Of course you could add a thermostat in a simulation but that would defeat the purpose of "zoning" spaces around a common thermostat. So if this is left as a zone object the user would need to correctly group spaces of like ventilation/infiltration to a single zone instead of grouping spaces near a thermostat to a single zone. So it seems to me ventilation/infiltration should be applied at the space level.

@rraustad
Copy link
Contributor

NFP describes a good overall plan.

Surfaces
Internal Gains (People, Lights, etc.)

Enclosure
Copy link

@macumber macumber Nov 25, 2020

Choose a reason for hiding this comment

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

This doesn't look like a correct tree hierarchy to me. OpenStudio's geometry is in a tree hierarchy (due to the physical reality that Spaces are physical objects that can't overlap). ThermalZones are off to the side, they are not part of the tree, they reference Spaces by ID rather than literally nesting the Spaces inside them.

  • Spaces
    • Space
      • ID
      • Surfaces
      • Internal Gains
  • ThermalZones
    • ThermalZone
      • Space IDs
  • Enclosures
    • Enclosure
      • Space IDs

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think the proposed hierarchy is conceptual and conceptually matches OS.

Copy link

@macumber macumber Nov 25, 2020

Choose a reason for hiding this comment

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

Very minimal changes to current inputs.

* Old Zone object becomes Space object plus some tags.
* New Zone object has a name and a list of Spaces.
Copy link

@macumber macumber Nov 25, 2020

Choose a reason for hiding this comment

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

Suggest the name ThermalZone which would align with OpenStudio and allow for other types of Zones.

I'd suggest naming Enclosure something like RadiantTransferZone or something that better captures its simulation purpose. To me, the word Enclosure is more about being waterproof and keeping your stuff out of the elements.

OpenStudio envisioned Zones for calculating and controlling lighting like DaylightingZone, ElectricLightingZone. There could also be AirflowZone or others in the future. BuildingStory and Unit (e.g. apartment or commercial unit) are other useful groupings of Spaces.

Copy link

@macumber macumber Nov 25, 2020

Choose a reason for hiding this comment

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

It seems like these different *Zones could be used to partition the simulation into separate calculations and might even be useful in parallelizing the simulation in the future?

* ZoneHVAC and Thermostats remain as-is and continue to reference a Zone or ZoneList.
* New SpaceList object (equivalent to ZoneList, but for Spaces).
* Internal gains reference Space or Spacelist (instead of Zone or ZoneList).
* ZoneInfiltration and ZoneVentilation remain at the zone level.

Choose a reason for hiding this comment

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

Is it possible to make infiltration or ventilation at the space level? That would simplify input for cases where ventilation requirements are a function of space type.

Copy link
Member

Choose a reason for hiding this comment

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

I think it is possible, I think we probably just want to avoid having both (e.g. SpaceInfiltration and ZoneInfiltration). Moving everything to the space level will probably be more work, but if that's the right solution then we should do that.

Copy link
Contributor

Choose a reason for hiding this comment

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

Whether it's SpaceInfiltration or ZoneInfiltration, it'd be nice to be able to get individual outputs for each object. With current EnergyPlus, you can have multiple ZoneInfiltration objects in a zone, but can only get aggregate outputs (i.e., the key for Zone Infiltration Sensible Heat Gain Energy is a zone object, not an infiltration object).

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 have outputs for both space and zone to show individual as well as aggregated.

Copy link
Member

Choose a reason for hiding this comment

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

I agree with this: limit to one input approach but output at both the space and zone level.

Enclosures are used for radiant and solar/daylighting exchange. Enclosures are primarily concerned with Surfaces, but they
are also related to internal gains (which cast radiant and visible energy into the enclosure). By default there is one Enclosure for each
Zone. By using Construction:AirBoundary, multiple zones may be grouped into a single larger Enclosure.
Enclosures are assigned automatically based on the zones connected by an air boundary surface.

Choose a reason for hiding this comment

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

If Spaces are fully enclosed (air boundaries count as being fully enclosed because the volume is defined) and Spaces cannot overlap, then ThermalZones and Enclosures composed of Spaces will be fully enclosed (but possibly disjoint) and non-overlapping.

Copy link
Member

@jasondegraw jasondegraw Nov 25, 2020

Choose a reason for hiding this comment

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

Spaces are fully enclosed as you describe (with the possibility of air boundaries), so yes to the rest.

@amirroth
Copy link
Collaborator

Are we going to continue to support the current Zone-based input scheme for a while (and internally create a Space for each Zone) or just rip the bandaid off? If we are ripping bandaids, are we notifying vendors and users that this is coming and asking for their feedback?

Copy link
Collaborator

@amirroth amirroth left a comment

Choose a reason for hiding this comment

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

At a high-level, we should be able to bury some of these details and assignments within EnergyPlus itself. For instance, we could attach only a floor surface to each space. Then EnergyPlus could use surface adjacency checks to find all the other surfaces and attach them to the space and create the necessary air-boundary surfaces. Similarly, the user would only need to explicitly group Spaces into Zones. EnergyPlus would use surface adjacency checks to figure out what the Enclosures should be. Does it make sense to have that stuff in place before we push this out to reduce change burden on users and application vendors?


Space, !- Same fields as old Zone object plus new tags at the end
Name,
Origin,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we really need to continue using origin? Why?

Volume,
Floor Area,
Convection algorithms,
Part of Total Floor Area,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Part of total floor area of what? Zone? Enclosure?

Multiplier,
Ceiling Height,
Volume,
Floor Area,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Shouldn't floor area be calculated from the floor surface attached to this space? Shouldn't ceiling height and air volume be calculated from the other surfaces attached to this space?

@shorowit
Copy link
Contributor

Then EnergyPlus could use surface adjacency checks to find all the other surfaces and attach them to the space and create the necessary air-boundary surfaces. Similarly, the user would only need to explicitly group Spaces into Zones. EnergyPlus would use surface adjacency checks to figure out what the Enclosures should be.

I hope that EnergyPlus will not start enforcing self-enclosed 3D building geometry. For residential buildings, we have many use cases where the inputs collected (wall areas, floor area, ceiling/roof areas) cannot possibly be reconstructed back into a 3D building geometry, so we create EnergyPlus models that don't even try to achieve this. I recall that CBECC-Com does a similar thing.

@jasondegraw
Copy link
Member

@shorowit It's not the intent here to start enforcing a fully enclosed condition and I agree that EnergyPlus shouldn't require that. My expectation is that the previous treatment of non-enclosed zones would be carried over into spaces (with maybe some minor changes, need to think about that to be sure).

@mjwitte
Copy link
Contributor Author

mjwitte commented Dec 1, 2020

Thanks(?) for the engaging comments everyone. I'll try to summarize and clarify here and then update the NFP:

  1. ZoneInfiltration and ZoneVentilation - there's good reason for space-level objects for these. May still decide to support both space and zone level inputs. Will also attempt to add individual outputs for these objects.
  2. Alternate names for Zone were considered and discarded in past discussions. From a heat balance perspective, the modeling of a Zone will not change under this proprosal. The Spaces are simply input building blocks for the Zone. ThermalZone can be reconsidered as a way to clarify what the zoning is being used for.
  3. Spaces and Zones and Enclosures, oh my!
    a. All of these are expected to be fully defined by surfaces and/or air boundaries (but it's not required, the models work with a missing side, for example).
    b. Typically Spaces will not overlap geometrically (but there's nothing preventing a model with simplified geometry where they do, just don't try to use FullInterior solar).
    c. ThermalZones are a collection of Spaces, lumping together the surfaces and air volume and internal gains etc. Geometry doesn't really matter for these calculations (just need areas and orientations for convection).
    d. Enclosures are a collection of Spaces where geometry does affect view factors and solar distribution. Again, if using FullInterior solar distribution, the Enclosure should be fully enclosed by real surfaces (not air boundaries). Air boundaries within the Enclosure are what connect multiple Spaces into a single Enclosure.
  4. The current proposal is to rip off the bandaid and require both Space and Zone objects.
  5. Once the team is close to consensus on the proposal, feedback will be sought from other stakeholders (vendors and users).
  6. Regarding explicit air boundary surfaces:
    a. They're already in use (at least by some) to connect Zones.
    b. They fit with the EnergyPlus "philosophy" that everything (or nearly so) in the input model is explicitly described. The absence of a surface does not imply an opening to another zone or to the outdoors.
    c. If automated surface adjacency is added in the future, it's not a big lift to omit the air boundary surfaces from the input for models with complete geometry.
  7. There are some questions about the existing inputs in the Zone object which will be retained in the Space object:
    a. Origin - optionally used with RelativeCoordinates, all zeros if using WorldCoordinates.
    b. Part of Total Floor Area - is it part of the building area for reporting things like energy use per square foot
    c. Volume, ceiling height, and floor area are all automatically calculated, but the user can input values to override.

@nealkruis
Copy link
Member

Here’s Big Ladder’s perspective:

  1. EnergyPlus should continue to support the current Zone-based input scheme. Backwards compatibility is going to be important for users, middleware developers, and interface developers. As proposed, the introduction of Spaces would be a highly impactful change because it affects nearly every EnergyPlus model, and it would be a non-trivial change; it's not just inserting or deleting a field.

    The repurposing of the "Zone" name for the new top-level object, and the renaming of the existing Zone object to Space will also cause significant confusion for users. Existing users will have to relearn these concepts and there won't be a common language for discussing the change when Zone no longer means Zone. (Think of how confusing it was when the OpenStudio name was recycled.)

    We feel there is no real reason to break compatibility and force anyone away from using the current Zone-based input scheme. Under the hood, a simple 1-to-1 Zone/Space mapping could be generated automatically to preserve compatibility.

  2. We are concerned that this is a complex solution to two mostly separate issues: 1) calculating area-based internal gains from separate Spaces into a single Zone, and 2) separating the air-node and radiant enclosure concepts. It’s also not clear there will be any noticeable performance or accuracy gains from this change. Consider the floor plan below:

    spaces_zones

    With the proposed addition of Spaces, this could be modeled in a number of ways:

    1. Each room could be modeled as a separate Zone with an interzone surface between them. Each Zone would have 6 opaque surfaces and 1 window. There would be two 7x7 IR exchange matrices, 14 conduction heat balances, and 2 Zone heat balances.

    2. Each room could be modeled as part of a single Zone with pre-processed internal gain calculations by floor area (as many interfaces do already). The interior surface could be modeled as thermal mass, and the windows could be combined into a single window. This represents some of the "best practices" we recommend to help manage simulation time. The Zone would have 7 opaque surfaces (including the internal mass) and one window. There would be one 8x8 IR exchange matrices, 7 conduction heat balances, and 2 Zone heat balances.

    3. Each room could be modeled as a separate Space within a single Zone (would the interior surface be modeled as matched surfaces in the same Zone?). Each Space would have 6 opaque surfaces and 1 window. There would be two 7x7 IR exchange matrices, 14 conduction heat balances, and 1 Zone heat balance.

    It's not clear that two 7x7 matrices are faster to calculate than a single 8x8 matrix (especially if they are calculated serially as they are currently in the code). When you add the additional heat balances, it becomes less clear, especially if any of the floors are in contact with the ground (e.g., Kiva instances).

    The side effect of this feature is that it will encourage modelers to create more surfaces--likely more than are really necessary for an accurate thermal model. Many of the surfaces in the third approach described above are split between Spaces for the sake of internal gain calculations, but are likely to have very similar heat transfer characteristics (same constructions and boundary conditions). The results will likely be slightly more accurate, but only because of the resolution of shading and infrared calculations (which are likely small impacts).

    More surfaces could also exacerbate one of EnergyPlus's biggest pain points: surface matching during geometry input.

    This feature is attempting to hit two birds with one stone, when maybe a simpler solution is to tackle them separately. For example, could “Spaces” simply be floor outlines drawn on a layer on top of the thermal geometry to calculate area-based internal gains without necessitating the division of thermal surfaces? We already have a way of connecting multiple Zones in the same radiant enclosure, and it’s not clear that having multiple radiant enclosures within a Zone buys much (especially since they are all part of the same “well-mixed” air-node anyway).

  3. Whatever direction this takes should consider implications for Zone mean radiant temperatures. Will comfort be calculated at a Space level using the mean radiant temperature of the Space and the air temperature of the Zone? This could also impact the Carroll Radiant Exchange Algorithm, but it is probably manageable.

@amirroth
Copy link
Collaborator

1a. I am also on the side of backwards compatibility and retaining the current Zone-only scheme as an option. The fact that we have epJSON actually gives us the opportunity to try to have it "both ways", i.e., retain backwards compatibility in IDF but remove it in epJSON. Although that in itself would be a complication.

1b. I am also on the side of retaining the current definition of Zone. I'm not sure that the scope expansion of the name OpenStudio from "graphical application" to SDK and also graphical application is the right analogy, but Zone feels less than a "brand" and more like an established concept.

  1. I disagree with some of the discussion around the motivation for this feature and the subsequent suggestions for simpler, alternative implementations. To me there are four separate motivations: i) embed the concept of Space and SpaceType within EnergyPlus to facilitate applications that require this concept including supporting reporting at this level, ii) enable a flexible relationship between (air) Zone and (surface) Enclosure concepts to increase the accuracy of modeling certain configurations or at least make it more intuitive and natural to model them, iii) make EnergyPlus "friendlier" to BIM architectural models by allowing those models to be simulated in a form that more closely resembles their original design, reducing the burden on that level of translation, which at this point I would say needs all the help it can get.

Creating a Space/SpaceType "overlay" addresses the first issue (kind of), but ignores the other two. Now, maybe the other two are not worth paying attention to, but I think that a bit more discussion is needed there. First, on the subject of flexible relationship between Zone and Enclosure and more accurate modeling of some configurations. I grant that the current proposal for a single "perfectly-mixed" air-node per zone makes this much less compelling, but what if there were multiple air-nodes that were less than perfectly mixed but yet controlled by a single thermostat? That starts to feel a bit more realistic, no? (I think this also touches Neal's third point). Does this sound wrong? I'm not being rhetorical here. I want to know if this doesn't pass the smell test (sorry about mixing my sensory metaphors here, I'm having literary synesthesia).

As for the third issue, the counter-argument about "best practices for reduced simulation time" feels grounded in the experience of traditional (but obviously still relevant) from-the-ground-up or self-contained modeling workflows. However, there is an argument to be made that facilitating the use of EnergyPlus in design workflows is just as important. I don't look at this as "encouraging modelers to create more surfaces than necessary". I look at it as "allowing users to take models that have more surfaces than necessary and simulate them in a reasonable way without requiring manual surface simplification." How about this? Does this sound wrong? Same thing here.

I think this brings us back nicely to the original point.

1a. I think continuing to support the current Zone-only only input model is important. Not only for software compatibility reasons, but because it is possibly better aligned with traditional self-contained from-the-ground-up modeling workflows. Sure, there is no fundamental reason why a user could not create a space for every zone, but there is also no fundamental reason why EnergyPlus cannot do such a simple thing itself.

@mjwitte
Copy link
Contributor Author

mjwitte commented Dec 11, 2020

Some responses. I will revise the NFP to clarify some of these points.

  1. Zone is not being repurposed. A Zone is still the same as it always has been. This proposal inserts Space as a new layer in assembling surfaces and gains into a zone. HVAC connections and controls remain at the Zone level.
  2. If Spaces and Zones are allowed to share the same names, then the transition rules could be reduced to:
    a. Write current Zone objects as Space
    b. Write one new 2-line Zone object for each Zone/Space.
  3. Keeping zone-only inputs would require allowing both zone-level and space-level specification of loads and other simulation parameters, when a space-level specification would be more in line with the other feedback.
  4. Air-nodes and radiant enclosures are already separated (ref. Construction:AirBoundary). This only adds flexibility.
  5. The statement about performance should say "potential". For a two-room example, there is little advantage. But for larger layouts, with many surfaces in a single HVAC zone, the potential savings in radiant exchange and solar calculations is significant. The added cost of managing additional interior surfaces is small.
  6. An additional benefit of smaller enclosures is for solar gain distribution to the proper surfaces.
  7. Surface matching and concepts like overlaying abstract space areas within a zone are interface issues. The simulation engine does not require that matching surfaces are in exactly the same place, and some current workflows rely on this lack of constraint.
  8. Doing the heat balance (air-node) at the Space level was discussed previously and ruled out. (The historical NFP document has been added to this PR for reference.) Adding this level of detail requires more complexity in the HVAC inputs, something which could be added in a later phase.
  9. Nothing in this proposal requires any change in modeling approach other than adding a 2-line Zone object for every Space and using a different object class name for a few objects. Everything else is optional.
  10. That said, concrete suggestions for alternative input methods are welcome.

@mjwitte mjwitte added this to the EnergyPlus 9.5.0 IOFreeze milestone Dec 14, 2020
@Myoldmopar
Copy link
Member

My instinct is always to support minimal changes to the input structure. If you look at things from the input structure perspective, then using Space, with fields from the current Zone object, looks like a radical deviation. And you also have to add a Zone object, re-purposing that name. That seems disruptive. However, if you look at the physics of what is going on, with this new approach, the meaning of Zone actually stays the same. Adding Space in there as a building block is adding flexibility for specifying the contents of each zone, which is key to all this.

I do wonder if there would be so much debate if the name "Zone" weren't being reused. For experienced modelers and users with strong backgrounds in building science that can easily grasp the difference between a space and zone, this change will probably just enable them to do better things. I think the counter argument is that for new users, or users who might not be as strong on the fundamental building science, seeing Zone be (what looks like) re-purposed could be really confusing. If that's really the issue here, maybe we can make another push to reconsider using the name Zone. If we had a different name, then it might ease the digestion of this for new users, and experienced users will easily understand it either way.

As for backward-compatibility, we're never really backward compatible, right? I think that arguments debating significant input changes are valid, but claiming this one thing will somehow break backward compatibility seems a step too far. We will provide transition tooling for users like we always do. And we'll provide transition rules for interfaces. In this case, if a user doesn't want to make any fundamental changes to how they use EnergyPlus, they'll just see the Zone objects trimmed down to a single field (the new Space name) and then all the fields from each Zone moved to the new Space object. Seems quite minimal. I don't want to minimize the effort of changing training materials and examples, but this seems less disruptive than some of the other changes we've made in the past (Connection Component:PlantLoop), and also less than the impending IDF-Deprecation that will come sometime as we advance to purely epJSON.

I definitely want to hear feedback from the key interface developers though.

@amirroth
Copy link
Collaborator

I think @mjwitte's item 9 is what cinches it. The required modeling change amounts to two lines per Zone. Everything else is optional. People are still going to complain but I think is a "10/90" or even "1/99" design choice.

@chriswmackey
Copy link

chriswmackey commented Dec 19, 2020

Thank you for opening this up for larger input, @mjwitte .

The approach seems good and, if I my understanding is correct, the main purpose for adding the space object is to provide a hierarchal separation between internal gains [space] and the HVAC parameters (setpoints, minimum ventilation, infiltration) [zone]. In this case, it would be good for the docs to clarify in which of these two categories the WaterUse:Equiment objects fit. It's a bit unclear since hot water is a lot like an internal gain in its description but the way that it connects to larger systems is a lot like an HVAC parameter. I feel like it should be at the Zone level but I don't have a particularly strong opinion.

Another question is what happens to all of the EnergyPlus simulation outputs that are reported on the Zone level? Will they all remain exactly as they are or will some of them move to the Space level? I would support leaving the Zone level outputs as they are and, if people want to add Space-level outputs, these should be in addition to the Zone ones (not a replacement for them).

Also for the record, I am REALLY in favor of allowing the same name/ID for the zone and a single space beneath it. It's not just going to be easier for backwards compatibility. It's going to make it much easier to match things based on name/ID when these can be the same.

@amirroth
Copy link
Collaborator

amirroth commented Jan 6, 2021

I thought I would throw it in here. It occurs to me that as we are changing the object-level relationship between Surfaces and Zones, it may be possible to do so in a way that allows us to better handle imported geometry models that do not have Space or Zone information. Maybe this is an opportunity to address the age-old "why can't I import a SketchUp model and add Zones to it?" issue.

@nrel-bot-3
Copy link

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

@mjwitte mjwitte removed this from the EnergyPlus 9.5.0 IOFreeze milestone Feb 15, 2021
@rraustad
Copy link
Contributor

@Myoldmopar pending any additional effort from @mjwitte this looks ready to go.

@mjwitte
Copy link
Contributor Author

mjwitte commented Aug 20, 2021

@Myoldmopar I'm done for now, pending any review comments that need to be addressed. A follow-up PR will finish the do-list above before release and possibly add more tabular report subtables (that's ok after I/O freeze, right? New subtables.)

@rraustad
Copy link
Contributor

There is agreement now between zone and space reports. With 1 space in the zone these should match.

image

@rraustad
Copy link
Contributor

And in Zone 5, with 2 spaces, the sum of the space reports agree with the zone report.

image

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 walkthrough (just the highlights that aren't self-explanatory). Part 1 of 2, 'cause this is getting too big.

@@ -116,6 +116,13 @@ namespace DataDaylighting {
}
};

struct EnclDaylightCalc
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Begin moving enclosure-level variables to their own struct to avoid confusion.

Comment on lines +596 to +601
int spaceOrSpaceListPtr = 0;
int numOfSpaces = 0;
int spaceStartPtr = 0;
bool spaceListActive = false;
EPVector<int> spaceNums; // Indexes to spaces associated with this input object
EPVector<std::string> names; // Names for each instance created from this input 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.

GlobalInternalGainMiscObject is currently used for mapping an internal gains input to multiple instances of that gain when using a ZoneList. This extends that to SpaceList and now Zone name could represent multiple spaces. So, ultimately, the input object is read, the "Zone or ZoneList or Space of SpaceList" name is interpreted and a list of all applicable spaces is stored in spaceNums along with a list of the expanded names for each instance of the internal gain that will be created.

{
}
};

struct ZoneSimData // Calculated data by Zone during each time step/hour
struct SpaceZoneSimData // Calculated data by Space or Zone during each time step/hour
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Track internal gains by Space now instead of Zone, here is for heat balance and feeding into output variables.

{
}
};

struct SpaceIntGainDeviceData
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The general internal gain structure is now Space-based.

@@ -2258,13 +2316,17 @@ struct HeatBalanceData : BaseGlobalStruct
EPVector<DataHeatBalance::GlobalInternalGainMiscObject> HotWaterEqObjects;
EPVector<DataHeatBalance::GlobalInternalGainMiscObject> SteamEqObjects;
EPVector<DataHeatBalance::GlobalInternalGainMiscObject> OtherEqObjects;
EPVector<DataHeatBalance::GlobalInternalGainMiscObject> ITEqObjects;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

IT equipment and zone baseboard inputs are now processed the same way that other internal gains are.

// CurrentModuleObject='Zone'
for (Loop = 1; Loop <= state.dataGlobal->NumOfZones; ++Loop) {
// Overall Zone Variables
SetupOutputVariable(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.

All of the SetupOutputVariable calls for the internal gains that changed are moved to a new functionsetupIHGOutputs.

Comment on lines 282 to 287
setupIHGZonesAndSpaces(state,
peopleModuleObject,
state.dataHeatBal->PeopleObjects,
state.dataHeatBal->NumPeopleStatements,
state.dataHeatBal->TotPeople,
ErrorsFound);
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 function to interpret the "Zone or ZoneList or Space or SpaceList Name" field and expand the input object as needed to all applicable Spaces.


state.dataHeatBal->People.allocate(state.dataHeatBal->TotPeople);
int peopleNum = 0;
for (int peopleInputNum = 1; peopleInputNum <= state.dataHeatBal->NumPeopleStatements; ++peopleInputNum) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Loop over each input object.

state.dataHeatBal->People(Loop).NumberOfPeoplePtr = GetScheduleIndex(state, AlphaName(3));
// Create one People instance for every space associated with this People input object
auto &thisPeopleInput = state.dataHeatBal->PeopleObjects(peopleInputNum);
for (int Item1 = 1; Item1 <= thisPeopleInput.numOfSpaces; ++Item1) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Loop over all spaces for a given input object (as determined by setupIHGZonesAndSpaces.

Comment on lines 1056 to 1057
setupIHGZonesAndSpaces(state,
lightsModuleObject,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Repeat the pattern for the next gain type, and so one.

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.

Walkthrough part 2 (not sure why some of the part 1 comments are marked outdated - sorry about that).

}
}

void setupIHGZonesAndSpaces(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.

Here's the new function to expand internal gains inputs to 1 or more space instances.

}
}

void setupIHGOutputs(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.

And here's where all of the output variables get set up for internal gains including new Space-level outputs.

@@ -6246,6 +7098,16 @@ namespace InternalHeatGains {
e.CO2Rate = 0.0;
}

for (auto &e : state.dataHeatBal->spaceRpt) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Lots of new loops by space now.

} else {
ValidateNStandardizeMeterTitles(state, MtrUnits, ResourceType, EndUse, EndUseSub, Group, ErrorsFound);
}
ValidateNStandardizeMeterTitles(state, MtrUnits, ResourceType, EndUse, EndUseSub, Group, ErrorsFound, ZoneName, SpaceType);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Various changes to add SpaceType meters.

@@ -2292,6 +2336,33 @@ namespace OutputProcessor {
MeterName = EndUseSub + ':' + EndUse + ':' + ResourceType;
Found = UtilityRoutines::FindItem(MeterName, op->EnergyMeters);
if (Found == 0) AddMeter(state, MeterName, MtrUnits, ResourceType, EndUse, EndUseSub, "");

if (Group == "Building") { // Match to Zone and 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.

Previously, EndUseBus:EndUse:ResourceType:Zone:ZoneName meters were not getting added. This fixes that for Zone and adds for SpaceType.

state.dataSize->ZoneSizingInput(ZoneSizIndex).OADesMethod = state.dataSize->OARequirements(OAIndex).OAFlowMethod;
state.dataSize->ZoneSizingInput(ZoneSizIndex).DesOAFlowPPer = state.dataSize->OARequirements(OAIndex).OAFlowPerPerson;
state.dataSize->ZoneSizingInput(ZoneSizIndex).DesOAFlowPerArea = state.dataSize->OARequirements(OAIndex).OAFlowPerArea;
state.dataSize->ZoneSizingInput(ZoneSizIndex).DesOAFlow = state.dataSize->OARequirements(OAIndex).OAFlowPerZone;
state.dataSize->ZoneSizingInput(ZoneSizIndex).ZoneDesignSpecOAIndex = OAIndex;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Previously, these values were all copied from the DSOA struct into the sizing data structures. This is pointless now. The DSOA inputs (single or list) are accessed later.

state.dataHeatBal->ZnOpqSurfInsFaceCondGnRepEnrg(zoneNum) = 0.0;
state.dataHeatBal->ZnOpqSurfInsFaceCondLsRepEnrg(zoneNum) = 0.0;
}
for (int enclNum = 1; enclNum <= state.dataViewFactor->NumOfSolarEnclosures; ++enclNum) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

More zone vs enclosure indexing.

++state.dataHeatBal->Zone(zoneNum).numSpaces;
assert(state.dataHeatBal->Zone(zoneNum).numSpaces == int(state.dataHeatBal->Zone(zoneNum).spaceIndexes.size()));
// If some surfaces in the zone are assigned to a space, the new space is the remainder of the zone
state.dataHeatBal->space(state.dataGlobal->numSpaces).Name = thisZone.Name + "-Remainder";
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 where the "-Remainder" spaces get created if needed. The full-zone default spaces were created in HeatBalanceManager::GetSpaceData.

// All grouped zones have been assigned to an enclosure, now assign remaining zones
for (int zoneNum = 1; zoneNum <= state.dataGlobal->NumOfZones; ++zoneNum) {
int zoneEnclosureNum = 0;
// Check for any spaces defined only by floor surface(s) and group them
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is a space has only floor surfaces it's lumped into an enclosure for all similar spaces in the same zone plus any "-Remainder" space.

Comment on lines 888 to 891
state.dataSize->ZoneSizing(DesDayNum, CtrlZoneNum).OADesMethod = state.dataSize->ZoneSizingInput(ZoneSizNum).OADesMethod;
state.dataSize->ZoneSizing(DesDayNum, CtrlZoneNum).DesOAFlowPPer = state.dataSize->ZoneSizingInput(ZoneSizNum).DesOAFlowPPer;
state.dataSize->ZoneSizing(DesDayNum, CtrlZoneNum).DesOAFlowPerArea = state.dataSize->ZoneSizingInput(ZoneSizNum).DesOAFlowPerArea;
state.dataSize->ZoneSizing(DesDayNum, CtrlZoneNum).DesOAFlow = state.dataSize->ZoneSizingInput(ZoneSizNum).DesOAFlow;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

More unnecessary copying of OA specs in sizing.

@rraustad
Copy link
Contributor

@Myoldmopar The way I see this is there is nothing more to do on this branch. Let me know if you want additional time for review. The only thing I see is the warning in the new example file which will be addressed in a follow up issue.

** Warning ** GetInternalHeatGains: People="ZONE 5-Remainder ZONELIST PEOPLE", specifies People per Floor Area, but Space Floor Area = 0.  0 People will result.

@mjwitte
Copy link
Contributor Author

mjwitte commented Aug 20, 2021

We need to wait for linux to catch up and check the performance score.

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 code changes found only a few issues which were corrected. Unit tests run locally and new example file was tested for issues. This new feature allows multiple spaces per zone with very targeted and limited changes to code (except for restructuring). A+ on implementation.

@rraustad
Copy link
Contributor

+1% on performance. Can we get someone to profile this branch and compare that to the most recent profile? I suspect the new OA functions are getting called. I didn't check but that could be called on FirstHVACIteration.

@mjwitte
Copy link
Contributor Author

mjwitte commented Aug 21, 2021

@rraustad Nah, I don't think the OA calc is the problem 15ZonePSZ shows the same runtime increase, and it only uses DesignSpec:OA for sizing.

I'm pretty sure the extra work is from the internal gain loops for space. I tried messing around to get the people loops to vectorize here and the run count got even longer, so that's not worth pursuing.

But looking at that same section of code it would help to only accumulate the space values in the main internal gain loop and then go through the zones and sum the space subtotals. I'll try that instead.

@Myoldmopar
Copy link
Member

Given the time, let's just get this in without waiting on the investigation of the 1%. Report on that in the follow-up issue. I'll tweak the example file to resolve the conflict and then get this merged. Thanks for all this @mjwitte and the reviewing @rraustad

@Myoldmopar
Copy link
Member

Pulled develop in, fixed example file conflict and ran tests. All tests pass. Ran regressions and reproduced exactly what CI reported the last time. This is going in. What. an. effort. Thanks @mjwitte for the work, thanks @rraustad for the reviewing. I am excited to see how this is used by clients moving forward.

@Myoldmopar Myoldmopar merged commit 55404d8 into develop Aug 21, 2021
@Myoldmopar Myoldmopar deleted the spaces branch August 21, 2021 20:09
@rraustad
Copy link
Contributor

I am wondering what improvement could be had by calculating the constants in CalcDesignSpecificationOutdoorAir early (e.g., OA flow per zone, floor area, ACH, etc.) and during the simulation it's just something like:

OA flow = PeopleFlow + thisSpace->DSOAContstant;

I see there are many variations for zone OA flow calculations but I still think the number of calculations during run time where CalcDesignSpecificationOutdoorAir is called could be reduced.

  \key Flow/Person
  \key Flow/Area
  \key Flow/Zone
  \key AirChanges/Hour
  \key Sum
  \key Maximum
  \key IndoorAirQualityProcedure
  \key ProportionalControlBasedOnDesignOccupancy
  \key ProportionalControlBasedOnOccupancySchedule

@mjwitte
Copy link
Contributor Author

mjwitte commented Aug 23, 2021

@rraustad Great suggestion. I'm planning to move this to be a member function and then any fixed values can be stored as member variables.

@mjwitte
Copy link
Contributor Author

mjwitte commented Sep 17, 2021

I am wondering what improvement could be had by calculating the constants in CalcDesignSpecificationOutdoorAir early (e.g., OA flow per zone, floor area, ACH, etc.) and during the simulation it's just something like:

@rraustad Just to close the loop on this. The problem with precalculating anything is that the same DesignSpec:OA object can be applied to more than one zone/space, so the way it's currently structured won't work for that. In order that that to work, I think we'd need to clone the DSOA object for each instance that it's referenced, and then those fixed values could be initialized once.

@Myoldmopar Myoldmopar changed the title Add Space Add Space Concept to EnergyPlus Zone Structure, Part 1 Sep 22, 2021
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.