NREL / OpenStudio

OpenStudio is a cross-platform collection of software tools to support whole building energy modeling using EnergyPlus and advanced daylight analysis using Radiance.
https://www.openstudio.net/
Other
484 stars 185 forks source link

Interior windows missing required Outside Boundary Condition Object when ModelMerger #5153

Closed macumber closed 2 months ago

macumber commented 2 months ago

Issue overview

Originally reported as an issue with FloorspaceJS translator at https://github.com/NREL/OpenStudio/issues/4670

@manuvarkey found the issue actually occurs in ModelMerger

Current Behavior

Adjacent Surfaces are matched after cloning in ModelMerger but adjacent SubSurfaces are not

Expected Behavior

Adjacent surfaces and SubSurfaces are matched after cloning in ModelMerger

Steps to Reproduce

import openstudio as os

# Open floorplan and get translated OSM model
with open('floorplan.json') as fp:
    json_str = fp.read()
fp_json = os.FloorplanJS(json_str)
ts_scene = fp_json.toThreeScene(True)
rt = os.model.ThreeJSReverseTranslator()
os_model_tr = rt.modelFromThreeJS(ts_scene).get()
model_handle_mapping = rt.handleMapping()

# Merge translated model with blank model and save
model = os.model.Model()
mm = os.model.ModelMerger()
mm.mergeModels(model, os_model_tr, model_handle_mapping)
model.save('out2.osm')

Possible Solution

On going through the code for OpenStudio/src/model/ModelMerger.cpp, the following code segment was found dealing with setting adjacentSurface for Surface elements.

https://github.com/NREL/OpenStudio/blob/d2f0bdd21a70e6a9711f6da8f5aabecadc4a1a1c/src/model/ModelMerger.cpp#L349C1-L359C6

      // DLM: this should probably be moved to a mergeSurface method
      auto clone = newSurface.clone(m_currentModel).cast<Surface>();
      clone.setSpace(currentSpace);

      m_newMergedHandles.insert(newSurface.handle());
      m_currentToNewHandleMapping[clone.handle()] = newSurface.handle();
      m_newToCurrentHandleMapping[newSurface.handle()] = clone.handle();

      boost::optional<Surface> newAdjacentSurface = newSurface.adjacentSurface();
      if (newAdjacentSurface) {
        boost::optional<UUID> currentAdjacentSurfaceHandle = getCurrentModelHandle(newAdjacentSurface->handle());
        if (currentAdjacentSurfaceHandle) {
          boost::optional<Surface> currentAdjacentSurface = m_currentModel.getModelObject<Surface>(*currentAdjacentSurfaceHandle);
          if (currentAdjacentSurface) {
            clone.setAdjacentSurface(*currentAdjacentSurface);
            }
          }
        }
      }

Best solution would be to implement a mergeSurface method that tracks the handles of sub surfaces too.

Details

Environment

Some additional details about your environment for this issue (if relevant):

Context

Incorrect simulation results

macumber commented 2 months ago

I'm working on a potential fix

AltamarMx commented 1 month ago

i still see this problem, with interiors doors and windows, each subsurface is missing the outside boundary condition pointing to the other subsurface

I am using OS app 1.7.1+... with SDK 3.7.0

The geometry was created with floorspace within OpenStudio