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

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

Closed
macumber opened this issue Apr 18, 2024 · 2 comments · Fixed by #5154
Closed
Labels
Triage Issue needs to be assessed and labeled, further information on reported might be needed

Comments

@macumber
Copy link
Contributor

macumber commented Apr 18, 2024

Issue overview

Originally reported as an issue with FloorspaceJS translator at #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):

  • Platform (Operating system, version):
  • Version of OpenStudio (if using an intermediate build, include SHA): 3.7.0

Context

Incorrect simulation results

@macumber macumber added the Triage Issue needs to be assessed and labeled, further information on reported might be needed label Apr 18, 2024
@macumber
Copy link
Contributor Author

I'm working on a potential fix

macumber added a commit to openstudiocoalition/OpenStudio that referenced this issue Apr 18, 2024
jmarrec pushed a commit to openstudiocoalition/OpenStudio that referenced this issue Apr 18, 2024
jmarrec pushed a commit to openstudiocoalition/OpenStudio that referenced this issue Apr 19, 2024
@AltamarMx
Copy link

AltamarMx commented Jun 6, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Triage Issue needs to be assessed and labeled, further information on reported might be needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants