Skip to content
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.

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

Produce Convexity Check Warning Only When Appropriate #10621

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
047735d
10299 Split GetShadowingInput
RKStrand Jul 10, 2024
51d00e0
Merge branch 'develop' into 10299CheckConvexityChangeCheckForShadowCa…
RKStrand Jul 10, 2024
c5f3945
10299 Separation of Get and Process
RKStrand Jul 10, 2024
fd61c32
Merge branch 'develop' into 10299CheckConvexityChangeCheckForShadowCa…
RKStrand Jul 10, 2024
ef2072c
Merge branch 'develop' into 10299CheckConvexityChangeCheckForShadowCa…
RKStrand Jul 12, 2024
eaf73f9
10299 Fix and Reverse Old Unit
RKStrand Jul 12, 2024
77886fb
Merge branch 'develop' into 10299CheckConvexityChangeCheckForShadowCa…
RKStrand Jul 12, 2024
4a50416
10299 Unit Test and Fixes
RKStrand Jul 12, 2024
b7c83d9
Merge branch 'develop' into 10299CheckConvexityChangeCheckForShadowCa…
RKStrand Jul 12, 2024
608530e
10299 Unit Test Change Backup
RKStrand Jul 13, 2024
ce283a3
10299 Correction of Unit Test enum
RKStrand Jul 19, 2024
c688048
10299 Move a PixelCounting Check
RKStrand Jul 22, 2024
e5809d3
10299 Correction of Unit Test Errors
RKStrand Jul 22, 2024
f7be748
10299 Another Round of Unit Test Fixes
RKStrand Jul 23, 2024
0a0aa68
10299 Get Rid of Unneeded Get Flag
RKStrand Jul 23, 2024
5664dfa
10299 Clang Format Final
RKStrand Jul 24, 2024
a0cff9a
10299 Request to Improve Variable Name
RKStrand Jul 24, 2024
1841936
Merge branch 'develop' into 10299CheckConvexityChangeCheckForShadowCa…
RKStrand Jul 24, 2024
ef9b816
10299 Back Out Variable Name Change
RKStrand Jul 25, 2024
54e4025
Merge branch 'develop' into 10299CheckConvexityChangeCheckForShadowCa…
RKStrand Jul 25, 2024
08909a8
10299 Attempt 2 at Variable Rename
RKStrand Jul 25, 2024
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
5 changes: 5 additions & 0 deletions src/EnergyPlus/DataSystemVariables.hh
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,11 @@ struct SystemVarsData : BaseGlobalStruct
bool ReportExtShadingSunlitFrac = false; // when true, the sunlit fraction for all surfaces are exported as a csv format output
bool DisableGroupSelfShading = false; // when true, defined shadowing surfaces group is ignored when calculating sunlit fraction
bool DisableAllSelfShading = false; // when true, all external shadowing surfaces is ignored when calculating sunlit fraction
bool DisableSelfShadingWithinGroup = false;
bool DisableSelfShadingBetweenGroup = false;

int shadingGroupsNum = 0; // number of shading groups
Array1D_string shadingGroupZoneListNames; // array of zone names in user input

bool TrackAirLoopEnvFlag = false; // If TRUE generates a file with runtime statistics for each HVAC
// controller on each air loop
Expand Down
2 changes: 2 additions & 0 deletions src/EnergyPlus/HeatBalanceManager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1967,6 +1967,8 @@ 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);
Expand Down
185 changes: 96 additions & 89 deletions src/EnergyPlus/SolarShading.cc
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ void InitSolarCalculations(EnergyPlusData &state)

if (state.dataSolarShading->GetInputFlag) {
checkShadingSurfaceSchedules(state);
GetShadowingInput(state);
processShadowingInput(state);
state.dataSolarShading->GetInputFlag = false;
state.dataSolarShading->MaxHCV =
(((max(15, state.dataSurface->MaxVerticesPerSurface) + 16) / 16) * 16) - 1; // Assure MaxHCV+1 is multiple of 16 for 128 B alignment
Expand Down Expand Up @@ -514,18 +514,6 @@ void GetShadowingInput(EnergyPlusData &state)
state.dataSysVars->shadingMethod = ShadingMethod::PolygonClipping;
}

if ((state.dataSysVars->shadingMethod == DataSystemVariables::ShadingMethod::PixelCounting) &&
state.dataSolarShading->anyScheduledShadingSurface) {
ShowSevereError(state, "The Shading Calculation Method of choice is \"PixelCounting\"; ");
ShowContinueError(state, "and there is at least one shading surface of type ");
ShowContinueError(state, "Shading:Site:Detailed, Shading:Building:Detailed, or Shading:Zone:Detailed, ");
ShowContinueError(state, "that has an active transmittance schedule value greater than zero or may vary.");
ShowContinueError(state, "With \"PixelCounting\" Shading Calculation Method, the shading surfaces will be treated as ");
ShowContinueError(state, "completely opaque (transmittance = 0) during the shading calculation, ");
ShowContinueError(state, "which may result in inaccurate or unexpected results.");
ShowContinueError(state, "It is suggested switching to another Shading Calculation Method, such as \"PolygonClipping\".");
}

aNum++;
if (NumAlphas >= aNum) {
if (Util::SameString(state.dataIPShortCut->cAlphaArgs(aNum), "Periodic")) {
Expand Down Expand Up @@ -629,30 +617,11 @@ void GetShadowingInput(EnergyPlusData &state)
state.dataIPShortCut->cAlphaArgs(aNum) = "No";
state.dataSysVars->ReportExtShadingSunlitFrac = false;
}
if (state.dataSysVars->shadingMethod == ShadingMethod::Imported) {
int ExtShadingSchedNum;
for (int SurfNum = 1; SurfNum <= state.dataSurface->TotSurfaces; ++SurfNum) {
ExtShadingSchedNum = ScheduleManager::GetScheduleIndex(state, state.dataSurface->Surface(SurfNum).Name + "_shading");
if (ExtShadingSchedNum != 0) {
state.dataSurface->Surface(SurfNum).SurfSchedExternalShadingFrac = true;
state.dataSurface->Surface(SurfNum).SurfExternalShadingSchInd = ExtShadingSchedNum;
} else {
ShowWarningError(state,
format("{}: sunlit fraction schedule not found for {} when using ImportedShading.",
cCurrentModuleObject,
state.dataSurface->Surface(SurfNum).Name));
ShowContinueError(state, "These values are set to 1.0.");
}
}
}

bool DisableSelfShadingWithinGroup = false;
bool DisableSelfShadingBetweenGroup = false;

aNum++;
if (NumAlphas >= aNum) {
if (Util::SameString(state.dataIPShortCut->cAlphaArgs(aNum), "Yes")) {
DisableSelfShadingWithinGroup = true;
state.dataSysVars->DisableSelfShadingWithinGroup = true;
state.dataIPShortCut->cAlphaArgs(aNum) = "Yes";
} else if (Util::SameString(state.dataIPShortCut->cAlphaArgs(aNum), "No")) {
state.dataIPShortCut->cAlphaArgs(aNum) = "No";
Expand All @@ -668,7 +637,7 @@ void GetShadowingInput(EnergyPlusData &state)
aNum++;
if (NumAlphas >= aNum) {
if (Util::SameString(state.dataIPShortCut->cAlphaArgs(aNum), "Yes")) {
DisableSelfShadingBetweenGroup = true;
state.dataSysVars->DisableSelfShadingBetweenGroup = true;
state.dataIPShortCut->cAlphaArgs(aNum) = "Yes";
} else if (Util::SameString(state.dataIPShortCut->cAlphaArgs(aNum), "No")) {
state.dataIPShortCut->cAlphaArgs(aNum) = "No";
Expand All @@ -681,66 +650,17 @@ void GetShadowingInput(EnergyPlusData &state)
state.dataIPShortCut->cAlphaArgs(aNum) = "No";
}

if (DisableSelfShadingBetweenGroup && DisableSelfShadingWithinGroup) {
if (state.dataSysVars->DisableSelfShadingBetweenGroup && state.dataSysVars->DisableSelfShadingWithinGroup) {
state.dataSysVars->DisableAllSelfShading = true;
} else if (DisableSelfShadingBetweenGroup || DisableSelfShadingWithinGroup) {
} else if (state.dataSysVars->DisableSelfShadingBetweenGroup || state.dataSysVars->DisableSelfShadingWithinGroup) {
state.dataSysVars->DisableGroupSelfShading = true;
}

aNum++;
int SurfZoneGroup, CurZoneGroup;
if (state.dataSysVars->DisableGroupSelfShading) {
Array1D_int DisableSelfShadingGroups;
int NumOfShadingGroups;
if (NumAlphas >= aNum) {
// Read all shading groups
NumOfShadingGroups = NumAlphas - (aNum - 1);
DisableSelfShadingGroups.allocate(NumOfShadingGroups);
for (int i = 1; i <= NumOfShadingGroups; i++) {
Found = Util::FindItemInList(
state.dataIPShortCut->cAlphaArgs(i + (aNum - 1)), state.dataHeatBal->ZoneList, state.dataHeatBal->NumOfZoneLists);
if (Found != 0) DisableSelfShadingGroups(i) = Found;
}

for (int SurfNum = 1; SurfNum <= state.dataSurface->TotSurfaces; SurfNum++) {
if (state.dataSurface->Surface(SurfNum).ExtBoundCond == 0) { // Loop through all exterior surfaces
SurfZoneGroup = 0;
// Check the shading zone group of each exterior surface
for (int ZoneGroupLoop = 1; ZoneGroupLoop <= NumOfShadingGroups; ZoneGroupLoop++) { // Loop through all defined shading groups
CurZoneGroup = DisableSelfShadingGroups(ZoneGroupLoop);
for (int ZoneNum = 1; ZoneNum <= state.dataHeatBal->ZoneList(CurZoneGroup).NumOfZones;
ZoneNum++) { // Loop through all zones in the zone list
if (state.dataSurface->Surface(SurfNum).Zone == state.dataHeatBal->ZoneList(CurZoneGroup).Zone(ZoneNum)) {
SurfZoneGroup = CurZoneGroup;
break;
}
}
}
// if a surface is not in any zone group, no self shading is disabled for this surface
if (SurfZoneGroup != 0) {
// if DisableSelfShadingWithinGroup, add all zones in the same zone group to the surface's disabled zone list
// if DisableSelfShadingBetweenGroups, add all zones in all other zone groups to the surface's disabled zone list
for (int ZoneGroupLoop = 1; ZoneGroupLoop <= NumOfShadingGroups; ZoneGroupLoop++) { // Loop through all defined shading groups
CurZoneGroup = DisableSelfShadingGroups(ZoneGroupLoop);
if (SurfZoneGroup == CurZoneGroup && DisableSelfShadingWithinGroup) {
for (int ZoneNum = 1; ZoneNum <= state.dataHeatBal->ZoneList(CurZoneGroup).NumOfZones;
ZoneNum++) { // Loop through all zones in the zone list
state.dataSurface->SurfShadowDisabledZoneList(SurfNum).push_back(
state.dataHeatBal->ZoneList(CurZoneGroup).Zone(ZoneNum));
}
} else if (SurfZoneGroup != CurZoneGroup && DisableSelfShadingBetweenGroup) {
for (int ZoneNum = 1; ZoneNum <= state.dataHeatBal->ZoneList(CurZoneGroup).NumOfZones; ZoneNum++) {
state.dataSurface->SurfShadowDisabledZoneList(SurfNum).push_back(
state.dataHeatBal->ZoneList(CurZoneGroup).Zone(ZoneNum));
}
}
}
}
}
}
} else {
ShowFatalError(state, "No Shading groups are defined when disabling grouped self shading.");
}
state.dataSysVars->shadingGroupsNum = NumAlphas - (aNum - 1);
state.dataSysVars->shadingGroupZoneListNames.allocate(state.dataSysVars->shadingGroupsNum);
for (int numZone = 1; numZone <= state.dataSysVars->shadingGroupsNum; ++numZone) {
state.dataSysVars->shadingGroupZoneListNames(numZone) = state.dataIPShortCut->cAlphaArgs(aNum - 1 + numZone);
}

if (!state.dataSysVars->DetailedSolarTimestepIntegration && state.dataSurface->ShadingTransmittanceVaries &&
Expand Down Expand Up @@ -797,6 +717,93 @@ void GetShadowingInput(EnergyPlusData &state)
state.dataIPShortCut->cAlphaArgs(7));
}

void processShadowingInput(EnergyPlusData &state)
{
// all shadow input processing that needed zones and surfaces to already be read into data (part of fix for Defect #10299)

if ((state.dataSysVars->shadingMethod == DataSystemVariables::ShadingMethod::PixelCounting) &&
state.dataSolarShading->anyScheduledShadingSurface) {
ShowSevereError(state, "The Shading Calculation Method of choice is \"PixelCounting\"; ");
ShowContinueError(state, "and there is at least one shading surface of type ");
ShowContinueError(state, "Shading:Site:Detailed, Shading:Building:Detailed, or Shading:Zone:Detailed, ");
ShowContinueError(state, "that has an active transmittance schedule value greater than zero or may vary.");
ShowContinueError(state, "With \"PixelCounting\" Shading Calculation Method, the shading surfaces will be treated as ");
ShowContinueError(state, "completely opaque (transmittance = 0) during the shading calculation, ");
ShowContinueError(state, "which may result in inaccurate or unexpected results.");
ShowContinueError(state, "It is suggested switching to another Shading Calculation Method, such as \"PolygonClipping\".");
}

if (state.dataSysVars->shadingMethod == DataSystemVariables::ShadingMethod::Imported) {
int ExtShadingSchedNum;
for (int SurfNum = 1; SurfNum <= state.dataSurface->TotSurfaces; ++SurfNum) {
ExtShadingSchedNum = ScheduleManager::GetScheduleIndex(state, state.dataSurface->Surface(SurfNum).Name + "_shading");
if (ExtShadingSchedNum != 0) {
state.dataSurface->Surface(SurfNum).SurfSchedExternalShadingFrac = true;
state.dataSurface->Surface(SurfNum).SurfExternalShadingSchInd = ExtShadingSchedNum;
} else {
ShowWarningError(state,
format("processShadowingInput: sunlit fraction schedule not found for {} when using ImportedShading.",
state.dataSurface->Surface(SurfNum).Name));
ShowContinueError(state, "These values are set to 1.0.");
}
}
}

int SurfZoneGroup, CurZoneGroup;
int Found = 0;
if (state.dataSysVars->DisableGroupSelfShading) {
Array1D_int DisableSelfShadingGroups;
int NumOfShadingGroups = state.dataSysVars->shadingGroupsNum;
if (NumOfShadingGroups > 0) {
DisableSelfShadingGroups.allocate(NumOfShadingGroups);
for (int i = 1; i <= NumOfShadingGroups; i++) {
Found = Util::FindItemInList(
state.dataSysVars->shadingGroupZoneListNames(i), state.dataHeatBal->ZoneList, state.dataHeatBal->NumOfZoneLists);
if (Found != 0) DisableSelfShadingGroups(i) = Found;
}

for (int SurfNum = 1; SurfNum <= state.dataSurface->TotSurfaces; SurfNum++) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I know this is existing code, and it's not a frequently used option, but this could be so much more efficient. Instead of looping through all of the zonegroups for each surface, this could simply go through the zones in each zonelist and loop over the exterior surrfaces in each zone (Zone.vZoneHTSurfaceList). Mostly just noting this here, since this seemingly simple fix has already had a lot of scope creep.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree that this would be much more efficient. I'd like to have us make a mental note of this and deal with it at a later date if you are okay with that. I wonder if there are more places where we could be much more efficient and we don't even realize it. If only we all had the time to do that sort of efficiency review in the code...

if (state.dataSurface->Surface(SurfNum).ExtBoundCond == 0) { // Loop through all exterior surfaces
SurfZoneGroup = 0;
// Check the shading zone group of each exterior surface
for (int ZoneGroupLoop = 1; ZoneGroupLoop <= NumOfShadingGroups; ZoneGroupLoop++) { // Loop through all defined shading groups
CurZoneGroup = DisableSelfShadingGroups(ZoneGroupLoop);
for (int ZoneNum = 1; ZoneNum <= state.dataHeatBal->ZoneList(CurZoneGroup).NumOfZones;
ZoneNum++) { // Loop through all zones in the zone list
if (state.dataSurface->Surface(SurfNum).Zone == state.dataHeatBal->ZoneList(CurZoneGroup).Zone(ZoneNum)) {
SurfZoneGroup = CurZoneGroup;
break;
}
}
}
// if a surface is not in any zone group, no self shading is disabled for this surface
if (SurfZoneGroup != 0) {
// if DisableSelfShadingWithinGroup, add all zones in the same zone group to the surface's disabled zone list
// if DisableSelfShadingBetweenGroups, add all zones in all other zone groups to the surface's disabled zone list
for (int ZoneGroupLoop = 1; ZoneGroupLoop <= NumOfShadingGroups; ZoneGroupLoop++) { // Loop through all defined shading groups
CurZoneGroup = DisableSelfShadingGroups(ZoneGroupLoop);
if (SurfZoneGroup == CurZoneGroup && state.dataSysVars->DisableSelfShadingWithinGroup) {
for (int ZoneNum = 1; ZoneNum <= state.dataHeatBal->ZoneList(CurZoneGroup).NumOfZones;
ZoneNum++) { // Loop through all zones in the zone list
state.dataSurface->SurfShadowDisabledZoneList(SurfNum).push_back(
state.dataHeatBal->ZoneList(CurZoneGroup).Zone(ZoneNum));
}
} else if (SurfZoneGroup != CurZoneGroup && state.dataSysVars->DisableSelfShadingBetweenGroup) {
for (int ZoneNum = 1; ZoneNum <= state.dataHeatBal->ZoneList(CurZoneGroup).NumOfZones; ZoneNum++) {
state.dataSurface->SurfShadowDisabledZoneList(SurfNum).push_back(
state.dataHeatBal->ZoneList(CurZoneGroup).Zone(ZoneNum));
}
}
}
}
}
}
} else {
ShowFatalError(state, "No Shading groups are defined when disabling grouped self shading.");
}
}
}

void checkScheduledSurfacePresent(EnergyPlusData &state)
{
// User has chosen "Scheduled" for sunlit fraction so check to see which surfaces don't have a schedule
Expand Down
2 changes: 2 additions & 0 deletions src/EnergyPlus/SolarShading.hh
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,8 @@ namespace SolarShading {

void GetShadowingInput(EnergyPlusData &state);

void processShadowingInput(EnergyPlusData &state);

void checkScheduledSurfacePresent(EnergyPlusData &state);

void AllocateModuleArrays(EnergyPlusData &state);
Expand Down
7 changes: 4 additions & 3 deletions src/EnergyPlus/SurfaceGeometry.cc
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@
#include <EnergyPlus/DataIPShortCuts.hh>
#include <EnergyPlus/DataLoopNode.hh>
#include <EnergyPlus/DataReportingFlags.hh>
#include <EnergyPlus/DataSystemVariables.hh>
#include <EnergyPlus/DataViewFactorInformation.hh>
#include <EnergyPlus/DataWindowEquivalentLayer.hh>
#include <EnergyPlus/DataZoneEquipment.hh>
Expand Down Expand Up @@ -15680,9 +15681,9 @@ namespace SurfaceGeometry {

if (SignFlag != PrevSignFlag) {
if (state.dataGlobal->DisplayExtraWarnings && surfaceTmp.ExtSolar &&
(state.dataHeatBal->SolarDistribution != DataHeatBalance::Shadowing::Minimal) &&
// Warn only once
surfaceTmp.IsConvex) {
(state.dataHeatBal->SolarDistribution != DataHeatBalance::Shadowing::Minimal) && surfaceTmp.IsConvex &&
!state.dataSysVars->SutherlandHodgman &&
(state.dataSysVars->shadingMethod == DataSystemVariables::ShadingMethod::PolygonClipping)) {
ShowWarningError(state,
format("CheckConvexity: Zone=\"{}\", Surface=\"{}\" is non-convex.",
state.dataHeatBal->Zone(surfaceTmp.Zone).Name,
Expand Down
Loading
Loading