Skip to content

Fix WindowProperty:FrameAndDivider convection from frame/dividers with projections - #11589

Merged
mitchute merged 14 commits into
developfrom
frame-and-divider-projections
Jun 6, 2026
Merged

mitchute merged 14 commits into
developfrom
frame-and-divider-projections

Conversation

@joseph-robertson

@joseph-robertson joseph-robertson commented May 14, 2026 •

Copy link
Copy Markdown
Collaborator

Pull request overview

image

Description of the purpose of this PR

Pull Request Author

  • Title of PR should be user-synopsis style (clearly understandable in a standalone changelog context)
  • Label the PR with at least one of: Defect, Refactoring, NewFeature, Performance, and/or DoNoPublish
  • Pull requests that impact EnergyPlus code must also include unit tests to cover enhancement or defect repair
  • Author should provide a "walkthrough" of relevant code changes using a GitHub code review comment process
  • If any diffs are expected, author must demonstrate they are justified using plots and descriptions
  • If changes fix a defect, the fix should be demonstrated in plots and descriptions
  • If any defect files are updated to a more recent version, upload new versions here or on DevSupport
  • If IDD requires transition, transition source, rules, ExpandObjects, and IDFs must be updated, and add IDDChange label
  • If structural output changes, add to output rules file and add OutputChange label
  • If adding/removing any LaTeX docs or figures, update that document's CMakeLists file dependencies
  • If adding/removing any output files (e.g., eplustbl.*)
    • Update ..\scripts\Epl-run.bat
    • Update ..\scripts\RunEPlus.bat
    • Update ..\src\EPLaunch\ MainModule.bas, epl-ui.frm, and epl.vbp (VersionComments)
    • Update ...github\workflows\energyplus.py

Reviewer

  • Perform a Code Review on GitHub
  • If branch is behind develop, merge develop and build locally to check for side effects of the merge
  • If defect, verify by running develop branch and reproducing defect, then running PR and reproducing fix
  • If feature, test running new feature, try creative ways to break it
  • CI status: all green or justified
  • Check that performance is not impacted (CI Linux results include performance check)
  • Run Unit Test(s) locally
  • Check any new function arguments for performance impacts
  • Verify IDF naming conventions and styles, memos and notes and defaults
  • If new idf included, locally check the err file and other outputs

@joseph-robertson joseph-robertson self-assigned this May 14, 2026
@joseph-robertson joseph-robertson added the Defect Includes code to repair a defect in EnergyPlus label May 14, 2026
@github-actions

Copy link
Copy Markdown

⚠️ Regressions detected on ubuntu-24.04 for commit 55f8b48

Regression Summary
  • EIO: 4
  • ESO Big Diffs: 4
  • SSZ Big Diffs: 2
  • ZSZ Big Diffs: 4
  • MTR Big Diffs: 2
  • SSZ Small Diffs: 2
  • Table Big Diffs: 2
  • Table String Diffs: 2

@github-actions

Copy link
Copy Markdown

⚠️ Regressions detected on macos-14 for commit 55f8b48

Regression Summary
  • EIO: 4
  • ESO Big Diffs: 4
  • SSZ Big Diffs: 2
  • ZSZ Big Diffs: 4
  • MTR Big Diffs: 2
  • SSZ Small Diffs: 2
  • Table Big Diffs: 2
  • Table String Diffs: 1

@github-actions

Copy link
Copy Markdown

⚠️ Regressions detected on ubuntu-24.04 for commit f24587b

Regression Summary
  • EIO: 4
  • ESO Big Diffs: 4
  • SSZ Big Diffs: 2
  • ZSZ Big Diffs: 4
  • MTR Big Diffs: 2
  • SSZ Small Diffs: 2
  • Table Big Diffs: 2
  • Table String Diffs: 2

@github-actions

Copy link
Copy Markdown

⚠️ Regressions detected on macos-14 for commit f24587b

Regression Summary
  • EIO: 4
  • ESO Big Diffs: 4
  • SSZ Big Diffs: 2
  • ZSZ Big Diffs: 4
  • MTR Big Diffs: 2
  • SSZ Small Diffs: 2
  • Table Big Diffs: 2
  • Table String Diffs: 1

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

Comment thread src/EnergyPlus/SurfaceGeometry.cc Outdated
Comment thread src/EnergyPlus/SurfaceGeometry.cc Outdated
Comment thread src/EnergyPlus/HeatBalanceManager.cc
@github-actions

Copy link
Copy Markdown

⚠️ Regressions detected on ubuntu-24.04 for commit 15b7d59

Regression Summary
  • EIO: 4
  • ESO Big Diffs: 4
  • SSZ Big Diffs: 2
  • ZSZ Big Diffs: 4
  • MTR Big Diffs: 2
  • SSZ Small Diffs: 2
  • Table Big Diffs: 2
  • Table String Diffs: 2

@github-actions

Copy link
Copy Markdown

⚠️ Regressions detected on macos-14 for commit 15b7d59

Regression Summary
  • EIO: 4
  • ESO Big Diffs: 4
  • SSZ Big Diffs: 2
  • ZSZ Big Diffs: 4
  • MTR Big Diffs: 2
  • SSZ Small Diffs: 2
  • Table Big Diffs: 2
  • Table String Diffs: 1

@github-actions

Copy link
Copy Markdown

⚠️ Regressions detected on ubuntu-24.04 for commit de74ce9

Regression Summary
  • EIO: 4
  • ESO Big Diffs: 4
  • SSZ Big Diffs: 2
  • ZSZ Big Diffs: 4
  • MTR Big Diffs: 2
  • SSZ Small Diffs: 2
  • Table Big Diffs: 2
  • Table String Diffs: 2

@github-actions

Copy link
Copy Markdown

⚠️ Regressions detected on macos-14 for commit de74ce9

Regression Summary
  • EIO: 4
  • ESO Big Diffs: 4
  • SSZ Big Diffs: 2
  • ZSZ Big Diffs: 4
  • MTR Big Diffs: 2
  • SSZ Small Diffs: 2
  • Table Big Diffs: 2
  • Table String Diffs: 1

@joseph-robertson
joseph-robertson marked this pull request as ready for review May 20, 2026 23:00
@github-actions

Copy link
Copy Markdown

⚠️ Regressions detected on ubuntu-24.04 for commit d6673a0

Regression Summary
  • EIO: 4
  • ESO Big Diffs: 4
  • SSZ Big Diffs: 2
  • ZSZ Big Diffs: 4
  • MTR Big Diffs: 2
  • SSZ Small Diffs: 2
  • Table Big Diffs: 2
  • Table String Diffs: 2

@github-actions

Copy link
Copy Markdown

⚠️ Regressions detected on macos-14 for commit d6673a0

Regression Summary
  • EIO: 4
  • ESO Big Diffs: 4
  • SSZ Big Diffs: 2
  • ZSZ Big Diffs: 4
  • MTR Big Diffs: 2
  • SSZ Small Diffs: 2
  • Table Big Diffs: 2
  • Table String Diffs: 1

@joseph-robertson
joseph-robertson requested a review from mitchute May 22, 2026 15:12
@mitchute

mitchute commented Jun 5, 2026

Copy link
Copy Markdown
Collaborator

@joseph-robertson it looks like if there's a bad frame geometry it isn't being caught here, which can result in bad data propagating downstream and cause a fatal error. I added the following bad object

 WindowProperty:FrameAndDivider,
    BadDividerGeometry,      !- Name
    0.05,                    !- Frame Width {m}
    0.05,                    !- Frame Outside Projection {m}
    0.05,                    !- Frame Inside Projection {m}
    5.0,                     !- Frame Conductance {W/m2-K}
    1.2,                     !- Ratio of Frame-Edge Glass Conductance to Center-Of-Glass Conductance
    0.8,                     !- Frame Solar Absorptance
    0.8,                     !- Frame Visible Absorptance
    0.9,                     !- Frame Thermal Hemispherical Emissivity
    DividedLite,             !- Divider Type
    0.5,                     !- Divider Width {m}
    20,                      !- Number of Horizontal Dividers
    20,                      !- Number of Vertical Dividers
    0.02,                    !- Divider Outside Projection {m}
    0.02,                    !- Divider Inside Projection {m}
    5.0,                     !- Divider Conductance {W/m2-K}
    1.2,                     !- Ratio of Divider-Edge Glass Conductance to Center-Of-Glass Conductance
    0.8,                     !- Divider Solar Absorptance
    0.8,                     !- Divider Visible Absorptance
    0.9,                     !- Divider Thermal Hemispherical Emissivity
    ,                        !- Outside Reveal Solar Absorptance
    ,                        !- Inside Sill Depth {m}
    ,                        !- Inside Sill Solar Absorptance
    ,                        !- Inside Reveal Depth {m}
    ,                        !- Inside Reveal Solar Absorptance
    ;                        !- NFRC Product Type for Assembly Calculations

into HybridVentilationControl.idf and referenced it from the Zn001:Wall001:Win001 FenestrationDetailed:Surface object and got the following failure.

   ** Warning ** ProcessSurfaceVertices: Calculated Divider Area <= 0.0 for Window=ZN001:WALL001:WIN001

 < file contents omitted >

   ** Severe  ** DualSetPointWithDeadBand: Unanticipated combination of heating and cooling loads - report to EnergyPlus Development Team
   **   ~~~   ** occurs in Zone=WEST ZONE During Warmup, Environment=CHICAGO_IL_USA ANNUAL HEATING 99% DESIGN CONDITIONS DB, at Simulation time=01/21 00:00 - 00:10
   **   ~~~   ** LoadToHeatingSetPoint=-N0AN, LoadToCoolingSetPoint=-N0AN
   **   ~~~   ** Zone Heating Set-point=15.00
   **   ~~~   ** Zone Cooling Set-point=30.00
   **   ~~~   ** Zone TempDepZnLd=546.95
   **   ~~~   ** Zone TempIndZnLd=-N0AN
   **   ~~~   ** Zone ThermostatSetPoint=0.00
   **  Fatal  ** Program terminates due to above conditions.

I think we need to capture and handle this condition rather than just issuing a message and moving on.

if (DivArea <= 0.0) {

This is out of scope on this current issue, so let's handle it separately.

@mitchute

mitchute commented Jun 5, 2026

Copy link
Copy Markdown
Collaborator

I'll hold on to this until you can take a look. If you'd rather address that here, that's fine, too.

Comment on lines +13068 to +13094
if ((FrWidth > 0.0) && (FrArea > 0.0) && !ErrorInSurface) {
// Correction factor for portion of frame subject to frame projection correction
state.dataSurface->SurfWinProjCorrFrOut(ThisSurf) =
(state.dataSurface->FrameDivider(FrDivNum).FrameProjectionOut / FrWidth) *
(ThisHeight + ThisWidth -
(state.dataSurface->FrameDivider(FrDivNum).HorDividers + state.dataSurface->FrameDivider(FrDivNum).VertDividers) *
DivWidth) /
(ThisHeight + ThisWidth + 2 * FrWidth);
state.dataSurface->SurfWinProjCorrFrIn(ThisSurf) =
(state.dataSurface->FrameDivider(FrDivNum).FrameProjectionIn / FrWidth) *
(ThisHeight + ThisWidth -
(state.dataSurface->FrameDivider(FrDivNum).HorDividers + state.dataSurface->FrameDivider(FrDivNum).VertDividers) *
DivWidth) /
(ThisHeight + ThisWidth + 2 * FrWidth);
}

if ((DivWidth > 0.0) && (DivArea > 0.0) && !ErrorInSurface) {
auto &surfWin = state.dataSurface->SurfaceWindow(ThisSurf);
surfWin.glazedFrac = surf.Area / (surf.Area + state.dataSurface->SurfWinDividerArea(ThisSurf));
// Correction factor for portion of divider subject to divider projection correction
DivFrac = (1.0 - state.dataSurface->FrameDivider(FrDivNum).HorDividers *
state.dataSurface->FrameDivider(FrDivNum).VertDividers * pow_2(DivWidth) / DivArea);
state.dataSurface->SurfWinProjCorrDivOut(ThisSurf) =
DivFrac * state.dataSurface->FrameDivider(FrDivNum).DividerProjectionOut / DivWidth;
state.dataSurface->SurfWinProjCorrDivIn(ThisSurf) =
DivFrac * state.dataSurface->FrameDivider(FrDivNum).DividerProjectionIn / DivWidth;
}

@joseph-robertson joseph-robertson May 20, 2026 •

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pulling these out from under the DivArea > 0 block. The top block is the core fix in this PR -- we want to still calculate the frame projection correction factor even when there are no dividers.

Comment thread src/EnergyPlus/SurfaceGeometry.cc Outdated
state.dataSurface->FrameDivider(FrDivNum).VertDividers * DivWidth);
state.dataSurface->SurfWinDividerArea(ThisSurf) = DivArea * surf.Multiplier;
if ((surf.Area - state.dataSurface->SurfWinDividerArea(ThisSurf)) <= 0.0) {
if (state.dataSurface->SurfWinDividerArea(ThisSurf) <= 0.0) {

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change allows the severe error to actually be thrown. We want to throw the error when calculated divider area is negative. Previously, if this value was negative, glazed opening minus divider area was a positive value.

}
surf.Area -= state.dataSurface->SurfWinDividerArea(ThisSurf); // Glazed area
if (DivArea <= 0.0) {
ShowWarningError(state, std::format("{}Calculated Divider Area <= 0.0 for Window={}", RoutineName, surf.Name));

@joseph-robertson joseph-robertson Jun 5, 2026 •

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe we don't need this warning if the other related severe error is activated.

@github-actions

github-actions Bot commented Jun 5, 2026

Copy link
Copy Markdown

⚠️ Regressions detected on ubuntu-24.04 for commit d9706c7

Regression Summary
  • EIO: 4
  • ESO Big Diffs: 4
  • SSZ Big Diffs: 2
  • ZSZ Big Diffs: 4
  • MTR Big Diffs: 2
  • SSZ Small Diffs: 2
  • Table Big Diffs: 2
  • Table String Diffs: 2

@github-actions

github-actions Bot commented Jun 5, 2026

Copy link
Copy Markdown

⚠️ Regressions detected on macos-14 for commit d9706c7

Regression Summary
  • EIO: 4
  • ESO Big Diffs: 4
  • SSZ Big Diffs: 2
  • ZSZ Big Diffs: 4
  • MTR Big Diffs: 2
  • SSZ Small Diffs: 2
  • Table Big Diffs: 2
  • Table String Diffs: 1

@joseph-robertson

joseph-robertson commented Jun 5, 2026 •

Copy link
Copy Markdown
Collaborator Author

Regressions are related to files having the WindowProperty:FrameAndDivider object with frame outside/inside projections but no dividers. For CmplxGlz_Daylighting_SouthVB45deg.idf, for example, the change in this PR causes total energy use to increase slightly (cooling down, heating up).

@joseph-robertson

Copy link
Copy Markdown
Collaborator Author

@joseph-robertson it looks like if there's a bad frame geometry it isn't being caught here, which can result in bad data propagating downstream and cause a fatal error. I added the following bad object

 WindowProperty:FrameAndDivider,
    BadDividerGeometry,      !- Name
    0.05,                    !- Frame Width {m}
    0.05,                    !- Frame Outside Projection {m}
    0.05,                    !- Frame Inside Projection {m}
    5.0,                     !- Frame Conductance {W/m2-K}
    1.2,                     !- Ratio of Frame-Edge Glass Conductance to Center-Of-Glass Conductance
    0.8,                     !- Frame Solar Absorptance
    0.8,                     !- Frame Visible Absorptance
    0.9,                     !- Frame Thermal Hemispherical Emissivity
    DividedLite,             !- Divider Type
    0.5,                     !- Divider Width {m}
    20,                      !- Number of Horizontal Dividers
    20,                      !- Number of Vertical Dividers
    0.02,                    !- Divider Outside Projection {m}
    0.02,                    !- Divider Inside Projection {m}
    5.0,                     !- Divider Conductance {W/m2-K}
    1.2,                     !- Ratio of Divider-Edge Glass Conductance to Center-Of-Glass Conductance
    0.8,                     !- Divider Solar Absorptance
    0.8,                     !- Divider Visible Absorptance
    0.9,                     !- Divider Thermal Hemispherical Emissivity
    ,                        !- Outside Reveal Solar Absorptance
    ,                        !- Inside Sill Depth {m}
    ,                        !- Inside Sill Solar Absorptance
    ,                        !- Inside Reveal Depth {m}
    ,                        !- Inside Reveal Solar Absorptance
    ;                        !- NFRC Product Type for Assembly Calculations

into HybridVentilationControl.idf and referenced it from the Zn001:Wall001:Win001 FenestrationDetailed:Surface object and got the following failure.

   ** Warning ** ProcessSurfaceVertices: Calculated Divider Area <= 0.0 for Window=ZN001:WALL001:WIN001

 < file contents omitted >

   ** Severe  ** DualSetPointWithDeadBand: Unanticipated combination of heating and cooling loads - report to EnergyPlus Development Team
   **   ~~~   ** occurs in Zone=WEST ZONE During Warmup, Environment=CHICAGO_IL_USA ANNUAL HEATING 99% DESIGN CONDITIONS DB, at Simulation time=01/21 00:00 - 00:10
   **   ~~~   ** LoadToHeatingSetPoint=-N0AN, LoadToCoolingSetPoint=-N0AN
   **   ~~~   ** Zone Heating Set-point=15.00
   **   ~~~   ** Zone Cooling Set-point=30.00
   **   ~~~   ** Zone TempDepZnLd=546.95
   **   ~~~   ** Zone TempIndZnLd=-N0AN
   **   ~~~   ** Zone ThermostatSetPoint=0.00
   **  Fatal  ** Program terminates due to above conditions.

I think we need to capture and handle this condition rather than just issuing a message and moving on.

if (DivArea <= 0.0) {

This is out of scope on this current issue, so let's handle it separately.

Addressed in 88c48fa.

@github-actions

github-actions Bot commented Jun 5, 2026

Copy link
Copy Markdown

⚠️ Regressions detected on macos-14 for commit 3e388cc

Regression Summary
  • EIO: 4
  • ESO Big Diffs: 4
  • SSZ Big Diffs: 2
  • ZSZ Big Diffs: 4
  • MTR Big Diffs: 2
  • SSZ Small Diffs: 2
  • Table Big Diffs: 2
  • Table String Diffs: 1

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

Comment thread tst/EnergyPlus/unit/SurfaceGeometry.unit.cc Outdated
Comment thread tst/EnergyPlus/unit/SurfaceGeometry.unit.cc Outdated
Comment thread src/EnergyPlus/SurfaceGeometry.cc Outdated
state.dataSurface->FrameDivider(FrDivNum).VertDividers * DivWidth);
state.dataSurface->SurfWinDividerArea(ThisSurf) = DivArea * surf.Multiplier;
if ((surf.Area - state.dataSurface->SurfWinDividerArea(ThisSurf)) <= 0.0) {
if (state.dataSurface->SurfWinDividerArea(ThisSurf) <= 0.0) {
@github-actions

github-actions Bot commented Jun 5, 2026

Copy link
Copy Markdown

⚠️ Regressions detected on ubuntu-24.04 for commit 3e388cc

Regression Summary
  • EIO: 4
  • ESO Big Diffs: 4
  • SSZ Big Diffs: 2
  • ZSZ Big Diffs: 4
  • MTR Big Diffs: 2
  • SSZ Small Diffs: 2
  • Table Big Diffs: 2
  • Table String Diffs: 2

@github-actions

github-actions Bot commented Jun 5, 2026

Copy link
Copy Markdown

⚠️ Regressions detected on macos-14 for commit 362d9a4

Regression Summary
  • EIO: 4
  • ESO Big Diffs: 4
  • SSZ Big Diffs: 2
  • ZSZ Big Diffs: 4
  • MTR Big Diffs: 2
  • SSZ Small Diffs: 2
  • Table Big Diffs: 2
  • Table String Diffs: 1

@github-actions

github-actions Bot commented Jun 5, 2026

Copy link
Copy Markdown

⚠️ Regressions detected on ubuntu-24.04 for commit 362d9a4

Regression Summary
  • EIO: 4
  • ESO Big Diffs: 4
  • SSZ Big Diffs: 2
  • ZSZ Big Diffs: 4
  • MTR Big Diffs: 2
  • SSZ Small Diffs: 2
  • Table Big Diffs: 2
  • Table String Diffs: 2

@github-actions

github-actions Bot commented Jun 6, 2026

Copy link
Copy Markdown

⚠️ Regressions detected on macos-14 for commit 6007920

Regression Summary
  • EIO: 4
  • ESO Big Diffs: 4
  • SSZ Big Diffs: 2
  • ZSZ Big Diffs: 4
  • MTR Big Diffs: 2
  • SSZ Small Diffs: 2
  • Table Big Diffs: 2
  • Table String Diffs: 1

@github-actions

github-actions Bot commented Jun 6, 2026

Copy link
Copy Markdown

⚠️ Regressions detected on ubuntu-24.04 for commit 6007920

Regression Summary
  • EIO: 4
  • ESO Big Diffs: 4
  • SSZ Big Diffs: 2
  • ZSZ Big Diffs: 4
  • MTR Big Diffs: 2
  • SSZ Small Diffs: 2
  • Table Big Diffs: 2
  • Table String Diffs: 2

@mitchute mitchute left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @joseph-robertson. I think we're all good on this now. Merging.

@mitchute
mitchute merged commit 50cbd7f into develop Jun 6, 2026
11 checks passed
@mitchute
mitchute deleted the frame-and-divider-projections branch June 6, 2026 20:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Defect Includes code to repair a defect in EnergyPlus

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants