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

Intersection issue impacting create bar measure #128

Open
DavidGoldwasser opened this issue Jul 2, 2021 · 12 comments
Open

Intersection issue impacting create bar measure #128

DavidGoldwasser opened this issue Jul 2, 2021 · 12 comments
Assignees

Comments

@DavidGoldwasser
Copy link
Contributor

DavidGoldwasser commented Jul 2, 2021

Here is issue filed in OpenStudio
NREL/OpenStudio#4372

I'l add a new test in spec on branch to replicate it. The use case is already bypassing intersection between different stories, but is hitting issue on intersection b waetween lls within a story. There is currently not a way to bypass this. Measure fails because of a check on expected exterior area by orientation when party walls are not used.

@DavidGoldwasser
Copy link
Contributor Author

Here is test. This seems to be .matchSurfaces and not about intersection, although I'll add one or more additional tests that might involve intersection.

Here is the resulting model, first floor is fine. Check in measure for expected exterior walls by orientation catches the problems and returns false with an error.

Screen Shot 2021-07-02 at 2 42 59 PM

@DavidGoldwasser
Copy link
Contributor Author

On an additional test where I change make_mid_floor_adiabatic to false that same failures happens and additional missed matches seen on this screenshot. This isn't a huge surprise, this method was made precisely because it failed so often. Failure for matching walls within a story was less common.

Screen Shot 2021-07-02 at 3 31 57 PM

@DavidGoldwasser
Copy link
Contributor Author

This last error may be un-related to intersection or matching, and maybe is in the measure i'll investigate this one.

ERROR MESSAGES
Hospital Corridor doesn't have the expected floor area (actual 5,175 ft^2, target 4,657 ft^2)
Hospital NurseStn doesn't have the expected floor area (actual 3,840 ft^2, target 4,609 ft^2)
Hospital PatRoom doesn't have the expected floor area (actual 2,512 ft^2, target 2,260 ft^2)
Sum of actual floor area is 22289.999999999964 ft^2, sum of target floor area is 22290.000000000004.

Screen Shot 2021-07-02 at 3 46 17 PM

@DavidGoldwasser
Copy link
Contributor Author

DavidGoldwasser commented Jul 2, 2021

Ok, I know put what may be three separate issues together, sorry. I've had breakthrough on the first issue, and I believe it is related to tolerance for intersection vs. tolerance for surface matching.

First, a little background on how create_bar used to work and how it works now (importantly it can revert to the old approach which should be work around for now)

I used to slice the bar up by space types, trying generally to avoid duplicate space types on multiple stories, although it is needed some times. I put smaller spaces on bottom story and work up to large space types. There is an option for mixed use buildings to keep space types from building types together but that doesn't play a role here. Below is the first story of a school the old way. The top story has a little more corridor and then classrooms.Each strip is split up to represent core and perimeter zones.

Screen Shot 2021-07-02 at 4 38 52 PM

Most building types have a primary space type that typically is double loaded around a corridor or hallway. The new default approach supports this by merging two space types together for a custom strip. Instead of determine the width of each of them from a default perimeter zone is is for the school the ratio of classroom to corridor that determines the corridor width. Below is the new way, and notice the corridor is much narrower than the center zone of the other space types. The walls intersect fine.

Screen Shot 2021-07-02 at 4 36 46 PM

Here is the hospital the old way, it works fine for intersecting and matching the walls.

Screen Shot 2021-07-02 at 4 22 55 PM

Here is the hospital the new way. What do you notice? One thing there are an enormous amount of corridors relative to patient rooms. I may consider allowing corridor to run through all space types vs. just the primary, although maybe that varies by building type, or the modeler can always choose.

Screen Shot 2021-07-02 at 4 24 24 PM

The second thing, the default core width looks almost exactly the same corridor width. In fact it is about 0.013 meter.

Screen Shot 2021-07-02 at 4 28 40 PM

SketchUp's intersection can identify and intersect it, but OpenStudio's intersect does not. It acts as if it is too small a difference to intersect, but too big of a difference to be considered a match. Whatever tolerance we have for intersection and matching there should be an overlap of coverage instead of a gap. After I manually intersected the six base surfaces in SketchUp, OpenStudio's surface matching properly picked them up as shown below.

Screen Shot 2021-07-02 at 4 56 56 PM

Conclusion, there is a fix needed in core OpenStudio, but the quick fix for now is to change the double_loaded_corridor argument value from Primary Space Type to None.

My suggested fix is to both fix the default tolerances so there is overlap, and then ideally expose the tolerance values to the measure writer in the API so they can adjust as needed for different use cases.

@jmarrec
Copy link
Contributor

jmarrec commented Jul 5, 2021

https://github.com/NREL/OpenStudio/blob/3bfe89f6c441d1e61e50b8e94e92e7218b4555a0/src/model/Surface.cpp#L1036-L1038

    boost::optional<SurfaceIntersection> Surface_Impl::computeIntersection(Surface& otherSurface) {
      double tol = 0.01;       //  1 cm tolerance
      double areaTol = 0.001;  // 10 cm2 tolerance

So what you measure in Sketchup (0.013 m), is right on the limit.

(Note: there seem to be some places where the tolerance might be different: https://github.com/NREL/OpenStudio/blob/3bfe89f6c441d1e61e50b8e94e92e7218b4555a0/src/utilities/geometry/Intersection.cpp#L108)

@DavidGoldwasser
Copy link
Contributor Author

@jmarrec what I mean, or at least what I'm observing in practice (not looking at code), is that 0.013m is too small of a delta to create an intersection, but too large of a delta to be counted as a matched surface. I see with line you refer to it should be intersected, unless it is rounded and has to be great than 0.01?

My suggestion is that if a value "A" is used as tolerance for surface intersection, that a value "B" used for tolerance allowed to still be considered match should be larger than "A". This way there is an overlap of coverage and not a gap or a hard line (both the same value) that may act like a gap).

I also think if it would be great if an optional argument was added to both intersect and match to allow API users to customize the tolerance.

@DavidGoldwasser
Copy link
Contributor Author

DavidGoldwasser commented Oct 11, 2021

I've bypassed this issue for our use in the extension gem by bypassing intersection and matching for walls similar to how I do for windows. This is controlled by the same argument about adiabatic mid stories. I'll keep this open for now so we can test core OS intersection and matching fix later on.

@mdahlhausen I don't think this should impact ComStock and EULP much. The walls will have construction from interior walls hard assigned but will have adiabatic boundary vs. surface boundary. Here is what it looks like now

Screen Shot 2021-10-11 at 10 24 14 AM

@DavidGoldwasser
Copy link
Contributor Author

Standards sometimes looks for adjacent spaces, so I'm going to update this to try to intersect when it can and use adiabatic as fallback. I'll also modify a test that exercises the adiabatic argument and has a kitchen to run through both create_bar and create_typical

@DavidGoldwasser
Copy link
Contributor Author

Here is updated approach on a branch now. This should generally support methods in standards to find spaces adjacent to kitchen space types, unless it happens to be where matching doesn't work on both sides or is at an end.

Screen Shot 2021-10-29 at 5 55 06 PM

@DavidGoldwasser
Copy link
Contributor Author

Decided to keep open until fixed in OS so can confirm it here.

@DavidGoldwasser
Copy link
Contributor Author

Here is basic test I made with 10' x 10' cubes that shift increasing amounts starting at 1/8th inch up to 2 inches. Very small shift is fine because it doesn't split but it is recognized as a match. The large gap is fine because it does split and then it matches. The middle ground is the problem where it doesn't split but also doesn't match. If We need to increase decrease the size of shift that gets split or increase the tolerance of difference that still counts as a match to catch the middle cases.

Screen Shot 2022-02-03 at 12 58 18 AM

@tijcolem
Copy link
Contributor

tijcolem commented Aug 26, 2022

@DavidGoldwasser Was this fixed in NREL/OpenStudio#4527 Could you retest with a 3.4.0 installer?

@tijcolem tijcolem assigned DavidGoldwasser and unassigned jmarrec Aug 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants