Space for IlluminanceMap and Internal Mass #10659

mjwitte · 2024-08-13T16:45:56Z

Pull request overview

Fixes Output:IlluminanceMap doesn't support Space #9140
Allow Output:IlluminanceMap to be for a zone or space.
Partially addresses InternalMass should use a 'Zone or ZoneList or Space or SpaceList' name like loads do #9141
For InternalMass, Zone or ZoneList Name is no longer required. If Zone/ZoneList Name is entered without a space name, then the internal mass will be added to the last space that belongs to the Zone(s). If a Space/SpaceList name is entered, then the Zone/ZoneList name is ignored. The internal mass will be added to the zone that the space(s) belongs to.

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.

mjwitte

Code walkthru. Small changes to clean up space handling. Added a new testfile 5ZoneAirCooledWithSpacesDaylightingIntMass.idf to test various applications of these inputs.

mjwitte · 2024-08-14T00:24:21Z

src/EnergyPlus/DaylightingManager.cc

-                                       ipsc->cAlphaFieldNames(2),
-                                       ipsc->cAlphaArgs(2)));
-                ErrorsFound = true;
+            int const spaceNum = Util::FindItemInList(state.dataIPShortCut->cAlphaArgs(2), state.dataHeatBal->space);


Accept a Space Name for Output:IlluminanceMap.

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?

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.

mjwitte · 2024-08-14T00:24:58Z

src/EnergyPlus/DaylightingManager.cc

-                                               ipsc->cAlphaArgs(1)));
-                        ErrorsFound = true;
-                        break;
+                illumMap.zoneIndex = Util::FindItemInList(ipsc->cAlphaArgs(2), state.dataHeatBal->Zone);


If name is not a space, then lookup a zone.

mjwitte · 2024-08-14T00:26:00Z

src/EnergyPlus/DaylightingManager.cc

@@ -4308,6 +4322,7 @@ void GetInputIlluminanceMap(EnergyPlusData &state, bool &ErrorsFound)
        }
    }
    ZoneMsgDone.deallocate();
+    if (ErrorsFound) return;


This was too late, and could cause an array bounds error in the eio printing below.

mjwitte · 2024-08-14T00:27:02Z

src/EnergyPlus/SurfaceGeometry.cc

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


When checking the surface space assignments, don't want a stray InternalMass object to trigger adding a new -Remainder space.

I don't really understand this, but it does seem to make sense.

jmarrec

Looks good to me!

jmarrec · 2024-08-19T09:09:11Z

src/EnergyPlus/DaylightingManager.cc

-                                       ipsc->cAlphaFieldNames(2),
-                                       ipsc->cAlphaArgs(2)));
-                ErrorsFound = true;
+            int const spaceNum = Util::FindItemInList(state.dataIPShortCut->cAlphaArgs(2), state.dataHeatBal->space);


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?

jmarrec · 2024-08-19T09:10:13Z

src/EnergyPlus/DaylightingManager.cc

+                            ShowContinueError(
+                                state,
+                                format("Zone=\"{}\" spans multiple enclosures. Use a Space Name instead.", state.dataHeatBal->Zone(zoneNum).Name));


jmarrec · 2024-08-19T09:14:00Z

src/EnergyPlus/SurfaceGeometry.cc

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


I don't really understand this, but it does seem to make sense.

jmarrec · 2024-08-19T09:14:11Z

src/Transition/InputRulesFiles/Rules24-1-0-to-24-2-0.md

+# epJSON Field Name Change: Output:IlluminanceMap
+`zone_name` --> `zone_or_space_name`


👍

(And no transition (fortran) needed because field didn't change location, and will continue to work)

jmarrec · 2024-08-19T09:15:05Z

testfiles/CMakeLists.txt

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


New test file, nice.

jmarrec · 2024-08-19T09:21:25Z

testfiles/5ZoneAirCooledWithSpacesDaylightingIntMass.idf

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

…MapInternalMass

mjwitte · 2024-08-19T13:43:28Z

src/EnergyPlus/DaylightingManager.cc

-                                       ipsc->cCurrentModuleObject));
-                ErrorsFound = true;
-                continue;
+        // Is it a zone or space name?


Reverse the order for consistency with other objects, check for zone name match first, then space name.

mjwitte · 2024-08-19T13:44:33Z

src/EnergyPlus/DaylightingManager.cc

+                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;


This check wasn't working at all before. Fixed the logic and changed to a warning.

mjwitte · 2024-08-19T13:47:23Z

src/EnergyPlus/DaylightingManager.cc

                }
+                spaceHasDaylightingControl(spaceNum) = true;


Same here. spaceHasDaylightingControl was never set to true before.

Myoldmopar · 2024-08-19T16:00:13Z

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

Space for IlluminanceMap

9bd2143

mjwitte added NewFeature Includes code to add a new feature to EnergyPlus IDDChange Code changes impact the IDD file (cannot be merged after IO freeze) labels Aug 13, 2024

mjwitte added this to the EnergyPlus 24.2 IOFreeze milestone Aug 13, 2024

Space cleanup for InternalMass

1c2a56c

mjwitte marked this pull request as ready for review August 13, 2024 21:44

mjwitte commented Aug 14, 2024

View reviewed changes

mjwitte mentioned this pull request Aug 14, 2024

InternalMass should use a 'Zone or ZoneList or Space or SpaceList' name like loads do #9141

Open

3 tasks

mjwitte requested review from Myoldmopar and jmarrec August 14, 2024 13:53

jmarrec approved these changes Aug 19, 2024

View reviewed changes

mjwitte added 2 commits August 19, 2024 07:28

Merge remote-tracking branch 'remotes/origin/develop' into spaceIllum…

7b89950

…MapInternalMass

Zone then Space and other cleanup

c07880f

mjwitte commented Aug 19, 2024

View reviewed changes

Myoldmopar merged commit e1599d2 into develop Aug 19, 2024
15 checks passed

Myoldmopar deleted the spaceIllumMapInternalMass branch August 19, 2024 16:00

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Space for IlluminanceMap and Internal Mass #10659

Space for IlluminanceMap and Internal Mass #10659

mjwitte commented Aug 13, 2024 •

edited

Loading

mjwitte left a comment

mjwitte Aug 14, 2024

jmarrec Aug 19, 2024

mjwitte Aug 19, 2024

mjwitte Aug 14, 2024

mjwitte Aug 14, 2024

mjwitte Aug 14, 2024

jmarrec Aug 19, 2024

jmarrec left a comment

jmarrec Aug 19, 2024

jmarrec Aug 19, 2024

jmarrec Aug 19, 2024

jmarrec Aug 19, 2024

jmarrec Aug 19, 2024

jmarrec Aug 19, 2024

mjwitte Aug 19, 2024

mjwitte Aug 19, 2024

mjwitte Aug 19, 2024

Myoldmopar commented Aug 19, 2024

		# epJSON Field Name Change: Output:IlluminanceMap
		`zone_name` --> `zone_or_space_name`

Space for IlluminanceMap and Internal Mass #10659

Space for IlluminanceMap and Internal Mass #10659

Conversation

mjwitte commented Aug 13, 2024 • edited Loading

Pull request overview

Pull Request Author

Reviewer

mjwitte left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jmarrec left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Myoldmopar commented Aug 19, 2024

mjwitte commented Aug 13, 2024 •

edited

Loading