-
Notifications
You must be signed in to change notification settings - Fork 466
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.
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
base: develop
Are you sure you want to change the base?
Changes from 10 commits
1840310
f889c1a
0d6bb5f
6a77e91
99ae2e9
1b6a01b
e3da607
1886efe
c8e2d9d
5527d24
1d01bae
937209b
e3475b7
929b663
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1805,11 +1805,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
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This PR inadvertently moved
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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); | ||
| } | ||
|
|
@@ -526,6 +529,7 @@ void GetShadowingInput(EnergyPlusData &state) | |
| } else { | ||
| state.dataIPShortCut->cAlphaArgs(aNum) = "PolygonClipping"; | ||
| state.dataSysVars->shadingMethod = ShadingMethod::PolygonClipping; | ||
| checkNotScheduledSurfacePresent(state); | ||
| } | ||
|
|
||
| aNum++; | ||
|
|
@@ -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; | ||
|
|
@@ -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) | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
||
| { | ||
| // 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)); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -670,215 +670,6 @@ | |
| , !- Sky Diffuse Modeling Algorithm | ||
| Yes; !- Output External Shading Calculation Results | ||
|
|
||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @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.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Got it. Thanks. |
||
| SurfaceProperty:LocalEnvironment, | ||
| LocEnv:EAST SIDE TREE, !- Name | ||
| EAST SIDE TREE, !- Exterior Surface Name | ||
| ExtShadingSch:EAST SIDE TREE, !- Sunlit Fraction Schedule Name | ||
| , !- Surrounding Surfaces Object Name | ||
| ; !- Outdoor Air Node Name | ||
|
|
||
| Schedule:File, | ||
| ExtShadingSch:EAST SIDE TREE, !- Name | ||
| Fraction, !- Schedule Type Limits Name | ||
| SolarShadingTest_Shading_Data.csv, !- File Name | ||
| 2, !- Column Number | ||
| 1, !- Rows to Skip at Top | ||
| , !- Number of Hours of Data | ||
| , !- Column Separator | ||
| , !- Interpolate to Timestep | ||
| 15, !- Minutes per Item | ||
| ; !- Adjust Schedule for Daylight Savings | ||
|
|
||
| SurfaceProperty:LocalEnvironment, | ||
| LocEnv:WEST SIDE TREE, !- Name | ||
| WEST SIDE TREE, !- Exterior Surface Name | ||
| ExtShadingSch:WEST SIDE TREE, !- Sunlit Fraction Schedule Name | ||
| , !- Surrounding Surfaces Object Name | ||
| ; !- Outdoor Air Node Name | ||
|
|
||
| Schedule:File, | ||
| ExtShadingSch:WEST SIDE TREE, !- Name | ||
| Fraction, !- Schedule Type Limits Name | ||
| SolarShadingTest_Shading_Data.csv, !- File Name | ||
| 4, !- Column Number | ||
| 1, !- Rows to Skip at Top | ||
| , !- Number of Hours of Data | ||
| , !- Column Separator | ||
| , !- Interpolate to Timestep | ||
| 15, !- Minutes per Item | ||
| ; !- Adjust Schedule for Daylight Savings | ||
|
|
||
| SurfaceProperty:LocalEnvironment, | ||
| LocEnv:ZN001:WALL001:SHADE001, !- Name | ||
| ZN001:WALL001:SHADE001, !- Exterior Surface Name | ||
| ExtShadingSch:ZN001:WALL001:SHADE001, !- Sunlit Fraction Schedule Name | ||
| , !- Surrounding Surfaces Object Name | ||
| ; !- Outdoor Air Node Name | ||
|
|
||
| Schedule:File, | ||
| ExtShadingSch:ZN001:WALL001:SHADE001, !- Name | ||
| Fraction, !- Schedule Type Limits Name | ||
| SolarShadingTest_Shading_Data.csv, !- File Name | ||
| 6, !- Column Number | ||
| 1, !- Rows to Skip at Top | ||
| , !- Number of Hours of Data | ||
| , !- Column Separator | ||
| , !- Interpolate to Timestep | ||
| 15, !- Minutes per Item | ||
| ; !- Adjust Schedule for Daylight Savings | ||
|
|
||
| SurfaceProperty:LocalEnvironment, | ||
| LocEnv:ZN002:WALL001:SHADE001, !- Name | ||
| ZN002:WALL001:SHADE001, !- Exterior Surface Name | ||
| ExtShadingSch:ZN002:WALL001:SHADE001, !- Sunlit Fraction Schedule Name | ||
| , !- Surrounding Surfaces Object Name | ||
| ; !- Outdoor Air Node Name | ||
|
|
||
| Schedule:File, | ||
| ExtShadingSch:ZN002:WALL001:SHADE001, !- Name | ||
| Fraction, !- Schedule Type Limits Name | ||
| SolarShadingTest_Shading_Data.csv, !- File Name | ||
| 8, !- Column Number | ||
| 1, !- Rows to Skip at Top | ||
| , !- Number of Hours of Data | ||
| , !- Column Separator | ||
| , !- Interpolate to Timestep | ||
| 15, !- Minutes per Item | ||
| ; !- Adjust Schedule for Daylight Savings | ||
|
|
||
| SurfaceProperty:LocalEnvironment, | ||
| LocEnv:ZN003:WALL001:SHADE001, !- Name | ||
| ZN003:WALL001:SHADE001, !- Exterior Surface Name | ||
| ExtShadingSch:ZN003:WALL001:SHADE001, !- Sunlit Fraction Schedule Name | ||
| , !- Surrounding Surfaces Object Name | ||
| ; !- Outdoor Air Node Name | ||
|
|
||
| Schedule:File, | ||
| ExtShadingSch:ZN003:WALL001:SHADE001, !- Name | ||
| Fraction, !- Schedule Type Limits Name | ||
| SolarShadingTest_Shading_Data.csv, !- File Name | ||
| 10, !- Column Number | ||
| 1, !- Rows to Skip at Top | ||
| , !- Number of Hours of Data | ||
| , !- Column Separator | ||
| , !- Interpolate to Timestep | ||
| 15, !- Minutes per Item | ||
| ; !- Adjust Schedule for Daylight Savings | ||
|
|
||
| SurfaceProperty:LocalEnvironment, | ||
| LocEnv:ZN004:WALL001:SHADE001, !- Name | ||
| ZN004:WALL001:SHADE001, !- Exterior Surface Name | ||
| ExtShadingSch:ZN004:WALL001:SHADE001, !- Sunlit Fraction Schedule Name | ||
| , !- Surrounding Surfaces Object Name | ||
| ; !- Outdoor Air Node Name | ||
|
|
||
| Schedule:File, | ||
| ExtShadingSch:ZN004:WALL001:SHADE001, !- Name | ||
| Fraction, !- Schedule Type Limits Name | ||
| SolarShadingTest_Shading_Data.csv, !- File Name | ||
| 12, !- Column Number | ||
| 1, !- Rows to Skip at Top | ||
| , !- Number of Hours of Data | ||
| , !- Column Separator | ||
| , !- Interpolate to Timestep | ||
| 15, !- Minutes per Item | ||
| ; !- Adjust Schedule for Daylight Savings | ||
|
|
||
| SurfaceProperty:LocalEnvironment, | ||
| LocEnv:ZN005:WALL001:SHADE001, !- Name | ||
| ZN005:WALL001:SHADE001, !- Exterior Surface Name | ||
| ExtShadingSch:ZN005:WALL001:SHADE001, !- Sunlit Fraction Schedule Name | ||
| , !- Surrounding Surfaces Object Name | ||
| ; !- Outdoor Air Node Name | ||
|
|
||
| Schedule:File, | ||
| ExtShadingSch:ZN005:WALL001:SHADE001, !- Name | ||
| Fraction, !- Schedule Type Limits Name | ||
| SolarShadingTest_Shading_Data.csv, !- File Name | ||
| 14, !- Column Number | ||
| 1, !- Rows to Skip at Top | ||
| , !- Number of Hours of Data | ||
| , !- Column Separator | ||
| , !- Interpolate to Timestep | ||
| 15, !- Minutes per Item | ||
| ; !- Adjust Schedule for Daylight Savings | ||
|
|
||
| SurfaceProperty:LocalEnvironment, | ||
| LocEnv:ZN006:WALL001:SHADE001, !- Name | ||
| ZN006:WALL001:SHADE001, !- Exterior Surface Name | ||
| ExtShadingSch:ZN006:WALL001:SHADE001, !- Sunlit Fraction Schedule Name | ||
| , !- Surrounding Surfaces Object Name | ||
| ; !- Outdoor Air Node Name | ||
|
|
||
| Schedule:File, | ||
| ExtShadingSch:ZN006:WALL001:SHADE001, !- Name | ||
| Fraction, !- Schedule Type Limits Name | ||
| SolarShadingTest_Shading_Data.csv, !- File Name | ||
| 16, !- Column Number | ||
| 1, !- Rows to Skip at Top | ||
| , !- Number of Hours of Data | ||
| , !- Column Separator | ||
| , !- Interpolate to Timestep | ||
| 15, !- Minutes per Item | ||
| ; !- Adjust Schedule for Daylight Savings | ||
|
|
||
| SurfaceProperty:LocalEnvironment, | ||
| LocEnv:ZN007:WALL001:SHADE001, !- Name | ||
| ZN007:WALL001:SHADE001, !- Exterior Surface Name | ||
| ExtShadingSch:ZN007:WALL001:SHADE001, !- Sunlit Fraction Schedule Name | ||
| , !- Surrounding Surfaces Object Name | ||
| ; !- Outdoor Air Node Name | ||
|
|
||
| Schedule:File, | ||
| ExtShadingSch:ZN007:WALL001:SHADE001, !- Name | ||
| Fraction, !- Schedule Type Limits Name | ||
| SolarShadingTest_Shading_Data.csv, !- File Name | ||
| 18, !- Column Number | ||
| 1, !- Rows to Skip at Top | ||
| , !- Number of Hours of Data | ||
| , !- Column Separator | ||
| , !- Interpolate to Timestep | ||
| 15, !- Minutes per Item | ||
| ; !- Adjust Schedule for Daylight Savings | ||
|
|
||
| SurfaceProperty:LocalEnvironment, | ||
| LocEnv:ZN008:WALL001:SHADE001, !- Name | ||
| ZN008:WALL001:SHADE001, !- Exterior Surface Name | ||
| ExtShadingSch:ZN008:WALL001:SHADE001, !- Sunlit Fraction Schedule Name | ||
| , !- Surrounding Surfaces Object Name | ||
| ; !- Outdoor Air Node Name | ||
|
|
||
| Schedule:File, | ||
| ExtShadingSch:ZN008:WALL001:SHADE001, !- Name | ||
| Fraction, !- Schedule Type Limits Name | ||
| SolarShadingTest_Shading_Data.csv, !- File Name | ||
| 20, !- Column Number | ||
| 1, !- Rows to Skip at Top | ||
| , !- Number of Hours of Data | ||
| , !- Column Separator | ||
| , !- Interpolate to Timestep | ||
| 15, !- Minutes per Item | ||
| ; !- Adjust Schedule for Daylight Savings | ||
|
|
||
| SurfaceProperty:LocalEnvironment, | ||
| LocEnv:ZN009:WALL001:SHADE001, !- Name | ||
| ZN009:WALL001:SHADE001, !- Exterior Surface Name | ||
| ExtShadingSch:ZN009:WALL001:SHADE001, !- Sunlit Fraction Schedule Name | ||
| , !- Surrounding Surfaces Object Name | ||
| ; !- Outdoor Air Node Name | ||
|
|
||
| Schedule:File, | ||
| ExtShadingSch:ZN009:WALL001:SHADE001, !- Name | ||
| Fraction, !- Schedule Type Limits Name | ||
| SolarShadingTest_Shading_Data.csv, !- File Name | ||
| 22, !- Column Number | ||
| 1, !- Rows to Skip at Top | ||
| , !- Number of Hours of Data | ||
| , !- Column Separator | ||
| , !- Interpolate to Timestep | ||
| 15, !- Minutes per Item | ||
| ; !- Adjust Schedule for Daylight Savings | ||
|
|
||
| SurfaceProperty:LocalEnvironment, | ||
| LocEnv:ZN001:WALL001, !- Name | ||
| ZN001:WALL001, !- Exterior Surface Name | ||
|
|
||
There was a problem hiding this comment.
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?
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.
There was a problem hiding this comment.
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:
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.
There was a problem hiding this comment.
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.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
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 AllHeatTranSurfNameschange (to pick up fenestration), but not get overly ambitious with a new reference as we may accidentally exclude a valid object type.