-
Notifications
You must be signed in to change notification settings - Fork 378
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
Add output wetbulb globe temperature in the csv output #10506
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.
@yujiex PsyTwbFnTdbWPb
is an iterative calculation, so we try to avoid calculating it unless it's requested as an output variable or used as an EMS sensor. See NodeInputManager::CalcMoreNodeInfo
for how this is managed for certain node outputs. It would be good to add a zone-level flag for this new output to skip the calculation if the variable has not been requested or used in EMS.
@@ -4928,6 +4928,9 @@ void calcMeanAirTemps(EnergyPlusData &state, | |||
thisAirRpt.MeanAirTemp = ZTAV; | |||
thisAirRpt.MeanAirHumRat = airHumRatAvg; | |||
thisAirRpt.OperativeTemp = 0.5 * (ZTAV + MRT); | |||
thisAirRpt.WetbulbTemp = Psychrometrics::PsyTwbFnTdbWPb(state, ZTAV, airHumRatAvg, state.dataEnvrn->OutBaroPress); | |||
thisAirRpt.WetbulbGlobeTemp = | |||
0.7 * Psychrometrics::PsyTwbFnTdbWPb(state, ZTAV, airHumRatAvg, state.dataEnvrn->OutBaroPress) + 0.3 * thisAirRpt.OperativeTemp; |
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.
Several lines below here, there is a block which recalculates thisAirRpt.OperativeTemp
if operative temperature control is being used. Should the WetbulbGlobeTemp
happen after that to be consistent?
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.
Definitely, it should be after the thisAirRpt.OperativeTemp has been recalculated. Thanks for noticing this.
I will add a zone-level flag so that the calculation is only triggered if an output:variable is requested for this new wetbulb glob temperature variable. |
The zone reporting flag will make sure the calculation only happens when the variable is requested The WBGT depends on the operative temperature (OT), so its calculation is moved to after OT obtains the most updated value.
only calculate WBGT when it is requested as a senser by EMS
@@ -4902,6 +4903,22 @@ void ReportZoneMeanAirTemp(EnergyPlusData &state) | |||
// PURPOSE OF THIS SUBROUTINE: | |||
// This subroutine updates the report variables for the AirHeatBalance. | |||
|
|||
for (int ZoneLoop = 1; ZoneLoop <= state.dataGlobal->NumOfZones; ++ZoneLoop) { |
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 good, but it needs to be protected by a oneTime variable so these loops don't execute every timestep.
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.
Oh make sense. I will add the oneTime flag.
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.
added
so that the reporting flag calculation only happens once
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.
@yujiex Sorry, I didn't think of this earlier, but this output variable will be generated for zones and for spaces (if state.dataHeatBal->doSpaceHeatBalanceSimulation
is true), see ReportZoneMeanAirTemp
).
@@ -4902,6 +4903,25 @@ void ReportZoneMeanAirTemp(EnergyPlusData &state) | |||
// PURPOSE OF THIS SUBROUTINE: | |||
// This subroutine updates the report variables for the AirHeatBalance. | |||
|
|||
if (state.dataHeatBalAirMgr->CalcExtraReportVarMyOneTimeFlag) { |
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.
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.
This is eerily similar to node wet-bulb temp. Shouldn't this be done over in NodeInputManager? and globe temp recorded as a node condition? Then these AirRpt variables could pick up data off the node.
for (auto const *reqVar : state.dataOutputProcessor->reqVars) {
if (Util::SameString(reqVar->key, state.dataLoopNodes->NodeID(iNode)) || reqVar->key.empty()) {
if (Util::SameString(reqVar->name, "System Node Wetbulb Temperature")) {
state.dataNodeInputMgr->NodeWetBulbRepReq(iNode) = true;
NodeWetBulbSchedPtr(iNode) = reqVar->SchedPtr;
} else if (Util::SameString(reqVar->name, "System Node Relative Humidity")) {
NodeRelHumidityRepReq(iNode) = true;
NodeRelHumiditySchedPtr(iNode) = reqVar->SchedPtr;
} else if (Util::SameString(reqVar->name, "System Node Dewpoint Temperature")) {
NodeDewPointRepReq(iNode) = true;
NodeDewPointSchedPtr(iNode) = reqVar->SchedPtr;
} else if (Util::SameString(reqVar->name, "System Node Specific Heat")) {
NodeSpecificHeatRepReq(iNode) = true;
NodeSpecificHeatSchedPtr(iNode) = reqVar->SchedPtr;
}
}
}
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.
Is there one node in each space/zone? I didn't find out how to get the node index from space or zone index
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 this mapping can be done relatively straightforward, then maybe I should calculate it at the nodes above and retrieve them during the reporting.
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'm not sure you want to make any big changes here. There is already Zone/Space Mean Air Temperature
and these changes mimic that report variable. I am just pointing out that there may be a cleaner way to do this. If this PR is clean and gets the job done then that is good enough for now. A new branch could be used to test using the node condition as the basis for these AirRpt variables.
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 see. I will test it out with some other test branches computing the node condition as the basis for the AirRpt variables.
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.
One issue here is that node conditions are updated at the system timestep and these zone values are the averages at the zone timestep. So I'm not sure it's worth pursuing this.
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.
Make sense. Maybe we can keep it as this and not calculate at the node level, otherwise, I assume there also needs to be some temporal aggregation steps in addition to node-to-zone/space matching.
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.
Yes, keep it like this for now.
if (state.dataHeatBalAirMgr->CalcExtraReportVarMyOneTimeFlag) { | ||
for (int ZoneLoop = 1; ZoneLoop <= state.dataGlobal->NumOfZones; ++ZoneLoop) { | ||
auto &thisZone = state.dataHeatBal->Zone(ZoneLoop); | ||
for (auto const *reqVar : state.dataOutputProcessor->reqVars) { |
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 this use case, I think it's better to make reqVars
the outer loop and check first for name="Zone Wetbulb Globe . . ." only first. Then if key.empty()
set ReportWBGT
for all zone (and spaces). If it's not empty, you can use FindItemInList
to find the zone or space number and then set ReportWBGT
for that single instance.
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 way is better. I will loop through the reqVars
first
auto &thisZone = state.dataHeatBal->Zone(ZoneLoop); | ||
for (auto const *reqVar : state.dataOutputProcessor->reqVars) { | ||
if (Util::SameString(reqVar->key, thisZone.Name) || reqVar->key.empty()) { | ||
if (Util::SameString(reqVar->name, "Zone Wetbulb Globe Temperature")) { |
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 (reqVar->name == "ZONE WETBULB GLOBE TEMPERATURE")) {
name
is already uppercase, so no need to use SameString
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.
indeed
} | ||
for (int loop = 1; loop <= state.dataRuntimeLang->NumSensors; ++loop) { | ||
if (state.dataRuntimeLang->Sensor(loop).UniqueKeyName == thisZone.Name && | ||
Util::SameString(state.dataRuntimeLang->Sensor(loop).OutputVarName, "Zone Wetbulb Globe Temperature")) { |
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 I'm not sure about the case of sensor OutputVarName
so probably keep SameString
.
auto const &thisZone = state.dataHeatBal->Zone(zoneNum); | ||
if (thisZone.ReportWBGT) { |
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 you move ReportWBGT
into thisAirRpt
(state.dataHeatBal->ZnAirRpt
and spaceAirRpt
) then it will work for both 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.
good idea. I'll move it to be one of the fields in thisAirRpt
Thanks for noticing this. I will add another space layer in the |
change loop order: *reqVar moved to the outer loop check first for variable name="Zone Wetbulb Globe . . ." Then if key.empty() set ReportWBGT for all zone (and spaces). If it's not empty, use FindItemInList to find the zone or space number and then set ReportWBGT for that single instance. reqVar->name changed to use "==" to check equality since it's upper case ReportWBGT moved into thisAirRpt, so it can work for both ZnAirRpt and spaceAirRpt
} else { | ||
auto &thisZnAirRpt = state.dataHeatBal->ZnAirRpt(ZoneLoop); | ||
thisZnAirRpt.ReportWBGT = 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 should not be an else
. If doSpaceHeatBalanceSimulation
is true, then there will be values for both zones and spaces.
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 see. I'll remove the else
.
} | ||
} else { | ||
if (state.dataHeatBal->doSpaceHeatBalanceSimulation) { | ||
int spaceNum = Util::FindItemInList(reqVar->name, 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.
These need to search on reqVar->key
, not name
. If you run a test file with a zone name in the output variable object, it won't work here. And this needs to test for spaceNum > 0
to prevent a subscript error. Same comments apply to zone below.
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'll search the "key" field here for the space and zone name instead of the "name" field, and add a check for the space and zone index > 0.
} else { | ||
int ZoneLoop = Util::FindItemInList(reqVar->name, 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.
Again, this shouldn't be an else
.
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.
indeed
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.
Although here there should probably still be a separation based on state.dataHeatBal->doSpaceHeatBalanceSimulation
T/F values, as when the spaceNum is found, the corresponding zone index containing this space will come from the zoneNum
field of the space. When there's no space level simulations, a FindItemInList
will be used to find the zone index
src/EnergyPlus/DataHeatBalance.hh
Outdated
@@ -1475,6 +1477,8 @@ namespace DataHeatBalance { | |||
Real64 OABalanceFanElec = 0.0; // Fan Electricity {W} due to OA air balance | |||
Real64 SumEnthalpyM = 0.0; // Zone sum of EnthalpyM | |||
Real64 SumEnthalpyH = 0.0; // Zone sum of EnthalpyH | |||
// reporting flags | |||
bool ReportWBGT = true; // whether the wetbulb globe temperature is reqeusted as an output variable or used as an EMS sensor |
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.
Sorry - it's this flag that should be initialized to false.
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.
Oh! right right. This should be false.
zone will report the variable when itself is requested and when its subspace is requested
when a specific space is requested to report WBGT, only that space is reported, not its parent zone. when no specific zone or space is requested, then Output:Variable,*,Zone Wetbulb Globe Temperature,hourly; -> report all zones Output:Variable,*,Space Wetbulb Globe Temperature,hourly;-> report all spaces
set up a Wetbulb Globe Temperature EMSInternalVariable for zone and space
std::string const &spaceName = state.dataHeatBal->space(spaceNum).Name; | ||
auto &thisSpaceAirRpt = state.dataHeatBal->spaceAirRpt(spaceNum); | ||
SetupEMSInternalVariable(state, "Space Wetbulb Globe Temperature", spaceName, "[C]", thisSpaceAirRpt.WetbulbGlobeTemp); |
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 not needed. Since the new WBGT is an output:variable is can be accessed with EMS:Sensor. The internal variable setup is to provide EMS access to values that are not time series data such as sizing results. See EMS Applicatoin Guide "Internal Variables" section.
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 I don't add this, it doesn't show up in the edd file. Is that expected?
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 I don't add this, it doesn't show up in the edd file. Is that expected?
Correct. The EDD will list available actuators and internal variables only. Everything in the RDD is available to use as an EMS sensor.
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.
Thanks for the clarification. I will remove the SetupEMSInternalVariables.
@@ -373,6 +377,8 @@ void GetSimpleAirModelInputs(EnergyPlusData &state, bool &ErrorsFound) // IF err | |||
OutputProcessor::TimeStepType::System, | |||
OutputProcessor::StoreType::Average, | |||
name); | |||
|
|||
SetupEMSInternalVariable(state, "Zone Wetbulb Globe Temperature", name, "[C]", thisZnAirRpt.WetbulbGlobeTemp); |
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 comment here, not needed.
if (reqVar->name == "ZONE WETBULB GLOBE TEMPERATURE" || reqVar->name == "SPACE WETBULB GLOBE TEMPERATURE") { | ||
if (reqVar->key.empty()) { | ||
for (int ZoneLoop = 1; ZoneLoop <= state.dataGlobal->NumOfZones; ++ZoneLoop) { | ||
if (state.dataHeatBal->doSpaceHeatBalanceSimulation) { | ||
for (int spaceNum : state.dataHeatBal->Zone(ZoneLoop).spaceIndexes) { | ||
auto &thisSpaceAirRpt = state.dataHeatBal->spaceAirRpt(spaceNum); | ||
thisSpaceAirRpt.ReportWBGT = true; | ||
} | ||
} | ||
auto &thisZnAirRpt = state.dataHeatBal->ZnAirRpt(ZoneLoop); | ||
thisZnAirRpt.ReportWBGT = 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.
Sorry I've made zone vs space so confusing. This code here will set ReportWBGT=true
for all zones and spaces if either "Zone Wetbulb..." or "Space Webulb..." is requested. The structure should be more like:
if (reqVar->name == "ZONE WETBULB GLOBE TEMPERATURE"){
if (reqVar->key.empty()) {
set true for all zones
else
set for matching zone name.
}
}
if (state.dataHeatBal->doSpaceHeatBalanceSimulation) {
if (reqVar->name == "SPACE WETBULB GLOBE TEMPERATURE"){
if (reqVar->key.empty()) {
set true for all spaces
else
set for matching 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.
I see I see. Cleaner this way. I had zones and space nested together earlier. I will change it to this format.
if (state.dataRuntimeLang->Sensor(loop).OutputVarName == "ZONE WETBULB GLOBE TEMPERATURE" || | ||
state.dataRuntimeLang->Sensor(loop).OutputVarName == "SPACE WETBULB GLOBE TEMPERATURE") { |
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.
Similar comment here. If Zone then search for matching zone name, if Space, then search for matching 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.
I will change it to separate zone and space checks like the pattern you showed above
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 to make sure, I don't need to check state.dataRuntimeLang->Sensor(loop).UniqueKeyName.empty()
right? They're guaranteed to be non-empty (otherwise it will err with "Unique Key Name not found.") @mjwitte
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 to make sure, I don't need to check
state.dataRuntimeLang->Sensor(loop).UniqueKeyName.empty()
right? They're guaranteed to be non-empty (otherwise it will err with "Unique Key Name not found.") @mjwitte
Correct.
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.
Thanks
Make the zone and space checks into the following pattern if (reqVar->name == "ZONE WETBULB GLOBE TEMPERATURE"){ if (reqVar->key.empty()) { set true for all zones else set for matching zone name. } } if (state.dataHeatBal->doSpaceHeatBalanceSimulation) { if (reqVar->name == "SPACE WETBULB GLOBE TEMPERATURE"){ if (reqVar->key.empty()) { set true for all spaces else set for matching 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.
@yujiex This is getting close. See comments below, update with develop, and remove and temporary changes (e.g. Zone Wetbulb Temperature).
} | ||
} | ||
if (state.dataHeatBal->doSpaceHeatBalanceSimulation) { | ||
if (reqVar->key.empty()) { |
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.
Almost there. Before this line:
if (reqVar->name == "SPACE WETBULB GLOBE TEMPERATURE") {
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.
Oh I forgot this space var check. Good catch
} | ||
} | ||
} | ||
if (state.dataHeatBal->doSpaceHeatBalanceSimulation) { |
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 can be else if
.
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.
right. it should not check again if reqVar->name
is already "ZONE WETBULB GLOBE TEMPERATURE"
thisZnAirRpt.ReportWBGT = true; | ||
} | ||
} | ||
if (state.dataHeatBal->doSpaceHeatBalanceSimulation) { |
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.
else if
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'll change to else if here
|
||
### Adding an Output:Variable, Wetbulb Globe Temperature | ||
|
||
An output variable will be added, Wetbulb Globe Temperature. |
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 should list both flavors, Zone Wetbulb Globe Temperature and 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'll add the space version too
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.
@yujiex This looks good, tested with various combinations of output variable and EMS sensor inputs. After you remove the Zone/Space Wetbulb Temperature from code and idfs, this will be ready to merge.
Wow. This has been run through the wringer! I'm not going to add more here, if it looks good at any point, merge away @mjwitte |
Pull request overview
introduction
This PR adds an Output:Variable, Wetbulb Globe Temperature, to the zone air report. Its calculation is as follows
Wetbulb Globe Temperature = 0.7 * zone wetbulb temperature + 0.3 * zone operative temperature
The attached file is a sample output file with the Wetbulb Globe Temperature added.
eplusout.csv
regression diffs
For example, in 1ZoneUncontrolled example file,
NumOfRVariable
increased by 2 as Wetbulb Globe Temperature and Wetbulb Temperature are added. TheNumConsideredOutputVariables
andNumOfRVariable(Actual)
increases by 3 as 3 more Output:Variable are added to the idf.in 1ZoneUncontrolled the "Meters for xx" index increased by 3 as 3 output:variables are requested for 1 zone; in 5ZoneAirCooledWithSpacesHVAC, the "Meters for xx" index increased by 14 as 2 outputs are requested for all 6 zones and two outputs are requested for one space
The following shows the added outputs in 5ZoneAirCooledWithSpacesHVAC.idf
NOTE: ENHANCEMENTS MUST FOLLOW A SUBMISSION PROCESS INCLUDING A FEATURE PROPOSAL AND DESIGN DOCUMENT PRIOR TO SUBMITTING CODE
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.