NatLabRockies / NatLabRockies/OpenStudio
ModelObject::clone() doesn't remap lateral (sibling-to-sibling) object-list references within the cloned subtree
Nobody has claimed this yet.
- Dominant language
- C++
- Stars
- 646
- Forks
- 237
- Avg merge
- 3d 11h
- Merged PRs (30d)
- 10
Description
Summary
ModelObject::clone() (and by extension Loop::clone() / AirLoopHVAC::clone()) clones each child individually and reattaches it via setParent(), so true parent-child ownership edges are correctly remapped to point within the new subtree. However, 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 is left broken on the clone.
Concretely:
CoilHeatingDesuperheater::heatingSource()— when the source is a sibling coil on the same loop, the field is cleared to empty outright on the clone, rather than pointing at the cloned sibling.SetpointManagernode references on anAirLoopHVACOutdoorAirSystem's outdoor-air/relief branch equipment — these still point at the original node objects rather than the clone's.
Both cause EnergyPlus to fail with a fatal error when the resulting model is simulated, since the referenced field is either blank or points at an object outside the new subtree.
To reproduce
- Build an
AirLoopHVACwith aCoilCoolingDXSingleSpeedand aCoilHeatingDesuperheaterwhoseheatingSource()is set to that cooling coil. - Clone the loop:
AirLoopHVAC newLoop = loop.clone(model).cast<AirLoopHVAC>(); - Find the cloned desuperheater coil and inspect
heatingSource()— it will be empty (or, for other field/type combinations, still point at the original object) instead of pointing at the cloned cooling coil.
Suggested fix
clone() would need to batch-clone the whole subtree into one shared old->new handle map first (rather than cloning children one at a time via setParent()), then do a second pass remapping any object-list field whose target has an entry in that handle map. This is a bigger structural change to clone()'s orchestration, so it likely needs its own design discussion rather than a quick patch.
Context
We hit this in openstudiocoalition/OpenStudioApplication#891 (adding hot-gas-reheat desuperheater support) and worked around it at the application level with a generic post-clone fixup pass (see src/utilities/CloneFixup.cpp in that PR) rather than touching clone() itself. Filing this so the underlying SDK gap is tracked — the app-level fix could be removed if this is addressed upstream.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start at ModelObject::clone() and trace how Loop::clone() and AirLoopHVAC::clone() clone and reattach children. Reproduce the sibling-reference case with the coils described in the issue, then compare the behavior with the application-level workaround in src/utilities/CloneFixup.cpp. Done means cloned sibling and node references resolve within the new subtree and the resulting model no longer fails simulation with these fatal errors.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp
- Domain
- backend
- Issue type
- Bug
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 42/100