Skip to content

Adjust SurfaceProperty:XXX IDD and fix/improve sunlit fraction schedule checks #11445

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
joseph-robertson wants to merge 14 commits into
base: develop
Choose a base branch
from
+93 −224
Open

Adjust SurfaceProperty:XXX IDD and fix/improve sunlit fraction schedule checks #11445

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
1840310
IDD adjustments for SurfaceProperty:LocalEnvironment and SurfacePrope…
joseph-robertson Mar 3, 2026
f889c1a
Move GetShadowingInput down below SetupZoneGeometry in GetBuildingData.
joseph-robertson Mar 3, 2026
0d6bb5f
Add method for checking Shading Calc Method not Scheduled when surfac…
joseph-robertson Mar 3, 2026
6a77e91
Improve and add unit tests for solar shading.
joseph-robertson Mar 3, 2026
99ae2e9
Formatting.
joseph-robertson Mar 3, 2026
1b6a01b
Formatting. Again.
joseph-robertson Mar 3, 2026
e3da607
Update unit test expected values related to autocalculatable defaults.
joseph-robertson Mar 3, 2026
1886efe
Update SurfaceProperty:LocalEnvironment docs to generalize acceptable…
joseph-robertson Mar 4, 2026
c8e2d9d
Remove SurfaceProperty:LocalEnvironment objects that reference shadin…
joseph-robertson Mar 4, 2026
5527d24
Merge branch 'develop' into surfprop-idd
joseph-robertson Mar 4, 2026
1d01bae
Merge branch 'develop' into surfprop-idd
joseph-robertson Mar 5, 2026
937209b
Merge branch 'develop' into surfprop-idd
mitchute Mar 6, 2026
e3475b7
fixup new format messages
mitchute Mar 6, 2026
929b663
Merge branch 'develop' into surfprop-idd
joseph-robertson Mar 12, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions idd/Energy+.idd.in
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18873,7 +18873,7 @@ SurfaceProperty:LocalEnvironment,
\reference SurfaceLocalEnvironmentNames
A2, \field Exterior Surface Name
\type object-list
\object-list SurfaceNames
\object-list AllHeatTranSurfNames
Copy link
Collaborator

@rraustad rraustad Mar 4, 2026

Choose a reason for hiding this comment

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

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

Copy link
Collaborator Author

@joseph-robertson joseph-robertson Mar 4, 2026

Choose a reason for hiding this comment

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

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.

Copy link
Collaborator

@rraustad rraustad Mar 4, 2026

Choose a reason for hiding this comment

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

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.

Copy link
Collaborator Author

@joseph-robertson joseph-robertson Mar 4, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

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.

\note Enter the name of an exterior surface object
A3, \field Sunlit Fraction Schedule Name
\type object-list
Expand Down Expand Up @@ -18920,7 +18920,8 @@ SurfaceProperty:SurroundingSurfaces,
N1, \field Sky View Factor
\minimum 0.0
\maximum 1.0
\default 0.5
\autocalculatable
\default autocalculate
\note optional
A2, \field Sky Temperature Schedule Name
\type object-list
Expand All @@ -18930,7 +18931,8 @@ SurfaceProperty:SurroundingSurfaces,
N2, \field Ground View Factor
\minimum 0.0
\maximum 1.0
\default 0.5
\autocalculatable
\default autocalculate
\note optional
A3, \field Ground Temperature Schedule Name
\type object-list
Expand Down
4 changes: 2 additions & 2 deletions src/EnergyPlus/HeatBalanceManager.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1809,11 +1809,11 @@ namespace HeatBalanceManager {
// METHODOLOGY EMPLOYED:
// The GetObjectItem routines are employed to retrieve the data.

SolarShading::GetShadowingInput(state);

GetZoneData(state, ErrorsFound); // Read Zone data from input file

SurfaceGeometry::SetupZoneGeometry(state, ErrorsFound);

SolarShading::GetShadowingInput(state);
Comment on lines 1817 to +1819
Copy link
Collaborator Author

@joseph-robertson joseph-robertson Mar 3, 2026

Choose a reason for hiding this comment

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

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.

Copy link
Collaborator Author

@joseph-robertson joseph-robertson Mar 3, 2026

Choose a reason for hiding this comment

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

@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.

Copy link
Contributor

@RKStrand RKStrand Mar 4, 2026

Choose a reason for hiding this comment

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

@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.

}

void GetZoneData(EnergyPlusData &state, bool &ErrorsFound) // If errors found in input
Expand Down
42 changes: 37 additions & 5 deletions src/EnergyPlus/SolarShading.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -490,12 +490,15 @@ void GetShadowingInput(EnergyPlusData &state)
format("Value entered=\"{}\" while no Schedule:File:Shading object is defined, InternalCalculation will be used.",
state.dataIPShortCut->cAlphaArgs(aNum)));
}
checkNotScheduledSurfacePresent(state);
} else if (Util::SameString(state.dataIPShortCut->cAlphaArgs(aNum), "PolygonClipping")) {
state.dataSysVars->shadingMethod = ShadingMethod::PolygonClipping;
state.dataIPShortCut->cAlphaArgs(aNum) = "PolygonClipping";
checkNotScheduledSurfacePresent(state);
} else if (Util::SameString(state.dataIPShortCut->cAlphaArgs(aNum), "PixelCounting")) {
state.dataSysVars->shadingMethod = ShadingMethod::PixelCounting;
state.dataIPShortCut->cAlphaArgs(aNum) = "PixelCounting";
checkNotScheduledSurfacePresent(state);
if (NumNumbers >= 3) {
pixelRes = (unsigned)state.dataIPShortCut->rNumericArgs(3);
}
Expand Down Expand Up @@ -526,6 +529,7 @@ void GetShadowingInput(EnergyPlusData &state)
} else {
state.dataIPShortCut->cAlphaArgs(aNum) = "PolygonClipping";
state.dataSysVars->shadingMethod = ShadingMethod::PolygonClipping;
checkNotScheduledSurfacePresent(state);
}

aNum++;
Expand Down Expand Up @@ -818,7 +822,7 @@ void processShadowingInput(EnergyPlusData &state)

void checkScheduledSurfacePresent(EnergyPlusData &state)
{
// User has chosen "Scheduled" for sunlit fraction so check to see which surfaces don't have a schedule
// User has chosen "Scheduled" for sunlit fraction so check to see which surfaces don't have a schedule.
int numNotDef = 0;
int constexpr maxErrMessages = 50;
auto &surfData = state.dataSurface;
Expand All @@ -831,16 +835,44 @@ void checkScheduledSurfacePresent(EnergyPlusData &state)
if (!thisSurf.SurfSchedExternalShadingFrac) {
numNotDef += 1;
if (numNotDef == 1) {
ShowWarningError(
state,
format("ShadowCalculation specified Schedule for the Shading Calculation Method but no schedule provided for {}", thisSurf.Name));
ShowWarningError(state,
format("ShadowCalculation specified Schedule for the Shading Calculation Method but no schedule provided for {}.",
thisSurf.Name));
ShowContinueError(
state, "When Schedule is selected for the Shading Calculation Method and no schedule is provided for a particular surface,");
ShowContinueError(
state, "EnergyPlus will assume that the surface is not shaded. Use SurfaceProperty:LocalEnvironment to specify a schedule");
ShowContinueError(state, "for sunlit fraction if this was not desired. Otherwise, this surface will not be shaded at all.");
} else if (numNotDef <= maxErrMessages) {
ShowWarningError(state, format("No schedule was provided for {} either. See above error message for more details", thisSurf.Name));
ShowWarningError(state, format("No schedule was provided for {} either. See above error message for more details.", thisSurf.Name));
}
}
}
if (numNotDef > maxErrMessages) {
ShowContinueError(state, format("This message is only shown for the first {} occurrences of this issue.", maxErrMessages));
}
}

void checkNotScheduledSurfacePresent(EnergyPlusData &state)
Copy link
Collaborator Author

@joseph-robertson joseph-robertson Mar 3, 2026

Choose a reason for hiding this comment

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

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.

{
// User has *not* chosen "Scheduled" for sunlit fraction so check to see which surfaces *have* a schedule.
int numNotDef = 0;
int constexpr maxErrMessages = 50;
auto &surfData = state.dataSurface;
for (int surfNum = 1; surfNum <= surfData->TotSurfaces; ++surfNum) {
auto &thisSurf = surfData->Surface(surfNum);
if ((thisSurf.Class == SurfaceClass::Shading || thisSurf.Class == SurfaceClass::Detached_F || thisSurf.Class == SurfaceClass::Detached_B ||
thisSurf.Class == SurfaceClass::Overhang || thisSurf.Class == SurfaceClass::Fin)) {
continue; // skip shading surfaces
}
if (thisSurf.SurfSchedExternalShadingFrac) {
numNotDef += 1;
if (numNotDef == 1) {
ShowWarningError(state,
format("ShadowCalculation did not specify Schedule for the Shading Calculation Method but schedule provided for {}.",
thisSurf.Name));
} else if (numNotDef <= maxErrMessages) {
ShowWarningError(state, format("Schedule was also provided for {}. See above error message for more details.", thisSurf.Name));
}
}
}
Expand Down
2 changes: 2 additions & 0 deletions src/EnergyPlus/SolarShading.hh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,8 @@ namespace SolarShading {

void checkScheduledSurfacePresent(EnergyPlusData &state);

void checkNotScheduledSurfacePresent(EnergyPlusData &state);

void AllocateModuleArrays(EnergyPlusData &state);

void AnisoSkyViewFactors(EnergyPlusData &state);
Expand Down
8 changes: 4 additions & 4 deletions tst/EnergyPlus/unit/InputProcessor.unit.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4365,9 +4365,9 @@ TEST_F(InputProcessorFixture, reportIDFRecordsStats_extensible_fields)
// 1 fields defaulted , 0 Autosizable, 0 Autocalculatable
"SurfaceProperty:SurroundingSurfaces,",
" SrdSurfs:Living:East, !- Name",
" 0.3, !- Sky View Factor", // Has numeric default
" 0.3, !- Sky View Factor", // Has autocalculate default
" , !- Sky Temperature Schedule Name",
" 0.1, !- Ground View Factor", // Has numeric default
" 0.1, !- Ground View Factor", // Has autocalculate default
" , !- Ground Temperature Schedule Name",
" SurroundingSurface1, !- Surrounding Surface 1 Name", // (begin extensible)
" 0.6, !- Surrounding Surface 1 View Factor", // Has numeric default
Expand All @@ -4386,13 +4386,13 @@ TEST_F(InputProcessorFixture, reportIDFRecordsStats_extensible_fields)
state->dataInputProcessing->inputProcessor->reportIDFRecordsStats(*state);

// TOTAL:
// 16 fields with defaults, 0 Autosizable, 0 Autocalculatable
// 16 fields with defaults, 0 Autosizable, 2 Autocalculatable
// 2 fields defaulted , 0 Autosized , 0 Autocalculated

EXPECT_EQ(5, state->dataOutput->iNumberOfRecords); // Number of IDF Records (=Objects)
EXPECT_EQ(16, state->dataOutput->iTotalFieldsWithDefaults); // Total number of fields that could be defaulted
EXPECT_EQ(0, state->dataOutput->iTotalAutoSizableFields); // Total number of autosizeable fields
EXPECT_EQ(0, state->dataOutput->iTotalAutoCalculatableFields); // Total number of autocalculatable fields
EXPECT_EQ(2, state->dataOutput->iTotalAutoCalculatableFields); // Total number of autocalculatable fields
EXPECT_EQ(2, state->dataOutput->iNumberOfDefaultedFields); // Number of defaulted fields in IDF
EXPECT_EQ(0, state->dataOutput->iNumberOfAutoSizedFields); // Number of autosized fields in IDF
EXPECT_EQ(0, state->dataOutput->iNumberOfAutoCalcedFields); // Number of autocalculated fields
Expand Down
44 changes: 42 additions & 2 deletions tst/EnergyPlus/unit/SolarShading.unit.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5262,7 +5262,7 @@ TEST_F(EnergyPlusFixture, SolarShadingTest_checkScheduledSurfacePresent)
// Test of check to see if all surfaces have a sunlit schedule when shadow calculations are set to "Scheduled"
{
auto &surfData = state->dataSurface;
surfData->TotSurfaces = 5;
surfData->TotSurfaces = 6;
surfData->Surface.allocate(surfData->TotSurfaces);

// Set up data for test: three of five surfaces are non-shading and two are shading. Shading surfaces
Expand All @@ -5283,17 +5283,57 @@ TEST_F(EnergyPlusFixture, SolarShadingTest_checkScheduledSurfacePresent)
surfData->Surface(5).Class = SurfaceClass::Overhang;
surfData->Surface(5).SurfSchedExternalShadingFrac = false;
surfData->Surface(5).Name = "SHADING2NOTOK";
surfData->Surface(6).Class = SurfaceClass::Window;
surfData->Surface(6).SurfSchedExternalShadingFrac = false;
surfData->Surface(6).Name = "WINDOW2NOTOK";
Comment on lines +5286 to +5288
Copy link
Collaborator Author

@joseph-robertson joseph-robertson Mar 3, 2026

Choose a reason for hiding this comment

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

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


checkScheduledSurfacePresent(*state);

std::string const error_string = delimited_string({
" ** Warning ** ShadowCalculation specified Schedule for the Shading Calculation Method but no schedule provided for WINDOW1NOTOK",
" ** Warning ** ShadowCalculation specified Schedule for the Shading Calculation Method but no schedule provided for WINDOW1NOTOK.",
" ** ~~~ ** When Schedule is selected for the Shading Calculation Method and no schedule is provided for a particular surface,",
" ** ~~~ ** EnergyPlus will assume that the surface is not shaded. Use SurfaceProperty:LocalEnvironment to specify a schedule",
" ** ~~~ ** for sunlit fraction if this was not desired. Otherwise, this surface will not be shaded at all.",
" ** Warning ** No schedule was provided for WINDOW2NOTOK either. See above error message for more details.",
});
EXPECT_TRUE(compare_err_stream(error_string, true));
}

TEST_F(EnergyPlusFixture, SolarShadingTest_checkNotScheduledSurfacePresent)
Copy link
Collaborator Author

@joseph-robertson joseph-robertson Mar 3, 2026

Choose a reason for hiding this comment

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

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

// Test of check to see if all surfaces do not have a sunlit schedule when shadow calculations are not set to "Scheduled"
{
auto &surfData = state->dataSurface;
surfData->TotSurfaces = 5;
surfData->Surface.allocate(surfData->TotSurfaces);

// Set up data for test: three of five surfaces are non-shading and two are shading. Shading surfaces
// are skipped so they should not report any errors. Two of the surfaces will be correctly defined while
// one surface will not be correctly defined and will generate an error.
surfData->Surface(1).Class = SurfaceClass::Wall;
surfData->Surface(1).SurfSchedExternalShadingFrac = true;
surfData->Surface(1).Name = "WALL1OK";
surfData->Surface(2).Class = SurfaceClass::Roof;
surfData->Surface(2).SurfSchedExternalShadingFrac = true;
surfData->Surface(2).Name = "ROOF1OK";
surfData->Surface(3).Class = SurfaceClass::Window;
surfData->Surface(3).SurfSchedExternalShadingFrac = false;
surfData->Surface(3).Name = "WINDOW1NOTOK";
surfData->Surface(4).Class = SurfaceClass::Shading;
surfData->Surface(4).SurfSchedExternalShadingFrac = true;
surfData->Surface(4).Name = "SHADING1OK";
surfData->Surface(5).Class = SurfaceClass::Overhang;
surfData->Surface(5).SurfSchedExternalShadingFrac = false;
surfData->Surface(5).Name = "SHADING2NOTOK";

checkNotScheduledSurfacePresent(*state);

std::string const error_string = delimited_string({
" ** Warning ** ShadowCalculation did not specify Schedule for the Shading Calculation Method but schedule provided for WALL1OK.",
" ** Warning ** Schedule was also provided for ROOF1OK. See above error message for more details.",
});
EXPECT_TRUE(compare_err_stream(error_string, true));
}

TEST_F(EnergyPlusFixture, SolarShadingTest_CalcBeamSolarOnWinRevealSurface)
{
state->dataGlobal->TimeStepsInHour = 6;
Expand Down
Loading