search

openstudiocoalition / openstudio-sketchup-plugin

The OpenStudio SketchUp Plug-in is an extension to Trimble’s popular 3D modeling tool that adds OpenStudio context to the SketchUp program.
https://openstudiocoalition.org
Other
45 stars 10 forks source link

Looser matching #115

Closed macumber closed 1 year ago

macumber commented 1 year ago

Fixes #75

Ski90Moo commented 1 year ago

Ok, I think I see what you are trying to do. I had thought the same thing when I started working on this, but the centroids seemed to be close enough to be successful 90+% of the time. Often the unmatched surfaces have some underlying issue that should be addressed by the energy modeler. Ultimately, if the surface areas are sufficiently mismatched, EP will throw an error when running the model.

Running this code on this test model test4.txt; the surface matching method appears to take 4.4x longer than the original one I had proposed in PR#113. Also, it only successfully matches ~50% of the surfaces.

If you think a looser match would improve successful matches, maybe we could try something simpler like truncate() on the centroid coordinates?

macumber commented 1 year ago

Yeah this algorithm isn't really done or ready yet. I think offering two buttons "Exact Match" and "Loose Match" might be a good idea. Matching based on area and centroid might be good enough for loose match, it would be nice to have a few more checks (e.g. to prevent a horizonal match with a rectangle in a "cross" shape).

The current issue I am concerned about is that sometimes SketchUp will add self intersecting polygons after an intersect. If we are supposed to get two rectangular surfaces but get one self intersecting polygon, then there is no matching algorithm that can solve the problem. If we need to detect self intersecting polygons, we might have to get into clipping or some other advanced algorithms. If we go that route, then we could use polygon difference methods to check for matches. Not quite sure the right way to go.

Ski90Moo commented 1 year ago

Can you attach the test model with the self intersecting polygon/horizontal match?

macumber commented 1 year ago

I don't have a test model with those surfaces, I was adding a method I could call as a unit test from the SketchUp console: https://github.com/openstudiocoalition/openstudio-sketchup-plugin/pull/115/files#diff-b7bced7aba5ce5a8c2104fffa4146134b3de09f29fa6f8c7a1590f6249736854R1190

Ski90Moo commented 1 year ago

I don't have a test model with those surfaces, I was adding a method I could call as a unit test from the SketchUp console: https://github.com/openstudiocoalition/openstudio-sketchup-plugin/pull/115/files#diff-b7bced7aba5ce5a8c2104fffa4146134b3de09f29fa6f8c7a1590f6249736854R1190

Ok, I wonder if it is even still a problem? As I stated, it seems SketchUp corrected these self-intersecting polygons a while ago.

Ski90Moo commented 1 year ago

I like the idea of giving the user an option for loose match and exact match.