Repository navigation
Conversation
…sets" (OSW) / excludeConstructionAssignmentSets
…nmentSet Generate FT/RT from TranslatorGenerator, then I had to modify some things manually ``` require_relative 'SubProjectClassGenerators/TranslatorGenerator' [ ['DefaultConstructionSet', 'ConstructionAssignmentSet'], ['DefaultSurfaceConstructions', 'SurfaceConstructionAssignments'], ['DefaultSubSurfaceConstructions', 'SubSurfaceConstructionAssignments'], ].each do |className, epIddName| t = TranslatorGenerator.new(epIddName) t.instance_variable_set(:@classname, className) t.write_forward_translator t.write_reverse_translator t.write_ft_rt_tests(true, true) end ```
…OS 5 levels into E+ 2 levels
Equivalence: on a model where all 5 levels win a surface, every surface resolves to the same construction after collapsing, at search distance 1 or 4 (with and without purge). Idempotence: a second collapse is a no-op
…d purge tests NotPruned: a lower level set that fills another category (ext vs int wall, ext vs ground floor) or another kind of slot (sub-surface, interior partition) gets merged in. Purge: a set moved down to the Space is kept, merged originals are removed along with their sub-objects, except the ones the merged set still uses
…t as base) Only checks what the Space resolves to slot by slot (Space > SpaceType > BuildingStory), and that lower level sets are untouched, so it holds whether the Space set is merged into in place or cloned (haven't decided yet)
…s untouched The Space's own set is also the Building's and its SpaceType contributes a slot: the Space resolves the merged content, but the Building's set must not pick it up, since other Spaces may fall through to it
…et that has no other use Only a Space or the Building can be merged into in place: directUseCount() == 1 means a single user only for them, not for a SpaceType or BuildingStory. Adds MergedInPlace tests for both groups, and BuildingGroup_Merged becomes MergedCloned_UsedBySpace
…oup, Building Group)
…d by another level is cloned
… before the call Counts drop as owners get pointed to merged clones, so 2 Spaces sharing a set would get a clone for the first and an in place merge for the second. Adds the SharedSameKey test: both get their own clone
…rge with a Space set base tests SharedDifferentKeys: Spaces sharing their set but with different SpaceTypes get one clone each. Purge: a set merged into in place survives, a shared set that got cloned is removed. BuildingGroupUntouched now also checks the Space is on a clone.
…nstruction Conflicts before? Documents a known limitation: collapsing changes search distances, which can flip the construction a matched pair resolves to, so the ForwardTranslator must resolve matched pairs before collapsing.
…structions via ConstructionAssignmentSet Call DefaultConstructionSet::collapseToSpaceAndBuilding before space types are removed and spaces combined, and write the Space's set to Space's ConstructionAssignmentSetName. S Surface/SubSurface only write their construction when hard-assigned, when the SubSurfaceType was flipped (Door/OverheadDoor <-> GlassDoor, E+ would look it up in a different slot), or when construction assignment sets are excluded. SubSurfaceTypes are no longer mapped to the legacy Window/Door since E+ supports FixedWindow/OperableWindow/Skylight/OverheadDoor. Surface DefaultConstruction test now covers both modes. Left a couple TODO
OpenStudio doesn't resolve a default construction for GroundFCfactorMethod, Foundation, OtherSideCoefficients and OtherSideConditionsModel, but EnergyPlus looks them up in the ConstructionAssignmentSet and may pick an incorrect one (eg a layered construction on a GroundFCfactorMethod surface). Not a common case, and I don't want to change the DefaultConstructionSet/Surface::isGroundSurface model behavior, but might as well warn. The proper fix would be to align DefaultConstructionSet_Impl::getDefaultConstruction with E+'s ConstructionAssignmentSetData::getConstructionAssignment: route Surfaces via Surface::isGroundSurface() (adds GroundFCfactorMethod, Foundation) to Ground Contact, OtherSideCoefficients to Exterior and OtherSideConditionsModel to Interior; and SubSurfaces whose base surface isn't Outdoors/Surface to Interior (Exterior for OtherSideCoefficients).
Space and Building (incl. Building SpaceType merge) reference the collapsed sets, SubSurfaceType passthrough and Door/GlassDoor flips, hard-assigned and matched pair constructions are written, and the warning for GroundFCfactorMethod. Collapse logic itself stays tested in the model namespace.
…uded ConstructionAssignmentSets rely on E+ Spaces to hold the Space level set. When Space translation is excluded there is no E+ Space, so defaulted constructions would end up unresolved. The FT now excludes ConstructionAssignmentSets for that translation (all constructions are written to the Surfaces), logs an Error, and restores the user's option at the end. Tests that exclude Space translation now also exclude ConstructionAssignmentSets so they don't hit the Error, and so does the Modelica path in OSWorkflow.
…lation is excluded ccd362b Will implement a better alternative
…en Space translation is excluded When Space translation is excluded there is no E+ Space to hold the Space level ConstructionAssignmentSet, so defaulted constructions coming from the Space, SpaceType or BuildingStory DefaultConstructionSets would end up unresolved in E+. Before collapsing the DefaultConstructionSets, the FT now hard-assigns those constructions to the Surfaces, SubSurfaces and InteriorPartitionSurfaces, following the same search order as Space::getDefaultConstructionWithSearchDistance. Building level defaults are still left to E+ via the Building's ConstructionAssignmentSet.
…is excluded Checks that the FT hard-assigns the constructions from the Space, SpaceType and BuildingStory levels following the OpenStudio search order (SpaceType wins over BuildingStory, fall through to the BuildingStory, defaulted SpaceType skipped), for Surfaces and SubSurfaces alike, while the Building and Building SpaceType levels are left blank for E+ to resolve via the Building's ConstructionAssignmentSet.
| bool excludeConstructionAssignmentSets() const; | ||
| bool isExcludeConstructionAssignmentSetsDefaulted() const; | ||
| void setExcludeConstructionAssignmentSets(bool excludeConstructionAssignmentSets); | ||
| void resetExcludeConstructionAssignmentSets(); |
There was a problem hiding this comment.
New WorkflowJSON ft_options: no_construction_assignment_sets.
| static constexpr bool DEFAULT_NO_VARIABLE_DICTIONARY = false; | ||
| static constexpr bool DEFAULT_NO_SPACE_TRANSLATION = false; // At 3.4.1, this was changed to false. | ||
| static constexpr bool DEFAULT_NO_SPACE_LOAD_INSTANCES = false; | ||
| static constexpr bool DEFAULT_NO_CONSTRUCTION_ASSIGNMENT_SETS = false; |
There was a problem hiding this comment.
Exclusion = false by default, meaning the default behavior is now to translate to ConstructionAssignmentSets
| app | ||
| ->add_flag( | ||
| "--construction-assignment-sets,!--no-construction-assignment-sets", | ||
| [opt](std::int64_t val) { | ||
| if (val != 0) { | ||
| opt->runOptions.forwardTranslatorOptions().setExcludeConstructionAssignmentSets((val != 1)); | ||
| } | ||
| }, | ||
| "Translate DefaultConstructionSets to the E+ ConstructionAssignmentSet objects referenced by Space/Building [Default: True]") | ||
| ->group(ftGroupName); |
There was a problem hiding this comment.
New CLI flag to opt-out. Same as --no-space-translation / --no-space-load-instances
| if (!(this->adiabaticSurfaceConstruction())) { | ||
| construction = other.adiabaticSurfaceConstruction(); | ||
| if (construction) { | ||
| this->setAdiabaticSurfaceConstruction(*construction); | ||
| } | ||
| } |
There was a problem hiding this comment.
Fixup the DCS::merge, which wasn't handling the Adiabatic (was doing all the others ok)
There was a problem hiding this comment.
Should this fix be isolated from this PR?
| /** Collapses the DefaultConstructionSet inheritance (Space, SpaceType, BuildingStory, Building, Building SpaceType) to the | ||
| * Space and Building levels, the only two that the EnergyPlus ConstructionAssignmentSet supports. | ||
| * | ||
| * Space -> SpaceType (explicit only) -> BuildingStory -> Building -> Building SpaceType | ||
| * \__________________ ___________________________/ \_______________ _____________/ | ||
| * V V | ||
| * Space Group Building Group | ||
| * | ||
| * For the Building, then for each Space, the sets of the levels it inherits from are considered in priority order. A | ||
| * lower-priority set that fills no slot left empty by the ones above it is ignored. If one set remains, it is assigned as is. | ||
| * If several remain, they are merged into a new set, created once per combination of owners and shared by all Spaces with | ||
| * that combination (e.g. "SpaceType Office @ BuildingStory L1"). | ||
| * | ||
| * If purge is false, SpaceType and BuildingStory sets are left in place: they no longer affect any surface. | ||
| * If purge is true, they are reset, then the sets that were referenced before the call and are now unused are removed, | ||
| * along with their DefaultSurfaceConstructions / DefaultSubSurfaceConstructions if those are now unused too. Sets that were | ||
| * already unused before the call are left alone. | ||
| * | ||
| * Every surface resolves to the same construction afterwards, but search distances change, which affects how the | ||
| * constructions of matched surfaces are resolved: resolve those first if needed. | ||
| * This modifies the model in place: call it on a clone to keep the original. | ||
| * Returns the DefaultConstructionSets that were created. */ | ||
| static std::vector<DefaultConstructionSet> collapseToSpaceAndBuilding(Model& model, bool purge = false); |
There was a problem hiding this comment.
Start here. OS has 5 levels of DefaultConstructionSet, E+ only 2. This collapses:
- Space / SpaceType (explicit only) / BuildingStory into the Space
- Building / Building SpaceType into the Building
preserving the OS search order. The doc comment above has the diagram
| if (!m_forwardTranslatorOptions.excludeConstructionAssignmentSets()) { | ||
| // NOTE: the DefaultConstructionSet of the Building is already merged with the DCS of the Building's SpaceType if needed (collapseToSpaceAndBuilding) | ||
| if (auto defaultConstructionSet_ = modelObject.defaultConstructionSet()) { | ||
| if (auto wo_ = translateAndMapModelObject(*defaultConstructionSet_)) { | ||
| idfObject.setString(BuildingFields::ConstructionAssignmentSetName, wo_->nameString()); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
The Space and Building reference their (collapsed) set
| // Construction Assignment Set Name | ||
| if (!m_forwardTranslatorOptions.excludeConstructionAssignmentSets()) { | ||
| // NOTE: the DefaultConstructionSet of the Space is already merged with the DCS of its SpaceType and BuildingStory if needed (collapseToSpaceAndBuilding) | ||
| if (auto defaultConstructionSet_ = modelObject.defaultConstructionSet()) { | ||
| if (auto wo_ = translateAndMapModelObject(*defaultConstructionSet_)) { | ||
| idfObject.setString(SpaceFields::ConstructionAssignmentSetName, wo_->nameString()); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
The Space and Building reference their (collapsed) set
| // Unlike Surface/SubSurface, in E+, the ConstructionAssignmentSet (= DefaultConstructionSet in OS) does not have the 3 Space/Building/Site | ||
| // Shading Construction Fields (nor does the E+ Shading:* object have a Construction field): we use the construction it resolves to, to fill the ShadingProperty:Reflectance | ||
| boost::optional<model::ConstructionBase> constructionBase = modelObject.construction(); |
There was a problem hiding this comment.
Shading isn't in the E+ set, so nothing changes for shading surfaces
| EXPECT_EQ("*H.a.r.d.e.s.t*^\\1|-|8/_#($name$)?# 2", clone.name().get()); | ||
| } | ||
|
|
||
| TEST_F(ModelFixture, DefaultConstructionSet_Merge) { |
There was a problem hiding this comment.
I'm not going to comment on the 1800 lines of tests I added for the DefaultConstructionSet::collapseToSpaceAndBuilding, but It's thoroughly tested and usually includes comments explaining what we're testing
| TEST_F(ModelFixture, DefaultConstructionSet_collapseToSpaceAndBuilding_MatchedPairCanFlip) { | ||
|
|
||
| // Known limitation, documented here. For a matched pair (2 surfaces that are the 2 sides of the same wall) whose sides resolve | ||
| // different constructions, Surface::construction() picks the side with the lower search distance. Collapsing changes search | ||
| // distances (2, 3 -> 1 and 5 -> 4), so it can change which side wins. Callers that care (the ForwardTranslator) must resolve | ||
| // matched pairs before collapsing. | ||
| Model m; | ||
|
|
||
| // Two 10x10 Spaces side by side, sharing the wall at x = 10 | ||
| std::vector<Point3d> floorPoints{{0.0, 0.0, 0.0}, {0.0, 10.0, 0.0}, {10.0, 10.0, 0.0}, {10.0, 0.0, 0.0}}; | ||
| auto spaceA = Space::fromFloorPrint(floorPoints, 3.0, m).get(); | ||
| auto spaceB = Space::fromFloorPrint(floorPoints, 3.0, m).get(); | ||
| spaceB.setXOrigin(10.0); | ||
| spaceA.matchSurfaces(spaceB); | ||
|
|
||
| // B's side of the shared wall, and A's side | ||
| boost::optional<Surface> wallB; | ||
| for (const auto& surface : spaceB.surfaces()) { | ||
| if (surface.adjacentSurface()) { | ||
| wallB = surface; | ||
| } | ||
| } | ||
| ASSERT_TRUE(wallB); | ||
| auto wallA = wallB->adjacentSurface().get(); | ||
|
|
||
| // A's interior wall construction comes from A's own DCS: search distance 1 | ||
| Construction aIntWall(m); | ||
| EXPECT_TRUE(spaceA.setDefaultConstructionSet(makeDCS(m, {.intWall = aIntWall}))); | ||
|
|
||
| // B's interior wall construction comes from B's SpaceType DCS: search distance 2 | ||
| SpaceType spaceType(m); | ||
| Construction stIntWall(m); | ||
| EXPECT_TRUE(spaceType.setDefaultConstructionSet(makeDCS(m, {.intWall = stIntWall}))); | ||
| EXPECT_TRUE(spaceB.setSpaceType(spaceType)); | ||
|
|
||
| // Before: each side's own lookup... | ||
| EXPECT_EQ(1, wallA.constructionWithSearchDistance()->second); | ||
| EXPECT_EQ(2, wallB->constructionWithSearchDistance()->second); | ||
| EXPECT_EQ(stIntWall, wallB->constructionWithSearchDistance()->first); | ||
| // ... but the matched pair resolution picks A's (1 < 2), for both sides | ||
| EXPECT_EQ(aIntWall, wallA.construction().get()); | ||
| EXPECT_EQ(aIntWall, wallB->construction().get()); | ||
|
|
||
| // Collapsing moves the SpaceType's DCS down onto B | ||
| DefaultConstructionSet::collapseToSpaceAndBuilding(m); | ||
|
|
||
| // After: B's own lookup now comes from B's DCS, at search distance 1 too | ||
| EXPECT_EQ(1, wallA.constructionWithSearchDistance()->second); | ||
| EXPECT_EQ(1, wallB->constructionWithSearchDistance()->second); | ||
| // It's a tie, so each side keeps its own: B's side flipped from A's construction to its SpaceType's | ||
| EXPECT_EQ(aIntWall, wallA.construction().get()); | ||
| EXPECT_EQ(stIntWall, wallB->construction().get()); | ||
| } |
There was a problem hiding this comment.
MatchedPairCanFlip: document why we still call resolveMatchedSurfaceConstructionConflicts / resolveMatchedSubSurfaceConstructionConflicts in the FT.
|
CI Results for 360bb2d:
|
|
You still need to update the OSW schema on OpenStudio-workflow-gem (similar to NatLabRockies/OpenStudio-workflow-gem#160 but for Like NatLabRockies/OpenStudio-resources#234 for |
I had prepared it, just hadn't pushed yet. I'm reusing the same PR:
Yes, that PR deals with this feature, so I don't see why I would need to create two separate PRs? (Maybe I misunderstood the question though?) |
Pull request overview
ConstructionAssignmentSetobject (withSurfaceConstructionAssignments/SubSurfaceConstructionAssignments), referenced by theSpaceandBuildingobjects, to resolve default constructionsBehavior is that
DefaultConstructionSets will now by default translate to these new objects, and defaulted constructions are no longer written to the surfaces: E+ resolves them. But you can opt-outA matching key in WorkflowJSON:
no_construction_assignment_setsA matching method
ForwardTranslator::setExcludeConstructionAssignmentSets(bool)Details
DefaultConstructionSet(Space, SpaceType, BuildingStory, Building, Building SpaceType), E+ only 2 (Space, Building). The newDefaultConstructionSet::collapseToSpaceAndBuilding(Model&, bool purge = false)merges them, preserving the OpenStudio search order. The FT calls it on the model copyNote: I will flag it during review. But I went back and forth for a reaaaaaallly long time trying to figure out the smartest way to do that merge and the keying to 1) avoid too many duplicates and 2) preserve intent.
A lower level's DCS is only kept when it fills a slot that no higher level fills. The resulting merged DCS is created once per combination of owners (Space / SpaceType / BuildingStory / Building), not per combination of DCS nor per contents, and named after them (e.g.
SpaceType Office @ BuildingStory L1), so the name always says where it came from.Keying by contents (reusing any existing DCS with the same constructions) would give misleading names in the IDF. Counter example:
Lab Seton Space 104: exterior wall = Wall-AL1 Seton Story L1: ground floor = Slab-150Retail Seton SpaceType Retail: exterior wall = Wall-A, ground floor = Slab-150Merging Space 104's chain (
Lab Set, thenL1 Set) gives exactly the contents ofRetail Set, so Space 104 would end up on theRetail SetConstructionAssignmentSet even though nothing Retail is involved. The constructions would be right, but the name would be misleading. The more common case is two stories with copy-pasted identical sets: Spaces on L2 would share a set named after L1.The cost: two owners that share the same DCS (e.g. SpaceTypes Office and OpenOffice) each get their own merged set, so a few duplicates.
DefaultConstructionSet::merge, which was missing the adiabatic surface constructionresolveMatchedSurfaceConstructionConflicts)Window/Door, so E+ can pick the rightConstructionAssignmentSetslot (FixedWindow, OperableWindow, Skylight, OverheadDoor...)GroundFCfactorMethod,Foundation,OtherSideCoefficientsandOtherSideConditionsModel, OpenStudio doesn't resolve a default construction but E+ does look them up in theConstructionAssignmentSet: the FT warnsDesign
OpenStudio Lookup Order
Who points at what
Story and Space Type are independent: two Spaces on the same Story can have different Space Types, or none. The Building's Space Type is a regular SpaceType that a Space can also point at explicitly.
Collapsing to the two E+ levels
E+ resolves per slot too, but only through the surface, then the Space's set, then the Building's set. So the five levels are split in two groups, each collapsed onto one set:
In each group, going down:
SpaceType Office @ BuildingStory L1Collapsing changes the search distances (Space Type and Story become Space, Building Space Type becomes Building), which can change which side of a matched pair wins. That's why the matched pairs are resolved before collapsing.
Full doc: construction-set-inheritance.html
OpenStudio-resources
ForwardTranslate to new E+ ConstructionAssignmentSet (= DefaultConstructionSet in OS SDK) (opt-out possible) OpenStudio-resources#236
USE_CONSTRUCTION_ASSIGNMENT_SETSenvironment variabletest_construction_assignment_sets_py: sets at every level, checks that E+ resolves the same construction for every surface as when they are all written to the surfaces, and how (Construction Assignment Sourcein the EnvelopeSummary). Passes with and without E+ Spaces, and with and without the setsI need to run the full regression tests suite at:
develop
this PR with USE_CONSTRUCTION_ASSIGNMENT_SETS=false
this PR with USE_CONSTRUCTION_ASSIGNMENT_SETS=true
and confirm there are no failing tests and no EUI deviations.
Pull Request Author
src/model/test)src/energyplus/Test)src/osversion/VersionTranslator.cpp)Labels:
IDDChangeAPIChangePull Request - Ready for CIso that CI builds your PRReview Checklist
This will not be exhaustively relevant to every PR.