Adjust SurfaceProperty:XXX IDD and fix/improve sunlit fraction schedule checks by joseph-robertson · Pull Request #11445 · NatLabRockies/EnergyPlus

joseph-robertson · 2026-03-03T17:37:12Z

Pull request overview

Fixes SurfaceProperty:LocalEnvironment and SurfaceProperty:SurroundingSurfaces: IDD adjustments #9275.

Description of the purpose of this PR

Pull Request Author

Reviewer

…rty:SurroundingSurfaces.

…e has sunlit fraction schedule.

joseph-robertson · 2026-03-03T17:40:07Z

src/EnergyPlus/HeatBalanceManager.cc

        SurfaceGeometry::SetupZoneGeometry(state, ErrorsFound);
+
+        SolarShading::GetShadowingInput(state);


This PR inadvertently moved GetShadowingInput before SetupZoneGeometry. But we need it after so that surface data is read first. The existing relevant unit test didn't catch this because of this reason.

@RKStrand Are you OK with this change? I believe the only regression implication is to change the order of a few lines in the EIO file.

@joseph-robertson Um, yeah, as long as it doesn't trip any weird things in the unit tests, I'm guessing it will be okay. I seem to vaguely remember some recent work here and there were some issues with order dependence on some of the calls in this area of the code. I think I couldn't move something up as high as I thought because it caused other crashes because stuff wasn't read in yet. Anyway, if you move it an there are no unit test or test suite issues that pop up, then it's probably fine.

joseph-robertson · 2026-03-03T17:42:36Z

src/EnergyPlus/SolarShading.cc

+    }
+}
+
+void checkNotScheduledSurfacePresent(EnergyPlusData &state)


New method for issuing a warning when a surface has a sunlit fraction schedule but the ShadowCalculation object has Shading Calculation Method != Scheduled. This is the contrapositive (?!) to checkScheduledSurfacePresent.

joseph-robertson · 2026-03-03T17:44:11Z

tst/EnergyPlus/unit/SolarShading.unit.cc

+    surfData->Surface(6).Class = SurfaceClass::Window;
+    surfData->Surface(6).SurfSchedExternalShadingFrac = false;
+    surfData->Surface(6).Name = "WINDOW2NOTOK";


Just improving the existing unit test -- we should see an additional warning line for a second valid surface without sunlit fraction schedule.

joseph-robertson · 2026-03-03T17:45:00Z

tst/EnergyPlus/unit/SolarShading.unit.cc

+    EXPECT_TRUE(compare_err_stream(error_string, true));
+}
+
+TEST_F(EnergyPlusFixture, SolarShadingTest_checkNotScheduledSurfacePresent)


New unit test, much like the existing unit test, for checking the new checkNotScheduledSurfacePresent method.

github-actions · 2026-03-03T18:05:13Z

⚠️ Regressions detected on ubuntu-24.04 for commit `1496723`

Regression Summary

EIO: 811

github-actions · 2026-03-03T18:13:45Z

⚠️ Regressions detected on macos-14 for commit `1496723`

Regression Summary

EIO: 807

github-actions · 2026-03-03T18:25:39Z

⚠️ Regressions detected on ubuntu-24.04 for commit `f707897`

Regression Summary

EIO: 811

github-actions · 2026-03-03T18:30:58Z

⚠️ Regressions detected on macos-14 for commit `f707897`

Regression Summary

EIO: 807

github-actions · 2026-03-03T19:59:55Z

⚠️ Regressions detected on ubuntu-24.04 for commit `8bba3d3`

Regression Summary

EIO: 811

joseph-robertson · 2026-03-03T23:12:10Z

There are technically IDD changes in this PR; however, they don't involve adding/removing/renaming any fields. Would it therefore be OK to merge this to be included in v26.1.0, but post IOFreeze?

@mitchute @rraustad @RKStrand

mitchute · 2026-03-03T23:44:11Z

I think the \object-list change is likely to only affect IDF Editor. The others are less clear to me, but even so, seem like they may be OK to admit at this point.

rraustad · 2026-03-04T14:58:57Z

idd/Energy+.idd.in

   A2, \field Exterior Surface Name
       \type object-list
-       \object-list SurfaceNames
+       \object-list AllHeatTranSurfNames


I assume this change was made to include FenestrationSurface:Detailed in the list of object names in the IDF Editor. However, AllHeatTranSurfNames (and SurfaceNames) also includes objects that should not show up in that list provided by the IDF Editor. It may also be that the IO Ref description below does not include all valid surface types but I did not check that. The fix would be to add a new reference to the valid objects and use that here instead?

1.11.28 SurfaceProperty:LocalEnvironment The object links to an exterior surface object BuildingSurface:Detailed or an exterior fenestration object FenestrationSurface:Detailed and is used when ... FenestrationSurface:Detailed, \memo Allows for detailed entry of subsurfaces \memo (windows, doors, glass doors, tubular daylighting devices). \min-fields 18 \format vertices A1 , \field Name \required-field \type alpha \reference SubSurfNames \reference GlazedExtSubSurfNames \reference SurfAndSubSurfNames \reference AllHeatTranSurfNames

For example, what good would it do to use SurfaceProperty:LocalEnvironment with an adiabatic floor? Maybe I am overthinking this and this change is sufficient to at least include the relevant objects.

Floor:Adiabatic, \memo Allows for simplified entry of exterior floors \memo ignoring ground contact or interior floors. \memo View Factor to Ground is automatically calculated. A1 , \field Name \required-field \type alpha \reference SurfaceNames \reference SurfAndSubSurfNames \reference AllHeatTranSurfNames

Good point. For example currently if you open SolarShadingTest_ExternalFraction.idf in IDF Editor, the list of SurfaceProperty:LocalEnvironment objects will initially populate the Exterior Surface Name fields with:

Shading:Site:Detailed (invalid)

FenestrationSurface:Detailed (invalid)

BuildingSurface:Detailed (valid)

But the dropdowns don't show any Shading:Site:Detailed or FenestrationSurface:Detailed objects. If we switched SurfaceNames -> AllHeatTranSurfNames we'd still be including invalid Floor:Adiabatic in the dropdowns. So assuming the docs are correct, we'd need a reference that is shared between only FenestrationSurface:Detailed and BuildingSurface:Detailed; if one doesn't already exist, we need to add a new one.

Be careful when thinking the docs are correct. I think as a user I would expect Wall:Detailed to be the same as BuildingSurface:Detailed in the context of SurfaceProperty:LocalEnvironment. Even an exterior door should be able to use the local environment properties? I am sure you can think of other use cases for this object.

See 1886efe. Indeed Wall:Detailed, e.g., is a valid object (I checked locally). Based on the code, shading objects are certainly invalid. So I propose that we stick with the \object-list AllHeatTranSurfNames change (to pick up fenestration), but not get overly ambitious with a new reference as we may accidentally exclude a valid object type.

… exterior surface and fenestration objects.

…g objects in SolarShadingTest_ExternalFraction.idf.

github-actions · 2026-03-04T18:21:41Z

⚠️ Regressions detected on ubuntu-24.04 for commit `5b18bce`

Regression Summary

EIO: 811
Audit: 1
Table Big Diffs: 1

github-actions · 2026-03-04T18:22:06Z

⚠️ Regressions detected on macos-14 for commit `5b18bce`

Regression Summary

EIO: 807
Audit: 1
Table Big Diffs: 1

rraustad · 2026-03-04T18:52:43Z

testfiles/SolarShadingTest_ExternalFraction.idf

@@ -670,215 +670,6 @@
    ,                        !- Sky Diffuse Modeling Algorithm
    Yes;                     !- Output External Shading Calculation Results



@joseph-robertson I thought I saw a comment of why these were removed but can't find that now. Could you elaborate why these would need to be removed.

The SurfaceProperty:LocalEnvironment objects referencing shading objects don't need to be removed; but I think they can safely be removed since they have no impact on the model.

Got it. Thanks.

github-actions · 2026-03-06T20:05:50Z

⚠️ Regressions detected on macos-14 for commit `ac2f517`

Regression Summary

EIO: 807
Audit: 1
Table Big Diffs: 1

joseph-robertson added 4 commits March 3, 2026 10:32

IDD adjustments for SurfaceProperty:LocalEnvironment and SurfacePrope…

1840310

…rty:SurroundingSurfaces.

Move GetShadowingInput down below SetupZoneGeometry in GetBuildingData.

f889c1a

Add method for checking Shading Calc Method not Scheduled when surfac…

0d6bb5f

…e has sunlit fraction schedule.

Improve and add unit tests for solar shading.

6a77e91

joseph-robertson self-assigned this Mar 3, 2026

joseph-robertson added the Defect Includes code to repair a defect in EnergyPlus label Mar 3, 2026

joseph-robertson commented Mar 3, 2026

View reviewed changes

joseph-robertson added 2 commits March 3, 2026 10:56

Formatting.

99ae2e9

Formatting. Again.

1b6a01b

Update unit test expected values related to autocalculatable defaults.

e3da607

rraustad reviewed Mar 4, 2026

View reviewed changes

joseph-robertson added 3 commits March 4, 2026 10:51

Update SurfaceProperty:LocalEnvironment docs to generalize acceptable…

1886efe

… exterior surface and fenestration objects.

Remove SurfaceProperty:LocalEnvironment objects that reference shadin…

c8e2d9d

…g objects in SolarShadingTest_ExternalFraction.idf.

Merge branch 'develop' into surfprop-idd

5527d24

rraustad reviewed Mar 4, 2026

View reviewed changes

Merge branch 'develop' into surfprop-idd

1d01bae

joseph-robertson marked this pull request as ready for review March 6, 2026 16:19

mitchute added 2 commits March 6, 2026 11:50

Merge branch 'develop' into surfprop-idd

937209b

fixup new format messages

e3475b7

mitchute added this to the EnergyPlus 26.2 IOFreeze milestone Mar 10, 2026

		SurfaceGeometry::SetupZoneGeometry(state, ErrorsFound);

		SolarShading::GetShadowingInput(state);

		@@ -670,215 +670,6 @@
		, !- Sky Diffuse Modeling Algorithm
		Yes; !- Output External Shading Calculation Results

Conversation

joseph-robertson commented Mar 3, 2026

Pull request overview

Description of the purpose of this PR

Pull Request Author

Reviewer

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Mar 3, 2026

⚠️ Regressions detected on ubuntu-24.04 for commit 1496723

Uh oh!

github-actions bot commented Mar 3, 2026

⚠️ Regressions detected on macos-14 for commit 1496723

Uh oh!

github-actions bot commented Mar 3, 2026

⚠️ Regressions detected on ubuntu-24.04 for commit f707897

Uh oh!

github-actions bot commented Mar 3, 2026

⚠️ Regressions detected on macos-14 for commit f707897

Uh oh!

github-actions bot commented Mar 3, 2026

⚠️ Regressions detected on ubuntu-24.04 for commit 8bba3d3

Uh oh!

joseph-robertson commented Mar 3, 2026

Uh oh!

mitchute commented Mar 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

joseph-robertson Mar 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Mar 4, 2026

⚠️ Regressions detected on ubuntu-24.04 for commit 5b18bce

Uh oh!

github-actions bot commented Mar 4, 2026

⚠️ Regressions detected on macos-14 for commit 5b18bce

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Mar 6, 2026

⚠️ Regressions detected on macos-14 for commit ac2f517

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

⚠️ Regressions detected on ubuntu-24.04 for commit `1496723`

⚠️ Regressions detected on macos-14 for commit `1496723`

⚠️ Regressions detected on ubuntu-24.04 for commit `f707897`

⚠️ Regressions detected on macos-14 for commit `f707897`

⚠️ Regressions detected on ubuntu-24.04 for commit `8bba3d3`

mitchute commented Mar 3, 2026 •

edited

Loading

joseph-robertson Mar 4, 2026 •

edited

Loading

⚠️ Regressions detected on ubuntu-24.04 for commit `5b18bce`

⚠️ Regressions detected on macos-14 for commit `5b18bce`

⚠️ Regressions detected on macos-14 for commit `ac2f517`