From c7809705bea7d8c4140288a9c65c4c376f7d05c6 Mon Sep 17 00:00:00 2001 From: Julien Marrec Date: Mon, 10 Jul 2023 11:23:21 +0200 Subject: [PATCH 1/3] Add a test for #10103 ``` [ RUN ] EnergyPlusFixture.SurfaceGeometry_CheckConvexity_ColinearStability /home/julien/Software/Others/EnergyPlus/tst/EnergyPlus/unit/SurfaceGeometry.unit.cc:3331: Failure Expected equality of these values: 6 floorSurface.Sides Which is: 8 /home/julien/Software/Others/EnergyPlus/tst/EnergyPlus/unit/SurfaceGeometry.unit.cc:3350: Failure Expected equality of these values: 6 ceilingSurface.Sides Which is: 9 /home/julien/Software/Others/EnergyPlus/tst/EnergyPlus/unit/SurfaceGeometry.unit.cc:3354: Failure Expected equality of these values: floorSurface.Sides Which is: 8 ceilingSurface.Sides Which is: 9 ``` --- tst/EnergyPlus/unit/SurfaceGeometry.unit.cc | 80 +++++++++++++++++++++ 1 file changed, 80 insertions(+) diff --git a/tst/EnergyPlus/unit/SurfaceGeometry.unit.cc b/tst/EnergyPlus/unit/SurfaceGeometry.unit.cc index 3514025723b..2961676d9e6 100644 --- a/tst/EnergyPlus/unit/SurfaceGeometry.unit.cc +++ b/tst/EnergyPlus/unit/SurfaceGeometry.unit.cc @@ -3274,6 +3274,86 @@ TEST_F(EnergyPlusFixture, SurfaceGeometry_CheckConvexityTest_ASHRAE901_Hospital_ EXPECT_FALSE(surface.IsConvex); } +TEST_F(EnergyPlusFixture, SurfaceGeometry_CheckConvexity_ColinearStability) +{ + // Test for #10103 - Regardless of the order of the vertices, we should drop colinear vertices consistently to avoid a fatal error due to vertex + // side mismatch + // + // y + // ▲ 7 8 9 + // │ ┌─────x────┐ + // │ │ │ + // │ │ │ + // │ │ │ + // │5 │ │ + // ├──────────┘ │ + // │ 6 │ + // │ │ + // │ │ + // │ │10 + // └────x─────x─────x────┴─────► + // 4 3 2 1 x + + const std::vector floorVertices = { + {30.0, 0., 0.}, + {20.0, 0., 0.}, + {10.0, 0., 0.}, + {0.0, 0., 0.}, + {0.0, 20., 0.}, + {20.0, 20., 0.}, + {20.0, 40., 0.}, + {30.0, 40., 0.}, + {40.0, 40., 0.}, + {40.0, 0., 0.}, + }; + const int nVertices = static_cast(floorVertices.size()); + + state->dataSurface->TotSurfaces = 2; + const int floorSurfNum = 1; + const int ceilingSurfNum = 2; + state->dataSurface->MaxVerticesPerSurface = nVertices; + state->dataSurfaceGeometry->SurfaceTmp.allocate(state->dataSurface->TotSurfaces); + + auto &floorSurface = state->dataSurfaceGeometry->SurfaceTmp(floorSurfNum); + auto &ceilingSurface = state->dataSurfaceGeometry->SurfaceTmp(ceilingSurfNum); + { + floorSurface.Azimuth = 0.0; + floorSurface.Tilt = 0.0; + floorSurface.Sides = nVertices; + floorSurface.GrossArea = 100.0; + floorSurface.Name = "Floor"; + floorSurface.Vertex.allocate(nVertices); + + floorSurface.Vertex = floorVertices; + + CheckConvexity(*state, floorSurfNum, floorSurface.Sides); + + EXPECT_EQ(6, floorSurface.Sides); + EXPECT_FALSE(floorSurface.IsConvex); + } + + { + auto ceilingVertices = floorVertices; + std::reverse(ceilingVertices.begin(), ceilingVertices.end()); + + ceilingSurface.Azimuth = 0.0; + ceilingSurface.Tilt = 0.0; + ceilingSurface.Sides = nVertices; + ceilingSurface.GrossArea = 100.0; + ceilingSurface.Name = "Ceiling"; + ceilingSurface.Vertex.allocate(nVertices); + + ceilingSurface.Vertex = ceilingVertices; + + CheckConvexity(*state, ceilingSurfNum, ceilingSurface.Sides); + + EXPECT_EQ(6, ceilingSurface.Sides); + EXPECT_FALSE(ceilingSurface.IsConvex); + } + + EXPECT_EQ(floorSurface.Sides, ceilingSurface.Sides); +} + TEST_F(EnergyPlusFixture, InitialAssociateWindowShadingControlFenestration_test) { state->dataSurface->TotWinShadingControl = 3; From 20cb122615d56d886593f14569bf9685b379a31b Mon Sep 17 00:00:00 2001 From: Julien Marrec Date: Mon, 10 Jul 2023 11:51:59 +0200 Subject: [PATCH 2/3] Fix #10103 - CheckConvexity removes colinear vertices but stops when surface is determined non-convex, causing vertex size mismatch fatal error --- src/EnergyPlus/SurfaceGeometry.cc | 47 ++++++++++++++++--------------- 1 file changed, 25 insertions(+), 22 deletions(-) diff --git a/src/EnergyPlus/SurfaceGeometry.cc b/src/EnergyPlus/SurfaceGeometry.cc index 2e8c2149639..bc8f2a3e5d8 100644 --- a/src/EnergyPlus/SurfaceGeometry.cc +++ b/src/EnergyPlus/SurfaceGeometry.cc @@ -15788,30 +15788,33 @@ namespace SurfaceGeometry { } if (SignFlag != PrevSignFlag) { - if (state.dataHeatBal->SolarDistribution != DataHeatBalance::Shadowing::Minimal && surfaceTmp.ExtSolar) { - if (state.dataGlobal->DisplayExtraWarnings) { - ShowWarningError(state, - format("CheckConvexity: Zone=\"{}\", Surface=\"{}\" is non-convex.", - state.dataHeatBal->Zone(surfaceTmp.Zone).Name, - surfaceTmp.Name)); - int Np1 = n + 1; - if (Np1 > NSides) { - Np1 -= NSides; - } - int Np2 = n + 2; - if (Np2 > NSides) { - Np2 -= NSides; - } - ShowContinueError(state, format("...vertex {} to vertex {} to vertex {}", n, Np1, Np2)); - ShowContinueError(state, format("...vertex {}=[{:.2R},{:.2R},{:.2R}]", n, X(n), Y(n), Z(n))); - ShowContinueError(state, format("...vertex {}=[{:.2R},{:.2R},{:.2R}]", Np1, X(n + 1), Y(n + 1), Z(n + 1))); - ShowContinueError(state, format("...vertex {}=[{:.2R},{:.2R},{:.2R}]", Np2, X(n + 2), Y(n + 2), Z(n + 2))); - // ShowContinueError(state, format("...theta angle=[{:.6R}]", Theta)); - // ShowContinueError(state, format("...last theta angle=[{:.6R}]", LastTheta)); - } + if (state.dataGlobal->DisplayExtraWarnings && surfaceTmp.ExtSolar && + (state.dataHeatBal->SolarDistribution != DataHeatBalance::Shadowing::Minimal) && + // Warn only once + surfaceTmp.IsConvex) { + ShowWarningError(state, + format("CheckConvexity: Zone=\"{}\", Surface=\"{}\" is non-convex.", + state.dataHeatBal->Zone(surfaceTmp.Zone).Name, + surfaceTmp.Name)); + int Np1 = n + 1; + if (Np1 > NSides) { + Np1 -= NSides; + } + int Np2 = n + 2; + if (Np2 > NSides) { + Np2 -= NSides; + } + ShowContinueError(state, format("...vertex {} to vertex {} to vertex {}", n, Np1, Np2)); + ShowContinueError(state, format("...vertex {}=[{:.2R},{:.2R},{:.2R}]", n, X(n), Y(n), Z(n))); + ShowContinueError(state, format("...vertex {}=[{:.2R},{:.2R},{:.2R}]", Np1, X(n + 1), Y(n + 1), Z(n + 1))); + ShowContinueError(state, format("...vertex {}=[{:.2R},{:.2R},{:.2R}]", Np2, X(n + 2), Y(n + 2), Z(n + 2))); + // ShowContinueError(state, format("...theta angle=[{:.6R}]", Theta)); + // ShowContinueError(state, format("...last theta angle=[{:.6R}]", LastTheta)); } surfaceTmp.IsConvex = false; - break; + // #10103 - We do not want to break early, because we do want to consistently remove colinear vertices + // to avoid potential vertex size mismatch fatal errors + // break; } PrevSignFlag = SignFlag; LastTheta = Theta; From 791803924a3bb082f740f5d7ea7bf512435a0006 Mon Sep 17 00:00:00 2001 From: Julien Marrec Date: Tue, 18 Jul 2023 16:26:01 +0200 Subject: [PATCH 3/3] custom_check in unit test --- tst/EnergyPlus/unit/SurfaceGeometry.unit.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tst/EnergyPlus/unit/SurfaceGeometry.unit.cc b/tst/EnergyPlus/unit/SurfaceGeometry.unit.cc index 2961676d9e6..f502dc4a80a 100644 --- a/tst/EnergyPlus/unit/SurfaceGeometry.unit.cc +++ b/tst/EnergyPlus/unit/SurfaceGeometry.unit.cc @@ -3309,8 +3309,8 @@ TEST_F(EnergyPlusFixture, SurfaceGeometry_CheckConvexity_ColinearStability) const int nVertices = static_cast(floorVertices.size()); state->dataSurface->TotSurfaces = 2; - const int floorSurfNum = 1; - const int ceilingSurfNum = 2; + constexpr int floorSurfNum = 1; + constexpr int ceilingSurfNum = 2; state->dataSurface->MaxVerticesPerSurface = nVertices; state->dataSurfaceGeometry->SurfaceTmp.allocate(state->dataSurface->TotSurfaces);