-
Notifications
You must be signed in to change notification settings - Fork 401
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
Mean Radiant Temperature from Zone to Enclosure #10244
Conversation
ShowContinueError(state, "As a result, MRT will be set to MAT for that zone"); | ||
} | ||
state.dataHeatBal->ZoneMRT(ZoneNum) = state.dataZoneTempPredictorCorrector->zoneHeatBalance(ZoneNum).MAT; | ||
} |
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 am staring at this if/else and wondering what the for loop at lines 5488 - 5495 represents when ZoneAESum(ZoneNum) <= 0.01. Same for the enclosure loop below at 5511. If ZoneAESum(ZoneNum) <= 0.01, then isn't sumAE also <= 0.01? And doesn't that mean that the for loop could be moved into this if block above line 5497? The only draw-back I see is the calculation of state.dataViewFactor->EnclRadInfo(state.dataSurface->Surface(SurfNum).RadEnclIndex).sumAET would only occur when ZoneAESum > 0.01.
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.
@rraustad Good point - no sense in accumulating sumAET if it's not going to be used, because sumAE is zero.
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.
Does this mean something still needs to be cleaned out 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.
@Myoldmopar The comment was addressed. if (state.dataHeatBalSurfMgr->ZoneAESum(ZoneNum) > 0.01) {
was moved up to line 5492, before the loop to accumulate zoneSumAET
.
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.
Excellent, thanks!
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. This is ready for final review.
\paragraph{Space Mean Radiant Temperature {[}C{]}}\label{space-mean-radiant-temperature-c} | ||
\paragraph{Enclosure Mean Radiant Temperature {[}C{]}}\label{enclosure-mean-radiant-temperature-c} |
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.
Two new output variables. Note that Zone MRT is still calculated the legacy way, oblivious to enclosure boundaries.
\key ZoneAveraged | ||
\key EnclosureAveraged |
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.
key change for People. This now uses the enclosure MRT for the associated space. No longer using zone MRT.
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.
\note Watts/Area => Watts per Zone Floor Area -- enter the number to apply. Value * Floor Area = Lights | ||
\note Watts/Area => Watts per Floor Area -- enter the number to apply. Value * Floor Area = Lights |
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 unrelated field name cleanup.
@@ -1913,7 +1916,6 @@ struct HeatBalanceData : BaseGlobalStruct | |||
Array1D<Real64> ZoneGroupSNLoadHeatRate; | |||
Array1D<Real64> ZoneGroupSNLoadCoolRate; | |||
|
|||
Array1D<Real64> ZoneMRT; // MEAN RADIANT TEMPERATURE (C) |
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 this into zoneHeatBalance.MRT (and space.....)
Real64 sumAE = 0.0; // Sum of surface area * emissivity for all surfaces in the enclosure | ||
Real64 sumAET = 0.0; // Sum of surface area * emissivity * temperature for all surfaces in the enclosure | ||
Real64 MRT = 0.0; // Mean radiant temperature of the enclosure | ||
bool reCalcMRT = false; // True when MRT needs to be recalculated |
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 enclosure fields.
src/EnergyPlus/ThermalComfort.cc
Outdated
thisAngFacList.SurfaceName(SurfNum) = state.dataIPShortCut->cAlphaArgs(SurfNum + 2); | ||
thisAngFacList.SurfaceName(SurfNum) = state.dataIPShortCut->cAlphaArgs(SurfNum + 1); |
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.
Shift fields for ComfortViewFactorAngles
case DataHeatBalance::CalcMRT::EnclosureAveraged: { | ||
int enclNum = state.dataHeatBal->space(thisPeople.spaceIndex).radiantEnclosureNum; | ||
state.dataThermalComforts->RadTemp = state.dataViewFactor->EnclRadInfo(enclNum).MRT; |
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.
Here's the change from zone to enclosure MRT for people.
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.
"So why didn't this PR just change this one line?", he asks as he runs away laughing maniacally.
tsky = state.dataHeatBal->ZoneMRT(ZoneNumAdj) + | ||
tsky = state.dataViewFactor->EnclRadInfo(enclNumAdj).MRT + |
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 enclosure MRT for interzone windows.
! So instead, let's just move forward with a new 4 character string and use that in this file and the future | ||
! If we get to version 100.1 and we are still using this Fortran transition then well....we can deal with it then | ||
sVersionNum = '***' | ||
sVersionNumFourChars='23.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.
Updated the transition code template. @Myoldmopar this might break the version update script?
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.
No it should be fine I think.
@@ -5841,6 +5846,77 @@ TEST_F(EnergyPlusFixture, HeatBalanceIntRadExchange_SetupEnclosuresWithAirBounda | |||
EXPECT_EQ(state->dataHeatBal->space(1).HTSurfaceLast, Zone1Floor); | |||
EXPECT_EQ(state->dataHeatBal->space(2).HTSurfaceLast, Zone2Floor); | |||
EXPECT_EQ(state->dataHeatBal->space(3).HTSurfaceLast, Zone3Floor); | |||
|
|||
// Check MRT calculations |
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.
Extend existing unit test to check MRT calcs. All other unit test changes are just to keep things working.
err diffs are new enclosure warning in a few files:
AUD, RDD, MTD diffs due to new output variables. eso small diffs in PMV, likely due to changes in |
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 looks great to me! There is one comment that seems to keep the door open for more changes, but I think it's for another day. I'm testing locally to confirm but expect this to be ready to merge.
\key ZoneAveraged | ||
\key EnclosureAveraged |
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.
\type object-list | ||
\object-list AllHeatTranSurfNames | ||
N99, \field Angle Factor 99 | ||
\type real | ||
\minimum 0.0 | ||
\maximum 1.0 | ||
A102, \field Surface 100 Name | ||
A101, \field Surface 100 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.
That's a lot of damage just for a field removal, but such is life!
@@ -12805,7 +12805,8 @@ namespace AirflowNetwork { | |||
Tcomfort = CurveValue(state, ComfortHighTempCurveNum, OutDryBulb); | |||
} | |||
ComfortBand = -0.0028 * (100 - MaxPPD) * (100 - MaxPPD) + 0.3419 * (100 - MaxPPD) - 6.6275; | |||
Toperative = 0.5 * (state.dataZoneTempPredictorCorrector->zoneHeatBalance(ZoneNum).MAT + state.dataHeatBal->ZoneMRT(ZoneNum)); | |||
Toperative = 0.5 * (state.dataZoneTempPredictorCorrector->zoneHeatBalance(ZoneNum).MAT + |
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.
Someday I wonder if we should rename the dataZoneTempPredictorCorrector member to something smaller. Would the increase in readability throughout the code make it worth the diffs? I think so actually. But obviously not here.
void calcMeanAirTemps(EnergyPlusData &state, | ||
Real64 const ZTAV, | ||
Real64 const airHumRatAvg, | ||
Real64 const MRT, |
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.
👍
ShowContinueError(state, "As a result, MRT will be set to MAT for that zone"); | ||
} | ||
state.dataHeatBal->ZoneMRT(ZoneNum) = state.dataZoneTempPredictorCorrector->zoneHeatBalance(ZoneNum).MAT; | ||
} |
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.
Does this mean something still needs to be cleaned out here?
case DataHeatBalance::CalcMRT::EnclosureAveraged: { | ||
int enclNum = state.dataHeatBal->space(thisPeople.spaceIndex).radiantEnclosureNum; | ||
state.dataThermalComforts->RadTemp = state.dataViewFactor->EnclRadInfo(enclNum).MRT; |
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.
"So why didn't this PR just change this one line?", he asks as he runs away laughing maniacally.
! So instead, let's just move forward with a new 4 character string and use that in this file and the future | ||
! If we get to version 100.1 and we are still using this Fortran transition then well....we can deal with it then | ||
sVersionNum = '***' | ||
sVersionNumFourChars='24.1' |
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.
...... how was this not already in there? That's .. ummm .. weird.
! So instead, let's just move forward with a new 4 character string and use that in this file and the future | ||
! If we get to version 100.1 and we are still using this Fortran transition then well....we can deal with it then | ||
sVersionNum = '***' | ||
sVersionNumFourChars='23.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.
No it should be fine I think.
|
||
# Object Change: ObjectStartsWithP | ||
[PR#10244](https://github.com/NREL/EnergyPlus/pull/10244/) |
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.
Excellent instructions here. Thanks!
This is all good locally, getting this merged now. Thanks @mjwitte ! |
Pull request overview
CalcSurfaceWeightedMRT
which was already using enclosures, but change from zone to space MAT as the fallback when the sum of surface areas*emissivities is zero.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.