From d1e217efd4f3334555ea208dfbfb7deaad236bf4 Mon Sep 17 00:00:00 2001 From: Ski90Moo Date: Tue, 14 Jul 2026 11:42:35 +0200 Subject: [PATCH 1/5] feat: add hot-gas reheat unitary system with Coil:Heating:Desuperheater Adds 'Unitary - Single Speed DX cooling - Elec heat - CAV - Hotgas reheat' to the default HVAC library, using CoolReheat dehumidification control with a Coil:Heating:Desuperheater as the reheat coil. The desuperheater reclaims waste heat from the unitary system's own DX cooling coil rather than using resistance reheat, avoiding the extra energy cost of electric reheat. Also fixes AirLoopHVACUnitarySystem cloning (drag-from-library and "copy system"): ModelObject::clone() only remaps parent-child ownership, but CoilHeatingDesuperheater::heatingSource() is a lateral reference to a sibling coil, so it was left unset on the clone. fixupClonedDesuperheaterHeatingSource() re-links the cloned desuperheater coil to the cloned cooling coil. Co-Authored-By: Claude Sonnet 5 --- .../Resources/default/hvac_library.osm | 58 +++++++++++++++++++ src/openstudio_lib/HVACSystemsController.cpp | 36 ++++++++++++ 2 files changed, 94 insertions(+) diff --git a/src/openstudio_app/Resources/default/hvac_library.osm b/src/openstudio_app/Resources/default/hvac_library.osm index 54c76447f..f23fb1598 100644 --- a/src/openstudio_app/Resources/default/hvac_library.osm +++ b/src/openstudio_app/Resources/default/hvac_library.osm @@ -6999,6 +6999,64 @@ OS:AirLoopHVAC:UnitarySystem, , !- Heat Recovery Water Outlet Node Name ; !- Design Specification Multispeed Object Name +OS:Coil:Heating:Desuperheater, + {bdb03443-be0a-4953-8eb4-e6014bbced83}, !- Handle + Desuperheater Reheat Coil, !- Name + {9f54092d-a4a8-41b8-a381-c4c332ecb843}, !- Availability Schedule Name + 0.25, !- Heat Reclaim Recovery Efficiency + , !- Air Inlet Node Name + , !- Air Outlet Node Name + {983b9c20-5a9e-49eb-9770-5c1545cc6b65}, !- Heating Source Name + 0; !- On Cycle Parasitic Electric Load {W} + +OS:AirLoopHVAC:UnitarySystem, + {dbc42aea-40d9-42a6-9016-87cc16813b11}, !- Handle + Unitary - Single Speed DX cooling - Elec heat - CAV - Hotgas reheat, !- Name + Load, !- Control Type + , !- Controlling Zone or Thermostat Location + CoolReheat, !- Dehumidification Control Type + {9f54092d-a4a8-41b8-a381-c4c332ecb843}, !- Availability Schedule Name + , !- Air Inlet Node Name + , !- Air Outlet Node Name + {0c016d27-85cb-4254-8b85-9a16c4586de5}, !- Supply Fan Name + BlowThrough, !- Fan Placement + {9f54092d-a4a8-41b8-a381-c4c332ecb843}, !- Supply Air Fan Operating Mode Schedule Name + {ca3b3fed-58b0-4f64-85f9-257f45d033a5}, !- Heating Coil Name + 1, !- DX Heating Coil Sizing Ratio + {983b9c20-5a9e-49eb-9770-5c1545cc6b65}, !- Cooling Coil Name + No, !- Use DOAS DX Cooling Coil + 2, !- DOAS DX Cooling Coil Leaving Minimum Air Temperature {C} + LatentOrSensibleLoadControl, !- Latent Load Control + {bdb03443-be0a-4953-8eb4-e6014bbced83}, !- Supplemental Heating Coil Name + SupplyAirFlowRate, !- Supply Air Flow Rate Method During Cooling Operation + autosize, !- Supply Air Flow Rate During Cooling Operation {m3/s} + , !- Supply Air Flow Rate Per Floor Area During Cooling Operation {m3/s-m2} + , !- Fraction of Autosized Design Cooling Supply Air Flow Rate + , !- Design Supply Air Flow Rate Per Unit of Capacity During Cooling Operation {m3/s-W} + SupplyAirFlowRate, !- Supply Air Flow Rate Method During Heating Operation + autosize, !- Supply Air Flow Rate During Heating Operation {m3/s} + , !- Supply Air Flow Rate Per Floor Area during Heating Operation {m3/s-m2} + , !- Fraction of Autosized Design Heating Supply Air Flow Rate + , !- Design Supply Air Flow Rate Per Unit of Capacity During Heating Operation {m3/s-W} + None, !- Supply Air Flow Rate Method When No Cooling or Heating is Required + , !- Supply Air Flow Rate When No Cooling or Heating is Required {m3/s} + , !- Supply Air Flow Rate Per Floor Area When No Cooling or Heating is Required {m3/s-m2} + , !- Fraction of Autosized Design Cooling Supply Air Flow Rate When No Cooling or Heating is Required + , !- Fraction of Autosized Design Heating Supply Air Flow Rate When No Cooling or Heating is Required + , !- Design Supply Air Flow Rate Per Unit of Capacity During Cooling Operation When No Cooling or Heating is Required {m3/s-W} + , !- Design Supply Air Flow Rate Per Unit of Capacity During Heating Operation When No Cooling or Heating is Required {m3/s-W} + Yes, !- No Load Supply Air Flow Rate Control Set To Low Speed + 80, !- Maximum Supply Air Temperature {C} + 21, !- Maximum Outdoor Dry-Bulb Temperature for Supplemental Heater Operation {C} + , !- Outdoor Dry-Bulb Temperature Sensor Node Name + 0, !- Ancilliary On-Cycle Electric Power {W} + 0, !- Ancilliary Off-Cycle Electric Power {W} + , !- Design Heat Recovery Water Flow Rate {m3/s} + 80, !- Maximum Temperature for Heat Recovery {C} + , !- Heat Recovery Water Inlet Node Name + , !- Heat Recovery Water Outlet Node Name + ; !- Design Specification Multispeed Object Name + OS:Fan:OnOff, {e803e540-d848-409c-9bad-0c0dc9c7b50a}, !- Handle Fan On Off 14, !- Name diff --git a/src/openstudio_lib/HVACSystemsController.cpp b/src/openstudio_lib/HVACSystemsController.cpp index f5f498e8e..a20b811e3 100644 --- a/src/openstudio_lib/HVACSystemsController.cpp +++ b/src/openstudio_lib/HVACSystemsController.cpp @@ -92,6 +92,12 @@ #include #include #include +#include +#include +#include +#include +#include +#include #include #include #include @@ -519,6 +525,34 @@ void HVACLayoutController::addLibraryObjectToTopLevel(const OSItemId& itemId) { message.exec(); } +namespace { + // ModelObject::clone() only remaps true parent-child ownership (children()), so it correctly + // duplicates a UnitarySystem's fan/coils. But CoilHeatingDesuperheater::heatingSource() is a + // lateral reference to a sibling coil rather than a child, so it is left unset by the clone. + // If that sibling is the unitary system's own cooling coil, re-link the clone to it. + // Walks the full cloned subtree since the UnitarySystem may be nested (e.g. cloning a whole + // AirLoopHVAC via "copy system") rather than being the clone root itself. + void fixupClonedDesuperheaterHeatingSource(model::ModelObject clonedObject) { + if (boost::optional unitary = clonedObject.optionalCast()) { + if (boost::optional supplementalCoil = unitary->supplementalHeatingCoil()) { + boost::optional desuperheater = supplementalCoil->optionalCast(); + if (desuperheater && !desuperheater->heatingSource()) { + if (boost::optional coolingCoil = unitary->coolingCoil()) { + desuperheater->setHeatingSource(coolingCoil.get()); + } + } + } + } + + // children() is declared on ParentObject, not the generic ModelObject, so cast before recursing. + if (boost::optional parent = clonedObject.optionalCast()) { + for (const model::ModelObject& child : parent->children()) { + fixupClonedDesuperheaterHeatingSource(child); + } + } + } +} // namespace + void HVACLayoutController::addLibraryObjectToModelNode(const OSItemId& itemId, model::HVACComponent& comp) { model::OptionalModelObject object; bool remove = false; @@ -528,6 +562,7 @@ void HVACLayoutController::addLibraryObjectToModelNode(const OSItemId& itemId, m if (object) { if (!doc->fromModel(itemId)) { object = object->clone(comp.model()); + fixupClonedDesuperheaterHeatingSource(object.get()); remove = true; } } @@ -893,6 +928,7 @@ void HVACSystemsController::onCopySystemClicked() { auto loop = currentLoop(); if (loop) { auto clone = loop->clone(loop->model()); + fixupClonedDesuperheaterHeatingSource(clone); setCurrentHandle(toQString(clone.handle())); } } From e77a9b03b2d44bd8981a60c68aa6f6faa1b8158c Mon Sep 17 00:00:00 2001 From: Ski90Moo Date: Tue, 14 Jul 2026 21:13:57 +0200 Subject: [PATCH 2/5] refactor: generalize clone lateral-reference fixup beyond desuperheater ModelObject::clone() clones each child individually and reattaches it via setParent(), so parent-child edges are correctly remapped, but lateral (sibling-to-sibling) object-list references within the cloned subtree are left pointing at the original objects. The previous fix special-cased just CoilHeatingDesuperheater::heatingSource(), but the same bug applies to any lateral reference (e.g. SetpointManager:MixedAir's node fields). Replace it with a generic fixup: build an old-handle -> clone map by walking the original and cloned subtrees in parallel, then use IddObject::objectLists()/WorkspaceObject::getTarget()/setPointer() to rewrite any object-list field in the clone that still points into the original subtree. This fixes the desuperheater case as before, and any future lateral-reference case, without needing a new special case per object type. Co-Authored-By: Claude Sonnet 5 --- src/openstudio_lib/HVACSystemsController.cpp | 78 ++++++++++++++------ 1 file changed, 56 insertions(+), 22 deletions(-) diff --git a/src/openstudio_lib/HVACSystemsController.cpp b/src/openstudio_lib/HVACSystemsController.cpp index a20b811e3..6e72b565c 100644 --- a/src/openstudio_lib/HVACSystemsController.cpp +++ b/src/openstudio_lib/HVACSystemsController.cpp @@ -92,10 +92,6 @@ #include #include #include -#include -#include -#include -#include #include #include #include @@ -132,6 +128,8 @@ #include #include +#include + #include #include #include @@ -526,31 +524,66 @@ void HVACLayoutController::addLibraryObjectToTopLevel(const OSItemId& itemId) { } namespace { - // ModelObject::clone() only remaps true parent-child ownership (children()), so it correctly - // duplicates a UnitarySystem's fan/coils. But CoilHeatingDesuperheater::heatingSource() is a - // lateral reference to a sibling coil rather than a child, so it is left unset by the clone. - // If that sibling is the unitary system's own cooling coil, re-link the clone to it. - // Walks the full cloned subtree since the UnitarySystem may be nested (e.g. cloning a whole - // AirLoopHVAC via "copy system") rather than being the clone root itself. - void fixupClonedDesuperheaterHeatingSource(model::ModelObject clonedObject) { - if (boost::optional unitary = clonedObject.optionalCast()) { - if (boost::optional supplementalCoil = unitary->supplementalHeatingCoil()) { - boost::optional desuperheater = supplementalCoil->optionalCast(); - if (desuperheater && !desuperheater->heatingSource()) { - if (boost::optional coolingCoil = unitary->coolingCoil()) { - desuperheater->setHeatingSource(coolingCoil.get()); - } + // ModelObject::clone() clones each child individually and reattaches it via setParent(), so + // true parent-child edges are correctly remapped to point within the new subtree. But children + // are cloned one at a time rather than as a batch sharing a single old->new handle table, so any + // *lateral* object-list reference between two siblings in the cloned subtree (e.g. + // CoilHeatingDesuperheater::heatingSource() pointing at a sibling cooling coil, or a + // SetpointManager's node references) is left pointing at the original objects instead of their + // clones. The functions below fix that up generically, for any object type, rather than special + // casing each lateral-reference field as it's discovered. + + // Walks `original` and `clone` in parallel -- children() order is deterministic and preserved by + // clone() -- building a map from each original object's handle to its corresponding clone. + void buildCloneHandleMap(const model::ModelObject& original, const model::ModelObject& clone, + std::map& handleMap) { + handleMap.emplace(original.handle(), clone); + + boost::optional originalParent = original.optionalCast(); + boost::optional cloneParent = clone.optionalCast(); + if (originalParent && cloneParent) { + std::vector originalChildren = originalParent->children(); + std::vector cloneChildren = cloneParent->children(); + if (originalChildren.size() == cloneChildren.size()) { + for (size_t i = 0; i < originalChildren.size(); ++i) { + buildCloneHandleMap(originalChildren[i], cloneChildren[i], handleMap); } } } + } + + // Rewrites every object-list field on clonedObject (and recursively its children) that still + // points at an object handleMap has a clone for, to point at that clone instead. + void remapLateralReferences(model::ModelObject clonedObject, const std::map& handleMap) { + IddObject iddObject = clonedObject.iddObject(); + for (unsigned index = 0; index < clonedObject.numFields(); ++index) { + if (iddObject.objectLists(index).empty()) { + continue; + } + boost::optional target = clonedObject.getTarget(index); + if (!target) { + continue; + } + auto it = handleMap.find(target->handle()); + if (it != handleMap.end() && it->second.handle() != target->handle()) { + clonedObject.setPointer(index, it->second.handle()); + } + } - // children() is declared on ParentObject, not the generic ModelObject, so cast before recursing. if (boost::optional parent = clonedObject.optionalCast()) { for (const model::ModelObject& child : parent->children()) { - fixupClonedDesuperheaterHeatingSource(child); + remapLateralReferences(child, handleMap); } } } + + // `original` is the subtree that was cloned; `clonedObject` is its clone. Re-links any lateral + // reference in the clone that still points into the original subtree. + void fixupClonedReferences(const model::ModelObject& original, model::ModelObject clonedObject) { + std::map handleMap; + buildCloneHandleMap(original, clonedObject, handleMap); + remapLateralReferences(clonedObject, handleMap); + } } // namespace void HVACLayoutController::addLibraryObjectToModelNode(const OSItemId& itemId, model::HVACComponent& comp) { @@ -561,8 +594,9 @@ void HVACLayoutController::addLibraryObjectToModelNode(const OSItemId& itemId, m object = doc->getModelObject(itemId); if (object) { if (!doc->fromModel(itemId)) { + model::ModelObject original = object.get(); object = object->clone(comp.model()); - fixupClonedDesuperheaterHeatingSource(object.get()); + fixupClonedReferences(original, object.get()); remove = true; } } @@ -928,7 +962,7 @@ void HVACSystemsController::onCopySystemClicked() { auto loop = currentLoop(); if (loop) { auto clone = loop->clone(loop->model()); - fixupClonedDesuperheaterHeatingSource(clone); + fixupClonedReferences(loop.get(), clone); setCurrentHandle(toQString(clone.handle())); } } From 59177f7e0870789574908f9527ef98fd5737f46b Mon Sep 17 00:00:00 2001 From: Ski90Moo Date: Wed, 15 Jul 2026 14:37:25 +0200 Subject: [PATCH 3/5] fix: correct clone fixup for fields clone() clears instead of staling Rename the library template to "Desuperheater reheat" for clarity. Testing the generalized fixup against the real SDK (loading hvac_library.osm and cloning the new UnitarySystem both within the same model and across models) showed remapLateralReferences was a no-op for CoilHeatingDesuperheater::heatingSource(): clone() doesn't leave that field pointing at the stale original object, it clears it outright. The previous implementation only rewrote fields that already had some target set on the clone, so it silently did nothing for this case -- reproduced live in the app as a fatal EnergyPlus error (missing heating_source_name / heating_source_object_type) when simulating the new system. Fixed by reading the field to remap from `original` instead of from the clone, and unconditionally forcing the clone's field to match whenever the original's target was itself part of the cloned subtree. Verified against the SDK directly (Ruby) for both same-model ("copy system") and cross-model (drag-from-library) clone scenarios, and confirmed the simulation now runs cleanly in the rebuilt app. Co-Authored-By: Claude Sonnet 5 --- .../Resources/default/hvac_library.osm | 2 +- src/openstudio_lib/HVACSystemsController.cpp | 53 ++++++++++++------- 2 files changed, 36 insertions(+), 19 deletions(-) diff --git a/src/openstudio_app/Resources/default/hvac_library.osm b/src/openstudio_app/Resources/default/hvac_library.osm index f23fb1598..5737c1576 100644 --- a/src/openstudio_app/Resources/default/hvac_library.osm +++ b/src/openstudio_app/Resources/default/hvac_library.osm @@ -7011,7 +7011,7 @@ OS:Coil:Heating:Desuperheater, OS:AirLoopHVAC:UnitarySystem, {dbc42aea-40d9-42a6-9016-87cc16813b11}, !- Handle - Unitary - Single Speed DX cooling - Elec heat - CAV - Hotgas reheat, !- Name + Unitary - Single Speed DX cooling - Elec heat - CAV - Desuperheater reheat, !- Name Load, !- Control Type , !- Controlling Zone or Thermostat Location CoolReheat, !- Dehumidification Control Type diff --git a/src/openstudio_lib/HVACSystemsController.cpp b/src/openstudio_lib/HVACSystemsController.cpp index 6e72b565c..2d00c50fe 100644 --- a/src/openstudio_lib/HVACSystemsController.cpp +++ b/src/openstudio_lib/HVACSystemsController.cpp @@ -529,9 +529,10 @@ namespace { // are cloned one at a time rather than as a batch sharing a single old->new handle table, so any // *lateral* object-list reference between two siblings in the cloned subtree (e.g. // CoilHeatingDesuperheater::heatingSource() pointing at a sibling cooling coil, or a - // SetpointManager's node references) is left pointing at the original objects instead of their - // clones. The functions below fix that up generically, for any object type, rather than special - // casing each lateral-reference field as it's discovered. + // SetpointManager's node references) is left broken on the clone -- for some field/type + // combinations still pointing at the original object, for others (empirically, heatingSource()) + // cleared to empty outright. The functions below fix that up generically, for any object type, + // rather than special casing each lateral-reference field as it's discovered. // Walks `original` and `clone` in parallel -- children() order is deterministic and preserved by // clone() -- building a map from each original object's handle to its corresponding clone. @@ -552,37 +553,53 @@ namespace { } } - // Rewrites every object-list field on clonedObject (and recursively its children) that still - // points at an object handleMap has a clone for, to point at that clone instead. - void remapLateralReferences(model::ModelObject clonedObject, const std::map& handleMap) { - IddObject iddObject = clonedObject.iddObject(); - for (unsigned index = 0; index < clonedObject.numFields(); ++index) { + // For every lateral object-list field on `original` whose target was itself cloned (i.e. has an + // entry in handleMap), forces the corresponding field on `clonedObject` to point at that clone. + // Fields are read from `original`, not from `clonedObject`: clone() doesn't necessarily leave a + // lateral reference pointing at the stale original object -- for CoilHeatingDesuperheater's + // heatingSource() it clears the field outright -- so the only reliable source of "what this + // field is supposed to point at" is the original. + void remapLateralReferences(const model::ModelObject& original, model::ModelObject clonedObject, + const std::map& handleMap) { + IddObject iddObject = original.iddObject(); + for (unsigned index = 0; index < original.numFields(); ++index) { if (iddObject.objectLists(index).empty()) { continue; } - boost::optional target = clonedObject.getTarget(index); - if (!target) { + boost::optional originalTarget = original.getTarget(index); + if (!originalTarget) { continue; } - auto it = handleMap.find(target->handle()); - if (it != handleMap.end() && it->second.handle() != target->handle()) { - clonedObject.setPointer(index, it->second.handle()); + auto it = handleMap.find(originalTarget->handle()); + if (it == handleMap.end()) { + continue; + } + boost::optional clonedTarget = clonedObject.getTarget(index); + if (clonedTarget && clonedTarget->handle() == it->second.handle()) { + continue; } + clonedObject.setPointer(index, it->second.handle()); } - if (boost::optional parent = clonedObject.optionalCast()) { - for (const model::ModelObject& child : parent->children()) { - remapLateralReferences(child, handleMap); + boost::optional originalParent = original.optionalCast(); + boost::optional cloneParent = clonedObject.optionalCast(); + if (originalParent && cloneParent) { + std::vector originalChildren = originalParent->children(); + std::vector cloneChildren = cloneParent->children(); + if (originalChildren.size() == cloneChildren.size()) { + for (size_t i = 0; i < originalChildren.size(); ++i) { + remapLateralReferences(originalChildren[i], cloneChildren[i], handleMap); + } } } } // `original` is the subtree that was cloned; `clonedObject` is its clone. Re-links any lateral - // reference in the clone that still points into the original subtree. + // reference in the clone that still points into (or should point into) the original subtree. void fixupClonedReferences(const model::ModelObject& original, model::ModelObject clonedObject) { std::map handleMap; buildCloneHandleMap(original, clonedObject, handleMap); - remapLateralReferences(clonedObject, handleMap); + remapLateralReferences(original, clonedObject, handleMap); } } // namespace From 203331fb3f411b84b22faae1aec4775a881154d1 Mon Sep 17 00:00:00 2001 From: Ski90Moo Date: Wed, 15 Jul 2026 15:51:20 +0200 Subject: [PATCH 4/5] fix: reach loop-branch equipment when fixing up cloned references ParentObject::children() is true ownership (e.g. a UnitarySystem's own fan/coils) but does not include HVACComponents placed on a Loop's supply/demand branches. A UnitarySystem sitting on an AirLoopHVAC's supply branch is only reachable via supplyComponents(), not children(). Cloning a whole loop ("Copy System") therefore never visited the branch equipment at all -- including the desuperheater's heating source inside it -- because buildCloneHandleMap/remapLateralReferences only recursed via children(). Reproduced live: copying an existing (working) desuperheater airloop and reassigning zones still hit the same fatal EnergyPlus error, since the copy's desuperheater was never touched by the fixup. Fixed by walking children(), supplyComponents(), and demandComponents() as independently size-matched groups rather than one combined list -- demand-side counts legitimately differ between original and clone (connected thermal zones are never duplicated by clone(), by design), so that mismatch must not block fixing up the supply side, where the equipment lines up 1:1 with the original. Verified against the SDK directly (Ruby) by reproducing the actual AirLoopHVAC topology (OA system, unitary system, zone, terminal) and confirmed live in the app: both "drag from library" and "Copy System" on a whole airloop now correctly re-wire the desuperheater's heating source and a manually-added SetpointManager:MixedAir's node references. Co-Authored-By: Claude Sonnet 5 --- src/openstudio_lib/HVACSystemsController.cpp | 50 +++++++++++++++----- 1 file changed, 38 insertions(+), 12 deletions(-) diff --git a/src/openstudio_lib/HVACSystemsController.cpp b/src/openstudio_lib/HVACSystemsController.cpp index 2d00c50fe..c3df593c8 100644 --- a/src/openstudio_lib/HVACSystemsController.cpp +++ b/src/openstudio_lib/HVACSystemsController.cpp @@ -534,17 +534,43 @@ namespace { // cleared to empty outright. The functions below fix that up generically, for any object type, // rather than special casing each lateral-reference field as it's discovered. - // Walks `original` and `clone` in parallel -- children() order is deterministic and preserved by - // clone() -- building a map from each original object's handle to its corresponding clone. + // ParentObject::children() is true ownership (e.g. a UnitarySystem's fan/coils) but does *not* + // include HVACComponents placed on a Loop's supply/demand branches -- a UnitarySystem sitting on + // an AirLoopHVAC's supply branch is reachable via supplyComponents(), not children(). Cloning a + // whole loop (the "copy system" toolbar action) needs both, or the branch equipment -- and + // anything lateral-referenced from inside it, like the desuperheater's heating source -- is + // never visited at all. + // + // Returned as separate groups (children / supplyComponents / demandComponents), each matched and + // recursed into independently: cloning a loop does not carry over the connected thermal zones on + // the demand side, so original vs. clone demandComponents() can legitimately have different + // sizes. Treating everything as one combined list would let a benign demand-side mismatch abort + // recursion into the supply side too, where the equipment (and lateral references like the + // desuperheater's heating source) *does* line up 1:1 with the original. + std::vector> childSubtreeObjectGroups(const model::ModelObject& object) { + std::vector> groups; + if (boost::optional loop = object.optionalCast()) { + groups.push_back(loop->supplyComponents()); + groups.push_back(loop->demandComponents()); + } + if (boost::optional parent = object.optionalCast()) { + groups.push_back(parent->children()); + } + return groups; + } + + // Walks `original` and `clone` in parallel -- childSubtreeObjectGroups() order is deterministic + // and preserved by clone() -- building a map from each original object's handle to its + // corresponding clone. void buildCloneHandleMap(const model::ModelObject& original, const model::ModelObject& clone, std::map& handleMap) { handleMap.emplace(original.handle(), clone); - boost::optional originalParent = original.optionalCast(); - boost::optional cloneParent = clone.optionalCast(); - if (originalParent && cloneParent) { - std::vector originalChildren = originalParent->children(); - std::vector cloneChildren = cloneParent->children(); + std::vector> originalGroups = childSubtreeObjectGroups(original); + std::vector> cloneGroups = childSubtreeObjectGroups(clone); + for (size_t g = 0; g < originalGroups.size() && g < cloneGroups.size(); ++g) { + const std::vector& originalChildren = originalGroups[g]; + const std::vector& cloneChildren = cloneGroups[g]; if (originalChildren.size() == cloneChildren.size()) { for (size_t i = 0; i < originalChildren.size(); ++i) { buildCloneHandleMap(originalChildren[i], cloneChildren[i], handleMap); @@ -581,11 +607,11 @@ namespace { clonedObject.setPointer(index, it->second.handle()); } - boost::optional originalParent = original.optionalCast(); - boost::optional cloneParent = clonedObject.optionalCast(); - if (originalParent && cloneParent) { - std::vector originalChildren = originalParent->children(); - std::vector cloneChildren = cloneParent->children(); + std::vector> originalGroups = childSubtreeObjectGroups(original); + std::vector> cloneGroups = childSubtreeObjectGroups(clonedObject); + for (size_t g = 0; g < originalGroups.size() && g < cloneGroups.size(); ++g) { + const std::vector& originalChildren = originalGroups[g]; + const std::vector& cloneChildren = cloneGroups[g]; if (originalChildren.size() == cloneChildren.size()) { for (size_t i = 0; i < originalChildren.size(); ++i) { remapLateralReferences(originalChildren[i], cloneChildren[i], handleMap); From 39e82f564f7b0234c65fef03c2077a6bf33dd288 Mon Sep 17 00:00:00 2001 From: Ski90Moo Date: Wed, 15 Jul 2026 18:00:22 +0200 Subject: [PATCH 5/5] fix: reach outdoor-air/relief branch equipment in clone fixup too Same traversal gap as loop supply/demand branches, one level deeper: equipment placed on an AirLoopHVACOutdoorAirSystem's outdoor-air or relief branch (e.g. an evaporative cooler, or a SetpointManager:MixedAir sitting on one of those nodes) is reachable only via oaComponents()/reliefComponents(), not children() and not the loop's own supplyComponents()/demandComponents() (which only see the OA system as a single opaque object on the main branch). Reproduced with the stock File > Examples > Example Model: its airloop's outdoor air system has an evaporative cooler with a SetpointManager: MixedAir on it. Copying that airloop left the copy's setpoint manager's node fields empty, since the fixup never visited that branch at all. Fixed by adding oaComponents()/reliefComponents() as two more independently size-matched groups in childSubtreeObjectGroups(). Verified against the SDK directly (Ruby, using OpenStudio::Model.exampleModel) -- all 4 of the example model's SetpointManagerMixedAir objects correctly resolve their node references on the clone -- and confirmed live in the rebuilt app. Co-Authored-By: Claude Sonnet 5 --- src/openstudio_lib/HVACSystemsController.cpp | 25 ++++++++++++-------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/src/openstudio_lib/HVACSystemsController.cpp b/src/openstudio_lib/HVACSystemsController.cpp index c3df593c8..2abc5d2e9 100644 --- a/src/openstudio_lib/HVACSystemsController.cpp +++ b/src/openstudio_lib/HVACSystemsController.cpp @@ -536,23 +536,28 @@ namespace { // ParentObject::children() is true ownership (e.g. a UnitarySystem's fan/coils) but does *not* // include HVACComponents placed on a Loop's supply/demand branches -- a UnitarySystem sitting on - // an AirLoopHVAC's supply branch is reachable via supplyComponents(), not children(). Cloning a - // whole loop (the "copy system" toolbar action) needs both, or the branch equipment -- and - // anything lateral-referenced from inside it, like the desuperheater's heating source -- is - // never visited at all. + // an AirLoopHVAC's supply branch is reachable via supplyComponents(), not children(). The same + // gap exists one level deeper: equipment on an AirLoopHVACOutdoorAirSystem's outdoor-air/relief + // branches (e.g. an evaporative cooler, or a SetpointManager:MixedAir sitting on one of those + // nodes) is reachable only via oaComponents()/reliefComponents(), not children() either. Cloning + // a whole loop (the "copy system" toolbar action) needs all of these, or the branch equipment -- + // and anything lateral-referenced from inside it -- is never visited at all. // - // Returned as separate groups (children / supplyComponents / demandComponents), each matched and - // recursed into independently: cloning a loop does not carry over the connected thermal zones on - // the demand side, so original vs. clone demandComponents() can legitimately have different - // sizes. Treating everything as one combined list would let a benign demand-side mismatch abort - // recursion into the supply side too, where the equipment (and lateral references like the - // desuperheater's heating source) *does* line up 1:1 with the original. + // Returned as separate groups, each matched and recursed into independently: cloning a loop does + // not carry over the connected thermal zones on the demand side, so original vs. clone + // demandComponents() can legitimately have different sizes. Treating everything as one combined + // list would let a benign demand-side mismatch abort recursion into the other groups too, where + // the equipment (and any lateral references inside it) *does* line up 1:1 with the original. std::vector> childSubtreeObjectGroups(const model::ModelObject& object) { std::vector> groups; if (boost::optional loop = object.optionalCast()) { groups.push_back(loop->supplyComponents()); groups.push_back(loop->demandComponents()); } + if (boost::optional oaSystem = object.optionalCast()) { + groups.push_back(oaSystem->oaComponents()); + groups.push_back(oaSystem->reliefComponents()); + } if (boost::optional parent = object.optionalCast()) { groups.push_back(parent->children()); }