-
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
Calculate Mean Radiant Temperature (MRT) for Enclosures (not zones) #9628
Conversation
Array1D<DataViewFactorInformation::EnclosureViewFactorInformation> EnclRadInfo; | ||
Array1D<DataViewFactorInformation::EnclosureViewFactorInformation> EnclSolInfo; | ||
EPVector<DataViewFactorInformation::EnclosureViewFactorInformation> EnclRadInfo; | ||
EPVector<DataViewFactorInformation::EnclosureViewFactorInformation> EnclSolInfo; |
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 love struct
/class
type names that include the word "Data" or "Information". So what you're saying is ... this struct
contains information? I think that's only allowed in C++20 and we're only on C++17.
for (int radEnclosureNum = 1; radEnclosureNum <= state.dataViewFactor->NumOfRadiantEnclosures; ++radEnclosureNum) { | ||
auto &thisEnclosure(state.dataViewFactor->EnclRadInfo(radEnclosureNum)); | ||
if (!state.dataHeatBal->EnclRadReCalc(radEnclosureNum)) continue; | ||
for (auto &thisRadEnclosure : state.dataViewFactor->EnclRadInfo) { |
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.
👍
@nealkruis @shorowit This change results in basement surface temperatures which are cooler on average. Is that what you expect? |
That makes sense to me if the non-basement surfaces are warmer than the basement surfaces on average. |
@Myoldmopar This is mostly ready for review. It addresses part of #9377 but not all. No diffs are expected for the base test suite. This still needs a unit test or two. Handing this off to @jcyuan2020 to add a unit test and respond to comments. |
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.
Highlights:
Array1D<Real64> EnclRadQThermalRad; // TOTAL THERMAL RADIATION ADDED TO ZONE or Radiant Enclosure (group of zones) | ||
Array1D<Real64> EnclRadThermAbsMult; // EnclRadThermAbsMult - MULTIPLIER TO COMPUTE 'ITABSF' | ||
Array1D<bool> EnclSolAbsFirstCalc; // for error message | ||
Array1D<bool> EnclRadReCalc; // Enclosure solar or thermal radiation properties needs to be recalc due to window/shading status change |
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.
Moved to enclosure data. Which then required non-substantive changes throughout the code.
Array1D<Real64> SurfaceAE; // Product of area and emissivity for each surface | ||
Array1D<Real64> ZoneAESum; // Sum of area times emissivity for all zone surfaces |
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.
Moved to Surface struct
@@ -372,7 +372,7 @@ void KivaInstanceMap::setBoundaryConditions(EnergyPlusData &state) | |||
state.dataHeatBalSurf->SurfQdotRadHVACInPerArea(floorSurface); // HVAC | |||
|
|||
bcs->slabConvectiveTemp = state.dataHeatBal->SurfTempEffBulkAir(floorSurface) + DataGlobalConstants::KelvinConv; | |||
bcs->slabRadiantTemp = ThermalComfort::CalcSurfaceWeightedMRT(state, zoneNum, floorSurface, false) + DataGlobalConstants::KelvinConv; | |||
bcs->slabRadiantTemp = ThermalComfort::CalcSurfaceWeightedMRT(state, floorSurface, false) + DataGlobalConstants::KelvinConv; |
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.
zoneNum is unnecessary. Get the enclosurenum now from the surface.
"...invalid " + state.dataIPShortCut->cAlphaFieldNames(2) + "=\"" + state.dataIPShortCut->cAlphaArgs(2) + "\"."); | ||
ErrorsFound = true; | ||
} | ||
// Ignore ZoneName cAlphaArgs(2) |
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.
Modify ComfortViewFactorAngles to ignore zone name and check that all surfaces and associated People object are in the same radiant enclosure.
for (auto const &thisRadEnclosure : state.dataViewFactor->EnclRadInfo) { | ||
for (int const SurfNum2 : thisRadEnclosure.SurfacePtr) { |
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 heart of all of this - loop over enclosure surfaces instead of zone surfaces.
auto &thisSurface = state.dataSurface->Surface(SurfNum); | ||
auto &thisRadEnclosure = state.dataViewFactor->EnclRadInfo(thisSurface.RadEnclIndex); | ||
// Recalc SurfaceEnclAESum only if needed due to window shades or EMS | ||
if (thisRadEnclosure.radReCalc) { |
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.
Recalculate area*emissivity only if needed.
Array1D<Real64> AngleFactor; // Angle factor of each surface | ||
std::string Name; // Angle factor list name | ||
Array1D_string SurfaceName; // Names of the Surfces | ||
Array1D_int SurfacePtr; // ALLOCATABLE to the names of the Surfces | ||
int TotAngleFacSurfaces; // Total number of surfaces | ||
std::string ZoneName; // Name of zone the system is serving | ||
int ZonePtr; // Point to this zone in the Zone derived type | ||
|
||
// Default Constructor | ||
AngleFactorData() : TotAngleFacSurfaces(0), ZonePtr(0) | ||
{ | ||
} | ||
EPVector<Real64> AngleFactor; // Angle factor of each surface |
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.
Some gratuitous Array1D --> EPVector scattered about.
this->runningAverageCEN = 0.0; | ||
this->useEpwDataCEN = false; | ||
this->firstDaySet = false; // first day is set with initiate -- so do not update | ||
new (this) ThermalComfortsData(); |
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.
Use the latest clear_state form.
|
||
With this choice, the method used will be a factor per floor area of the zone. (The People per Zone Floor Area field should be filled). | ||
\item[People/Area] With this choice, the method used will be a factor per floor area of the zone. (The People per Zone Floor Area field should be filled). | ||
|
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.
"floor area of the zone. (The People per Zone Floor Area" Since there is more work here, I would suggest adding to the end of the first sentance "or Space" in the future here and other places like this. Also it's just "People per Floor Area" now, another clean up that could happen in a future commit.
@@ -827,20 +802,20 @@ \subsection{Simplified ASHRAE 55 Warnings}\label{simplified-ashrae-55-warnings} | |||
\item | |||
Eliminate occupancy when conditioning equipment is off. | |||
\item | |||
Note that the ASHRAE graph lower limit is (19.6C to 21.7C) -- heating setpoints may need to be nearer 22.2C (72F) than 21.1C (70F). | |||
Note that the ASHRAE graph lower limit is (19.6$^\circ$C to 21.7$^\circ$C)---heating setpoints may need to be nearer 22.2$^\circ$C (72$^\circ$F) than 21.1$^\circ$C (70$^\circ$F). | |||
\item |
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.
for (int const SurfNum : thisEnclosure.SurfacePtr) { | ||
if (!Surface(SurfNum).HeatTransSurf) continue; | ||
int const ConstrNum = Surface(SurfNum).Construction; | ||
for (int const SurfNum : thisRadEnclosure.SurfacePtr) { |
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.
Should most of E+ for loops use int const X?
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 changes and unit test look good.
@mjwitte is there anything else needed for this pass? |
Not that I'm aware of. Thanks for the review. |
This change has no CI diffs and will merge later today. |
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.