-
Notifications
You must be signed in to change notification settings - Fork 389
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
Space for IlluminanceMap and Internal Mass #10659
Conversation
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.
Code walkthru. Small changes to clean up space handling. Added a new testfile 5ZoneAirCooledWithSpacesDaylightingIntMass.idf to test various applications of these inputs.
src/EnergyPlus/DaylightingManager.cc
Outdated
ipsc->cAlphaFieldNames(2), | ||
ipsc->cAlphaArgs(2))); | ||
ErrorsFound = true; | ||
int const spaceNum = Util::FindItemInList(state.dataIPShortCut->cAlphaArgs(2), state.dataHeatBal->space); |
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.
Accept a Space Name for Output:IlluminanceMap.
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.
The way you ordered the if-else ladder seem to imply it would be more common now to use a Space Name rather than a Zone Name. Is that so?
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.
The way you ordered the if-else ladder seem to imply it would be more common now to use a Space Name rather than a Zone Name. Is that so?
Good question. I didn't pay much attention to this.
For Daylighting:Controls
space is checked first.
For Daylighting:ReferencePoint
zone is first.
For Output:IlluminanceMap
space is first.
For internal gains (Lights
, People
, etc.) it is zone, then space, then zonelist, then spacelist.
I will change Daylighting:Controls
and Output:IlluminanceMap
to check zone first, then space. That will be consistent with the order in the field name.
src/EnergyPlus/DaylightingManager.cc
Outdated
ipsc->cAlphaArgs(1))); | ||
ErrorsFound = true; | ||
break; | ||
illumMap.zoneIndex = Util::FindItemInList(ipsc->cAlphaArgs(2), state.dataHeatBal->Zone); |
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.
If name is not a space, then lookup a zone.
@@ -4308,6 +4322,7 @@ void GetInputIlluminanceMap(EnergyPlusData &state, bool &ErrorsFound) | |||
} | |||
} | |||
ZoneMsgDone.deallocate(); | |||
if (ErrorsFound) return; |
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.
This was too late, and could cause an array bounds error in the eio printing below.
@@ -2846,7 +2846,8 @@ namespace SurfaceGeometry { | |||
|
|||
for (int surfNum = 1; surfNum <= state.dataSurface->TotSurfaces; ++surfNum) { | |||
auto &thisSurf = Surfaces(surfNum); | |||
if (!thisSurf.HeatTransSurf) continue; // ignore shading surfaces | |||
if (!thisSurf.HeatTransSurf) continue; // ignore shading surfaces | |||
if (thisSurf.Class == DataSurfaces::SurfaceClass::IntMass) continue; // skip internal mass surfaces for this check |
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.
When checking the surface space assignments, don't want a stray InternalMass object to trigger adding a new -Remainder space.
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 don't really understand this, but it does seem to make sense.
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.
Looks good to me!
src/EnergyPlus/DaylightingManager.cc
Outdated
ipsc->cAlphaFieldNames(2), | ||
ipsc->cAlphaArgs(2))); | ||
ErrorsFound = true; | ||
int const spaceNum = Util::FindItemInList(state.dataIPShortCut->cAlphaArgs(2), state.dataHeatBal->space); |
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.
The way you ordered the if-else ladder seem to imply it would be more common now to use a Space Name rather than a Zone Name. Is that so?
src/EnergyPlus/DaylightingManager.cc
Outdated
ShowContinueError( | ||
state, | ||
format("Zone=\"{}\" spans multiple enclosures. Use a Space Name instead.", state.dataHeatBal->Zone(zoneNum).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.
Nice.
@@ -2846,7 +2846,8 @@ namespace SurfaceGeometry { | |||
|
|||
for (int surfNum = 1; surfNum <= state.dataSurface->TotSurfaces; ++surfNum) { | |||
auto &thisSurf = Surfaces(surfNum); | |||
if (!thisSurf.HeatTransSurf) continue; // ignore shading surfaces | |||
if (!thisSurf.HeatTransSurf) continue; // ignore shading surfaces | |||
if (thisSurf.Class == DataSurfaces::SurfaceClass::IntMass) continue; // skip internal mass surfaces for this check |
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 don't really understand this, but it does seem to make sense.
# epJSON Field Name Change: Output:IlluminanceMap | ||
`zone_name` --> `zone_or_space_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.
👍
(And no transition (fortran) needed because field didn't change location, and will continue to work)
@@ -51,6 +51,7 @@ add_simulation_test(IDF_FILE 5ZoneAirCooledConvCoef.idf EPW_FILE USA_CO_Golden-N | |||
add_simulation_test(IDF_FILE 5ZoneAirCooledConvCoef_VSFan.idf EPW_FILE USA_IL_Chicago-OHare.Intl.AP.725300_TMY3.epw) | |||
add_simulation_test(IDF_FILE 5ZoneAirCooledKeepSiteLocation.idf EPW_FILE USA_CO_Golden-NREL.724666_TMY3.epw) | |||
add_simulation_test(IDF_FILE 5ZoneAirCooledWithSpaces.idf EPW_FILE USA_IL_Chicago-OHare.Intl.AP.725300_TMY3.epw) | |||
add_simulation_test(IDF_FILE 5ZoneAirCooledWithSpacesDaylightingIntMass.idf EPW_FILE USA_IL_Chicago-OHare.Intl.AP.725300_TMY3.epw) |
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.
New test file, nice.
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 diffed 5ZoneAirCooledWithSpaces.idf
and this one.
"Zone 1" and "Space 3" got new Daylighting:Controls, Daylighting:ReferencePoint, and Output:IlluminanceMap.
All Zones, and All Spaces, plus "Space 5 Office" and "Space 5 Conference" specifically, now have an InternalMass object.
--- 5ZoneAirCooledWithSpaces.idf 2024-05-16 17:56:08.849224791 +0200
+++ 5ZoneAirCooledWithSpacesDaylightingIntMass.idf 2024-08-19 11:16:50.565934236 +0200
@@ -126,7 +126,8 @@
GlobalGeometryRules,
UpperLeftCorner, !- Starting Vertex Position
CounterClockWise, !- Vertex Entry Direction
- Relative; !- Coordinate System
+ Relative, !- Coordinate System
+ Relative; !- Daylighting Reference Point Coordinate System
ScheduleTypeLimits,
Any Number; !- Name
@@ -924,9 +925,45 @@
0.2, !- Return Air Fraction
0.59, !- Fraction Radiant
0.2, !- Fraction Visible
- 0, !- Fraction Replaceable
+ 1.0, !- Fraction Replaceable
GeneralLights; !- End-Use Subcategory
+ Daylighting:Controls,
+ Zone 1 Daylighting Control, !- Name
+ Zone 1, !- Zone or Space Name
+ SplitFlux, !- Daylighting Method
+ , !- Availability Schedule Name
+ Stepped, !- Lighting Control Type
+ 0.3000, !- Minimum Input Power Fraction for Continuous or ContinuousOff Dimming Control
+ 0.2000, !- Minimum Light Output Fraction for Continuous or ContinuousOff Dimming Control
+ 3, !- Number of Stepped Control Steps
+ 1.0000, !- Probability Lighting will be Reset When Needed in Manual Stepped Control
+ Zone 1 Reference Point1, !- Glare Calculation Daylighting Reference Point Name
+ 180, !- Glare Calculation Azimuth Angle of View Direction Clockwise from Zone y-Axis {deg}
+ 20, !- Maximum Allowable Discomfort Glare Index
+ , !- DElight Gridding Resolution {m2}
+ Zone 1 Reference Point1, !- Daylighting Reference Point 1 Name
+ 1.0000, !- Fraction of Lights Controlled by Reference Point 1
+ 400.0000; !- Illuminance Setpoint at Reference Point 1 {lux}
+
+ Daylighting:ReferencePoint,
+ Zone 1 Reference Point1, !- Name
+ Zone 1, !- Zone or Space Name
+ 15.0, !- X-Coordinate of Reference Point {m}
+ 3.0, !- Y-Coordinate of Reference Point {m}
+ 0.7; !- Z-Coordinate of Reference Point {m}
+
+ Output:IlluminanceMap,
+ Zone 1 Daylit Map, !- Name
+ Zone 1, !- Zone Name
+ 0.8, !- Z height {m}
+ 3.9, !- X Minimum Coordinate {m}
+ 26.6, !- X Maximum Coordinate {m}
+ 5, !- Number of X Grid Points
+ 0.2, !- Y Minimum Coordinate {m}
+ 3.5, !- Y Maximum Coordinate {m}
+ 5; !- Number of Y Grid Points
+
ElectricEquipment,
All Spaces Elec Equip, !- Name
AllConditionedSpaces, !- Zone or ZoneList or Space or SpaceList Name
@@ -1307,9 +1344,45 @@
0.2, !- Return Air Fraction
0.59, !- Fraction Radiant
0.2, !- Fraction Visible
- 0, !- Fraction Replaceable
+ 1.0, !- Fraction Replaceable
GeneralLights; !- End-Use Subcategory
+ Daylighting:Controls,
+ Space 3 Open Office 1 Daylighting Control, !- Name
+ Space 3 Open Office 1, !- Zone or Space Name
+ SplitFlux, !- Daylighting Method
+ , !- Availability Schedule Name
+ Stepped, !- Lighting Control Type
+ 0.3000, !- Minimum Input Power Fraction for Continuous or ContinuousOff Dimming Control
+ 0.2000, !- Minimum Light Output Fraction for Continuous or ContinuousOff Dimming Control
+ 3, !- Number of Stepped Control Steps
+ 1.0000, !- Probability Lighting will be Reset When Needed in Manual Stepped Control
+ Space 3 Open Office 1 Reference Point1, !- Glare Calculation Daylighting Reference Point Name
+ 180, !- Glare Calculation Azimuth Angle of View Direction Clockwise from Zone y-Axis {deg}
+ 20, !- Maximum Allowable Discomfort Glare Index
+ , !- DElight Gridding Resolution {m2}
+ Space 3 Open Office 1 Reference Point1, !- Daylighting Reference Point 1 Name
+ 1.0000, !- Fraction of Lights Controlled by Reference Point 1
+ 400.0000; !- Illuminance Setpoint at Reference Point 1 {lux}
+
+ Daylighting:ReferencePoint,
+ Space 3 Open Office 1 Reference Point1, !- Name
+ Space 3 Open Office 1, !- Zone or Space Name
+ 8.0, !- X-Coordinate of Reference Point {m}
+ 12.5, !- Y-Coordinate of Reference Point {m}
+ 0.7; !- Z-Coordinate of Reference Point {m}
+
+ Output:IlluminanceMap,
+ Space 3 Open Office 1 Daylit Map, !- Name
+ Space 3 Open Office 1, !- Zone or Space Name
+ 0.8, !- Z height {m}
+ 3.8, !- X Minimum Coordinate {m}
+ 11.8, !- X Maximum Coordinate {m}
+ 5, !- Number of X Grid Points
+ 11.8, !- Y Minimum Coordinate {m}
+ 15.0, !- Y Maximum Coordinate {m}
+ 10; !- Number of Y Grid Points
+
Lights,
Space 3 Open Office 2 Lights, !- Name
Space 3 Open Office 2, !- Zone or ZoneList or Space or SpaceList Name
@@ -1338,6 +1411,31 @@
0, !- Fraction Replaceable
GeneralLights; !- End-Use Subcategory
+ Construction,
+ Furniture, !- Name
+ WD10; !- Outside Layer
+
+ InternalMass,
+ Zone 3 Furniture, !- Name
+ Furniture, !- Construction Name
+ Zone 3, !- Zone or ZoneList Name
+ , !- Space or SpaceList Name
+ 77; !- Surface Area {m2}
+
+ InternalMass,
+ Misc Furniture, !- Name
+ Furniture, !- Construction Name
+ , !- Zone or ZoneList Name
+ AllConditionedSpaces, !- Space or SpaceList Name
+ 5.0; !- Surface Area {m2}
+
+ InternalMass,
+ Misc Furniture 2, !- Name
+ Furniture, !- Construction Name
+ All Zones, !- Zone or ZoneList Name
+ , !- Space or SpaceList Name
+ 2.0; !- Surface Area {m2}
+
BuildingSurface:Detailed,
BACK-1, !- Name
WALL, !- Surface Type
@@ -1898,6 +1996,20 @@
0, !- Fraction Replaceable
GeneralLights; !- End-Use Subcategory
+ InternalMass,
+ Space 5 Office Furniture, !- Name
+ Furniture, !- Construction Name
+ , !- Zone or ZoneList Name
+ Space 5 Office, !- Space or SpaceList Name
+ 10.0; !- Surface Area {m2}
+
+ InternalMass,
+ Space 5 Conference Furniture, !- Name
+ Furniture, !- Construction Name
+ , !- Zone or ZoneList Name
+ Space 5 Conference, !- Space or SpaceList Name
+ 5.0; !- Surface Area {m2}
+
BuildingSurface:Detailed,
C5-1, !- Name
CEILING, !- Surface Type
ipsc->cCurrentModuleObject)); | ||
ErrorsFound = true; | ||
continue; | ||
// Is it a zone or space 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.
Reverse the order for consistency with other objects, check for zone name match first, then space name.
if (spaceHasDaylightingControl(zoneSpaceNum)) { | ||
ShowWarningError(state, | ||
format("{}=\"{}\" Space=\"{}\" already has a {} object assigned to it.", | ||
ipsc->cCurrentModuleObject, | ||
daylightControl.Name, | ||
state.dataHeatBal->space(zoneSpaceNum).Name, | ||
ipsc->cCurrentModuleObject)); | ||
ShowContinueError(state, "This control will override the lighting power factor for this space."); | ||
} | ||
spaceHasDaylightingControl(zoneSpaceNum) = true; |
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.
This check wasn't working at all before. Fixed the logic and changed to a warning.
} | ||
spaceHasDaylightingControl(spaceNum) = true; |
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.
Same here. spaceHasDaylightingControl
was never set to true before.
This really does seem fine and a minor IO change that would be good to drop in now. Locally it's still all happy, so let's merge this. Thanks @mjwitte |
Pull request overview
Pull Request Author
Add to this list or remove from it as applicable. This is a simple templated set of guidelines.
Reviewer
This will not be exhaustively relevant to every PR.