-
Notifications
You must be signed in to change notification settings - Fork 392
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
Daylighting output revisions and other cleanup related to Enclosures vs Zones #9102
Conversation
DataDaylighting::SkyType::ClearTurbid, | ||
DataDaylighting::SkyType::Intermediate, | ||
DataDaylighting::SkyType::Overcast}) { | ||
std::string skyTypeString; |
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.
constexpr std::array<std::string_view>
curRefPt.indexToFracAndIllum = refPtNum; // back reference to the index to the ZoneDaylight structure arrays related to reference points | ||
if (state.dataSurface->DaylRefWorldCoordSystem) { | ||
// transform only by appendix G rotation | ||
daylCntrl.DaylRefPtAbsCoord(1, refPtNum) = curRefPt.x * CosBldgRotAppGonly - curRefPt.y * SinBldgRotAppGonly; |
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 bad girl looks like it should be an Array1D<Vector3>
rather than Array2D
.
for (int daylightCtrlNum = 1; daylightCtrlNum <= state.dataDaylightingData->totDaylightingControls; ++daylightCtrlNum) { | ||
auto &thisDaylightControl = state.dataDaylightingData->daylightControl(daylightCtrlNum); | ||
auto curcolorno = ColorNo::DaylSensor1; | ||
std::string refPtType; |
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.
constexpr std::array<std::string_view>
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.
@@ -247,17 +247,11 @@ namespace DataDaylighting { | |||
Array2D<Real64> MapRefPtAbsCoord; // X,Y,Z coordinates of all illuminance map reference points | |||
// in absolute coordinate system (m) | |||
// Points 1 and 2 are the control reference points | |||
Array1D_bool MapRefPtInBounds; // True when coordinates are in bounds of zone coordinates | |||
Array1D<Real64> DaylIllumAtMapPt; // Daylight illuminance at illuminance map points (lux) |
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.
Delete unused glare arrays in MapCalcData
. More below.
static constexpr std::string_view Format_700( | ||
"! <Sky Daylight Factors>, MonthAndDay, Zone Name, Window Name, Reference Point, Daylight Factor\n"); | ||
"! <Sky Daylight Factors>, Sky Type, MonthAndDay, Daylighting Control Name, Enclosure 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.
Add missing Sky Type
in header.
print(state.files.eio, Format_700); | ||
for (int zoneNum = 1; zoneNum <= state.dataGlobal->NumOfZones; ++zoneNum) { |
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.
Remove outer loop by zone.
print(dfs, | ||
"{}\n", | ||
"Hour,Reference Point,Daylight Factor for Clear Sky,Daylight Factor for Clear Turbid Sky," | ||
"Daylight Factor for Intermediate Sky,Daylight Factor for Overcast Sky"); | ||
state.dataDaylightingManager->CreateDFSReportFile = false; | ||
} | ||
|
||
// TODO MJW: For now preserve order by zone | ||
for (int zoneNum = 1; zoneNum <= state.dataGlobal->NumOfZones; ++zoneNum) { |
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.
Remove outer loop by zone.
// TODO MJW: remove all of the glare calcs for map points | ||
// but these are arguments to some of the functions that are shared with regular reference points, so initalize here. |
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.
Remove as much as possible related to glare calculations in the map functions.
// TODO MJW: Keep zone loop for now to maintain order | ||
for (int zoneNum = 1; zoneNum <= state.dataGlobal->NumOfZones; ++zoneNum) { |
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.
Remove yet another zone loop (YAZL).
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.
Yay!
DXFDaylightingReferencePoints(state, dxffile, false); | ||
DXFDaylightingReferencePoints(state, dxffile, true); | ||
DXFDaylightingReferencePoints(state, dxffile); |
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 is the same thing again in DXFOutLines
.
DXFDaylightingReferencePoints(state, dxffile, false); | ||
DXFDaylightingReferencePoints(state, dxffile); |
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.
Just call this once and handle the different reference point types within the function.
@@ -1256,8 +1257,7 @@ void DXFOutWireFrame(EnergyPlusData &state, std::string const &ColorScheme) | |||
} | |||
} | |||
|
|||
DXFDaylightingReferencePoints(state, dxffile, false); | |||
DXFDaylightingReferencePoints(state, dxffile, true); | |||
DXFDaylightingReferencePoints(state, dxffile); |
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 again in DXFOutWireFrame
.
@@ -3028,3 +3034,299 @@ TEST_F(EnergyPlusFixture, DaylightingManager_ReportIllumMap) | |||
EXPECT_EQ(expectedResultName, state->dataDaylightingData->IllumMap(1).Name); | |||
EXPECT_EQ(expectedResultPtsHeader, state->dataDaylightingData->IllumMap(MapNum).pointsHeader); | |||
} | |||
TEST_F(EnergyPlusFixture, DaylightingManager_DayltgIlluminanceMap) |
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 unit test for map values.
@mjwitte @Myoldmopar it has been 28 days since this pull request was last updated. |
This is all around a really great cleanup. I pulled in develop, and resolved a couple dozen conflicts. They were pretty easy to resolve, just taking the functional changes from this branch and integrating the enum changes from develop. All tests pass, so I'm going to push the branch back up here to get fresh CI results. I'm happy with this going in assuming CI is happy. If anyone wants anything else added here, speak up! |
@@ -177,9 +174,7 @@ namespace DaylightingManager { | |||
bool const Triangle, | |||
Real64 &TVISIntWin, // Visible transmittance of int win at COSBIntWin for light from ext win | |||
Real64 &TVISIntWinDisk, // Visible transmittance of int win at COSBIntWin for sun | |||
Optional_int_const MapNum = _, | |||
// Optional< Real64 > MapWindowSolidAngAtRefPt = _, //Inactive | |||
Optional<Real64> MapWindowSolidAngAtRefPtWtd = _); |
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.
All these Optionals going away is a great move.
@@ -157,7 +155,6 @@ using namespace DataSurfaces; | |||
// Use statements for access to subroutines in other modules | |||
using namespace ScheduleManager; | |||
using namespace SolarShading; | |||
using namespace DaylightingManager; |
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.
@@ -651,25 +456,8 @@ void InitSurfaceHeatBalance(EnergyPlusData &state) | |||
if (state.dataHeatBalSurfMgr->InitSurfaceHeatBalancefirstTime) DisplayString(state, "Initializing Solar Heat Gains"); | |||
|
|||
InitSolarHeatGains(state); | |||
if (state.dataEnvrn->SunIsUp && (state.dataEnvrn->BeamSolarRad + state.dataEnvrn->GndSolarRad + state.dataEnvrn->DifSolarRad > 0.0)) { |
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 is a really nice cleanup in the HBSM.
// TODO MJW: Keep zone loop for now to maintain order | ||
for (int zoneNum = 1; zoneNum <= state.dataGlobal->NumOfZones; ++zoneNum) { |
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.
Yay!
CI looks similar to how it did before, builds and tests fine locally, checked the updated documentation and everything seems in order there. Merging this in, thanks @mjwitte! |
Daylighting output revisions and other cleanup related to Enclosures vs Zones
Pull request overview
int const
with defaults.HeatBalanceSurfaceManager::InitSurfaceHeatBalance
into new functions inDaylightingManager
.Diffs
ToDo List
Pull Request Author
Add to this list or remove from it as applicable. This is a simple templated set of guidelines.
If IDD requires transition, transition source, rules, ExpandObjects, and IDFs must be updated, and add IDDChange labelIf adding/removing any LaTeX docs or figures, update that document's CMakeLists file dependenciesReviewer
This will not be exhaustively relevant to every PR.