Fix WindowProperty:FrameAndDivider divider error checks#11629
Conversation
| state.dataSurface->FrameDivider(FrDivNum).VertDividers * surf.Height - | ||
| state.dataSurface->FrameDivider(FrDivNum).HorDividers * | ||
| state.dataSurface->FrameDivider(FrDivNum).VertDividers * DivWidth); | ||
| if (DivWidth * state.dataSurface->FrameDivider(FrDivNum).HorDividers > surf.Height) { |
There was a problem hiding this comment.
This check for total horizontal divider width not exceeding the height of the glazed opening, and the similar check below for vertical divider width, are the main additions in this PR.
| " 20, !- Number of Horizontal Dividers", | ||
| " 20, !- Number of Vertical Dividers", |
There was a problem hiding this comment.
Before, if we went from 19 -> 20 vertical dividers, the calculated divider area was less than the glazed opening (no error). Now we get an error -- the divider area calculation will be applied to a window with overlapping horizontal, or vertical, dividers (which doesn't make sense).
There was a problem hiding this comment.
Pull request overview
This PR is a follow-up to #11589 that tightens validation around WindowProperty:FrameAndDivider divider geometry in ProcessSurfaceVertices, and expands unit test coverage to assert the new severe error checks and resulting divider-area calculations.
Changes:
- Add severe error checks when horizontal/vertical divider widths cumulatively exceed the glazed opening height/width.
- Adjust divider-area error reporting and update the divider-geometry unit test to cover additional invalid cases.
- Extend the existing
SurfaceGeometry_BadDividerGeometryunit test with new frame/divider objects and fenestration surfaces.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/EnergyPlus/SurfaceGeometry.cc |
Adds new divider dimension validation and updates divider-area error handling in ProcessSurfaceVertices. |
tst/EnergyPlus/unit/SurfaceGeometry.unit.cc |
Expands the bad-divider-geometry test to cover new severe error conditions and divider area outcomes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
|
| {" ** Severe ** ProcessSurfaceVertices: Horizontal dividers exceed glazed opening height for window FENESTRATIONSURFACE", | ||
| " ** ~~~ ** Number of horizontal dividers=[20], divider width=[0.50] m, glazed opening height=[9.80] m.", | ||
| " ** Severe ** ProcessSurfaceVertices: Divider area exceeds glazed opening for window FENESTRATIONSURFACE", | ||
| " ** ~~~ ** Window surface area=[95.06] m2, divider area=[95.10] m2.", |
There was a problem hiding this comment.
First window: divider area exceeds glazed opening, and number of horizontal dividers is too large.
| " ** Severe ** ProcessSurfaceVertices: Horizontal dividers exceed glazed opening height for window FENESTRATIONSURFACE2", | ||
| " ** ~~~ ** Number of horizontal dividers=[20], divider width=[0.50] m, glazed opening height=[9.80] m.", | ||
| " ** Severe ** ProcessSurfaceVertices: Vertical dividers exceed glazed opening width for window FENESTRATIONSURFACE2", | ||
| " ** ~~~ ** Number of vertical dividers=[20], divider width=[0.50] m, glazed opening width=[9.70] m.", |
There was a problem hiding this comment.
Second window: divider area doesn't exceed glazed opening, but number of horizontal (and vertical) dividers is too large.
| " ** Severe ** ProcessSurfaceVertices: Horizontal dividers exceed glazed opening height for window FENESTRATIONSURFACE3", | ||
| " ** ~~~ ** Number of horizontal dividers=[50], divider width=[0.50] m, glazed opening height=[9.80] m.", | ||
| " ** Severe ** ProcessSurfaceVertices: Vertical dividers exceed glazed opening width for window FENESTRATIONSURFACE3", | ||
| " ** ~~~ ** Number of vertical dividers=[50], divider width=[0.50] m, glazed opening width=[9.70] m.", | ||
| " ** Severe ** ProcessSurfaceVertices: Calculated divider area <= 0.0 for window FENESTRATIONSURFACE3", | ||
| " ** ~~~ ** Window surface area=[95.06] m2, divider area=[-137.50] m2."}))); |
There was a problem hiding this comment.
Third window: divider area is negative, and number of horizontal (and vertical) dividers is too large.
mitchute
left a comment
mitchute
left a comment
There was a problem hiding this comment.
Thanks @joseph-robertson.
Pull request overview
Description of the purpose of this PR
Pull Request Author
Reviewer