Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Addition of Running Mean Outside Air Control for the Low Temperature Constant Flow Radiant System Model #8142

Merged
merged 9 commits into from
Aug 1, 2020

Conversation

RKStrand
Copy link
Contributor

@RKStrand RKStrand commented Jul 9, 2020

Pull request overview

  • Fixes Constant Flow Radiant System Control Temperature Setpoint Reset on Running Mean Outside Air Temperature #8055
  • The purpose of this pull request is the implementation of running mean outside air control for the low temperature constant flow radiant system model. This allows the constant flow radiant system to be controlled on the running average of the outside air temperature. This running average/mean is based on the history of previous days' average daily temperature. The new control also required a new input parameter that is a weighting factor which controls how much the previous day's history impacts the temperature being used to control the radiant system. This pull request includes the code to implement the enhancement, modified input files for existing test suite files, a new IDF to demonstrate that the new feature is working, a new unit test and enhancement of an existing unit test, documentation in the Input Output Reference, and transition changes. No differences in the output are expected in the modified IDFs because the new parameter is not used in those existing files. Some changes may result in the AUD, RDD, and html table files.

Pull Request Author

Add to this list or remove from it as applicable. This is a simple templated set of guidelines.

  • Title of PR should be user-synopsis style (clearly understandable in a standalone changelog context)
  • Label the PR with at least one of: Defect, Refactoring, NewFeature, Performance, and/or DoNoPublish
  • Pull requests that impact EnergyPlus code must also include unit tests to cover enhancement or defect repair
  • Author should provide a "walkthrough" of relevant code changes using a GitHub code review comment process
  • If any diffs are expected, author must demonstrate they are justified using plots and descriptions
  • If changes fix a defect, the fix should be demonstrated in plots and descriptions
  • If any defect files are updated to a more recent version, upload new versions here or on DevSupport
  • If IDD requires transition, transition source, rules, ExpandObjects, and IDFs must be updated, and add IDDChange label
  • If structural output changes, add to output rules file and add OutputChange label
  • If adding/removing any LaTeX docs or figures, update that document's CMakeLists file dependencies

Reviewer

This will not be exhaustively relevant to every PR.

  • Perform a Code Review on GitHub
  • If branch is behind develop, merge develop and build locally to check for side effects of the merge
  • If defect, verify by running develop branch and reproducing defect, then running PR and reproducing fix
  • If feature, test running new feature, try creative ways to break it
  • CI status: all green or justified
  • Check that performance is not impacted (CI Linux results include performance check)
  • Run Unit Test(s) locally
  • Check any new function arguments for performance impacts
  • Verify IDF naming conventions and styles, memos and notes and defaults
  • If new idf included, locally check the err file and other outputs

RKStrand added 5 commits June 8, 2020 15:59
Commit of the original new feature proposal for this work.
This commit includes changes needed to implement the running mean outside air temperature control in EnergyPlus.  It also includes changes to the IDD and various IDFs that use the constant flow system.  Finally, one unit test had to be modified to deal with a change in the variables sent to a subroutine.
This commit includes a variety of files to complete the implementation of the running mean outdoor air temperature control for constant flow radiant systems.  What's included in this commit: a new unit test, the expansion of an existing unit test, documentation in the IO Ref, transition changes for the new input parameter, and a new input file which shows that the control is operating as expected.
@RKStrand RKStrand 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 Jul 9, 2020
@RKStrand RKStrand requested review from Myoldmopar and mjwitte July 9, 2020 13:52
@RKStrand RKStrand self-assigned this Jul 9, 2020
Copy link
Contributor

@mjwitte mjwitte left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some initial comments/questions.

@@ -1549,6 +1553,24 @@ namespace LowTempRadiantSystem {
"System",
"Sum",
thisCFloSys.Name);
SetupOutputVariable("Constant Flow Running Mean Outdoor Air Temperature",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By itself "Constant Flow Running Mean Outdoor Air Temperature" doesn't say radiant system. I would name these three outputs as "Zone Radiant HVAC . . . ." It doesn't really matter that they can only be used for constant flow radiant.
Also, these variables should not be set up unless anyRadiantSystemUsingRunningMeanAverage = true, correct?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good. I have changed the names as you suggested (in the code, I/O Ref, and new example file) and I have also added the anyRadiantSystemUsingRunningMeanAverage check as well so this is only done if any system uses this control. Should show up in next commit.

Comment on lines 2103 to 2110
if (DataGlobals::BeginDayFlag && SystemType == ConstantFlowSystem &&
anyRadiantSystemUsingRunningMeanAverage && CFloRadSys(RadSysNum).setRunningMeanValuesAtBeginningOfDay) {
CFloRadSys(RadSysNum).calculateRunningMeanAverageTemperature();
CFloRadSys(RadSysNum).setRunningMeanValuesAtBeginningOfDay = false; // only set these once per system
} else if (!DataGlobals::BeginDayFlag && SystemType == ConstantFlowSystem &&
anyRadiantSystemUsingRunningMeanAverage && !CFloRadSys(RadSysNum).setRunningMeanValuesAtBeginningOfDay) {
CFloRadSys(RadSysNum).setRunningMeanValuesAtBeginningOfDay = true; // reset so that the next time BeginDayFlag is true this can get set
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like this block belongs up a few lines inside the main if (SystemType == ConstantFlowSystem) block.
Also, checking anyRadiantSystemUsingRunningMeanAverage seems redundant. CFloRadSys(RadSysNum).setRunningMeanValuesAtBeginningOfDay will always be false if anyRadiantSystemUsingRunningMeanAverage is false. Or perhaps better would be to wrap this entire block in if anyRadiantSystemUsingRunningMeanAverage so it's skipped entirely if not in use.

Also, it appears that the else if here will fire for every CFloRadSys , even ones that aren't using RunningMeanOutdoorAirTemperature?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup, agree. I've simplified this as you requested--moved it to inside the if (SystemType == ConstantFlowSystem) block and made a if (anyRadiantSystemUsingRunningMeanAverage) block. Should show up in the next commit.

Comment on lines 4751 to 4753
//NOTES: ALPHA COULD BE DIFFERENT FOR EACH SYSTEM BECAUSE IT WILL BE A FACTOR OF EACH SYSTEM--NEED TO CHANGE LOCATION OF SOME OF THESE VARIABLES
// AS A RESULT (RUNNING MEAN TEMPERATURES). alpha WILL NEED TO BE REPLACED BY APPROPRIATE TERM IN CONSTANT FLOW DATA AND STILL NEED TO READ
// THIS VALUE IN IN THE GET ROUTINE (AND MODIFY IDD, INPUT FILES, TRANSITION, ETC.)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this comment still relevant?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yah, that was supposed to get deleted. It was just a temporary note to myself while coding. It's gone now. Change will show up in the next commit. Thanks for catching that!

Comment on lines 4764 to 4765
for (int hourNumber = 1; hourNumber <= DataGlobals::HoursInDay; ++hourNumber) {
sum += WeatherManager::TodayOutDryBulbTemp(firstTimeStepIndex,hourNumber);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, the average is only using values for the first timestep in each hour? If the epw file is hourly, these won't match the epw values (those appear at the last timestep of each hour). If the epw file is sub-hourly, is this ok?
Also, so this looks ahead at the full day? Isn't that beyond what a real control system could do, or do they use predicted weather to control the actual system? I must be missing something here.

From the I/O Ref description, it seems like this should be averaging yesterday's temperatures, no Today's.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you check where the temperature is used, it's actually using yesterday's temperature. I'm calculating "today's" after moving today's temperature to the yesterday value at the start of each day. So, yes, I'm calculating today's value but it never gets used until the start of the next day when the yesterday value is then set. Does that make sense now?

As for the averaging, my bad--I can switch it to the last time step in the hour. I'll look at whether or not there is something that indicates whether the weather values are hourly or sub hourly. I believe that most standard (available from the E+ web site) are hourly values. As you can see, my understanding of the weather stuff is not complete, but I'll dig a bit more and make whatever change I can make. Or in reality, I guess I could just average the time step values since they have been "averaged" by the program anyway?

Changes should show up in the next commit.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK. A comment here to explain would be helpful. And regarding the first/last timestep of the hour - you need to double-check my understanding. It's can be a bit messy. And then there's the case of sub-hourly weather files. So, in order to keep this simple, why not just average at each timestep instead of hourly?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I'll add a comment as to why I'm calculating today and how it's used.

As for the averaging, yes, I can and will do it on the time step. When I started the work, I thought it was loading it into the first time step. Now, I'm not so sure and it doesn't really matter since we should allow for sub-hourly weather either way. So, I'll implement a time step averaging scheme and move forward with that.

@@ -44192,8 +44192,17 @@ ZoneHVAC:LowTemperatureRadiant:ConstantFlow,
\key OutdoorWetBulbTemperature
\key SurfaceFaceTemperature
\key SurfaceInteriorTemperature
\key RunningMeanOutdoorAirTemperature
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be RunningMeanOutdoorDryBulbTemperature to be more specific? Especially since OutdoorDryBulbTemperature and OutdoorWetBulbTemperature are also choices here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will fix this--should show up in the next commit.

\end{itemize}

\emph{Operative temperature} for radiant system controls is the average of Mean Air Temperature and Mean Radiant Temperature. The \emph{SurfaceFaceTemperature} option allows the user to control the radiant system using the inside face surface temperature of the radiant system (the inside surface temperature). The \emph{SurfaceInteriorTemperature} option will allow the user to control the radiant system using a surface temperature that is calculated inside the radiant system. This point will be defined by the \hyperref[constructioninternalsource]{Construction:InternalSource} description for the system. In that input, the user has the option to calculate a temperature at a particular point in the construction. The radiant system will then use this information for controlling the slab not just producing temperatures for outputting at that point. Users should consult the input for Field: Temperature Calculation Requested After Layer Number for more information. Note that for the SurfaceFaceTemperature and SurfaceInteriorTemperature the surface being used for control is the surface listed above in the field for Surface Name. If the user enters a group of surfaces for that input, the \emph{first} surface in the radiant group is the surface for control purposes.
\emph{Operative temperature} for radiant system controls is the average of Mean Air Temperature and Mean Radiant Temperature. The \emph{SurfaceFaceTemperature} option allows the user to control the radiant system using the inside face surface temperature of the radiant system (the inside surface temperature). The \emph{SurfaceInteriorTemperature} option will allow the user to control the radiant system using a surface temperature that is calculated inside the radiant system. This point will be defined by the \hyperref[constructioninternalsource]{Construction:InternalSource} description for the system. In that input, the user has the option to calculate a temperature at a particular point in the construction. The radiant system will then use this information for controlling the slab not just producing temperatures for outputting at that point. Users should consult the input for Field: Temperature Calculation Requested After Layer Number for more information. Note that for the SurfaceFaceTemperature and SurfaceInteriorTemperature the surface being used for control is the surface listed above in the field for Surface Name. If the user enters a group of surfaces for that input, the \emph{first} surface in the radiant group is the surface for control purposes. The \emph{RunningMeanOutdoorAirTemperature} option will allow the user to control the inlet water temperature to the system as a function of the running mean outdoor air temperature. The next field (\hyperref[field-running-mean-outdoor-air-temperature-weighting-factor]{Running Mean Outdoor Air Temperature Weighting Factor}) will define how this temperature is actually calculated within EnergyPlus.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given the long list of control types, this would be more readable using the {itemize} style (same as Condensation Control Type).
And this should make it clear that it's averaging outdoor dry bulb temperature. Same for the next field.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will fix this--should show up in the next commit.

RKStrand added 2 commits July 27, 2020 14:57
This commit addresses the first round of comments from @mjwitte regarding this pull request.
@RKStrand
Copy link
Contributor Author

@mjwitte Ok, I think I have fixed all of the things that you pointed out. I have committed those changes and then in a separate commit I merged in develop. Should be ready for another look assuming all of the checks come back ok.

Copy link
Contributor

@mjwitte mjwitte left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested the new idf with an annual run, as-is (alpha=0.8), and with alpha = 1 and 0. No warnings, so that's clean. With alpha=1, running mean is constant all year (not useful, but as expected).

But with alpha=0, I expected "Zone Radiant HVAC Running Mean Outdoor Dry Bulb Temperature" to equal "Zone Radiant HVAC Previous Day Average Outdoor Dry Bulb Temperature" but it does not. Am I missing something here?

\item
SurfaceInteriorTemperature
RunningMeanOutdoorDryBulbTemperature - This option will allow the user to control the inlet water temperature to the system as a function of the running mean outdoor air temperature. The next field (\hyperref[field-running-mean-outdoor-dry-bulb-temperature-weighting-factor]{Running Mean Outdoor Dry Bulb Temperature Weighting Factor}) will define how this temperature is actually calculated within EnergyPlus.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So much more readable.

If the user does not select a control type, \textbf{MeanAirTemperature} control is assumed by EnergyPlus. See the throttling range and control temperature schedule fields below for more information on how the setpoint temperature is established for this particular radiant system.

\paragraph{Field: Running Mean Outdoor Dry Bulb Temperature Weighting Factor}\label{field-running-mean-outdoor-dry-bulb-temperature-weighting-factor}

This field specifies the weighting factor that is used to calculate the running mean outdoor dry bulb temperature. The running mean outdoor dry bulb temperature is determined using the following equation:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit-picking - if there are other changes necessary here, all occurrences of "dry bulb" should be "dry-bulb". And technically the field name should be ... Outdoor Dry-Bulb Temperature ... for consistency with most other field names (sorry I didn't think of that before).

@@ -1487,6 +1505,12 @@ \subsubsection{Outputs}\label{outputs-6-007}
HVAC,Sum,Zone Radiant HVAC Cooling Fluid Heat Transfer Energy {[}J{]}
\item
HVAC,Sum,Zone Radiant HVAC Heating Fluid Heat Transfer Energy {[}J{]}
\item
HVAC,Average,Constant Flow Running Mean Outdoor Dry Bulb Temperature {[}C{]}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And for output variables, for whatever reason the current pattern is "Drybulb Temperature" (which is ugle), except for the ITE outputs which use "Dry-Bulb" (hmmm). (Searching "C:\EnergyPlusV9-3-0\SetupOutputVariables.csv") e.g.
"Surface Outside Face Outdoor Air Drybulb Temperature".

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🙄Gotta love the consistency in E+😏 So, if I do end up making changes, what are you requesting: Drybulb or Dry-Bulb for the output variables?

Copy link
Contributor

@mjwitte mjwitte Jul 28, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We try, but apparently this hasn't been covered well. I think Dry-Bulb is ugly in the output variable names, even if it's our supposed rule, and I'm the one that used it in the ITE. And I think Drybulb is ugly, too, but it's been around a long time. Anyone else have an opinion? @Myoldmopar @rraustad

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dry-Bulb, Wet-Bulb and Dew Point are the identifier for the word temperature (just as is Radiant, Ground, Water, Face, etc.). The hyphen or space is not always used (e.g., Dry Bulb or Drybulb).

@RKStrand
Copy link
Contributor Author

RMTAlpha0.xlsx
I think it is actually working properly. I've attached the file above to show only the time stamp, outside DB, and the three running mean temperature variables. I've focused on the zone where I changed the coefficient to 0.0. If you look at the pattern, the design days nothing changes as expected. For the "annual runs" that go for about a week in winter and one in summer, the pattern is also as expected. The winter week starts on 1/7 so the data on 1/7 and 1/8 should be the same since the previous day is the data for 1/7 in both cases. However, when the weather for the previous day for 1/9 is the data for 1/8, you see the temperature change for both the previous day's average temperature and the new running mean temperature which is equal to the previous day's average temperature as expected. This pattern also holds for the summer week as well--first two days stay the same and then things start changing on 7/9 because the previous day's weather has now changed since we are out of warmup and have passed to the third day.
So, I think this is operating correctly. Does this explain things or are you seeing something different on your machine?
Also, I agree with you--changing the coefficient to 1.0 does not seem very meaningful because then nothing ever changes.

@mjwitte
Copy link
Contributor

mjwitte commented Jul 28, 2020

Arrgh - never mind - I was looking at the wrong zone's running mean. Works perfectly here, too.

@RKStrand
Copy link
Contributor Author

@mjwitte Oh whew! I was a bit nervous that something was wrong but I'm glad that it's operating as expected. 😀

RKStrand added 2 commits July 29, 2020 15:27
Corrected various places where "dry-bulb" was written as "dry bulb".  Corrections were needed in IO Ref, new code, new input file, new input, and new output.
Copy link
Contributor

@mjwitte mjwitte left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dry-bulb conversion complete (thanks @RKStrand!). Tested transition. Tested new idf with variations. Reviewed I/O Ref changes.

Table diffs are "Number of fields with defaults" because there's a new field in ZoneHVAC:LowTemperatureRadiant:ConstantFlow.

All looks good. Will wait for linux and windows CI to come back and finish up before merging.

\default MeanAirTemperature
N3 , \field Rated Flow Rate
N3 , \field Running Mean Outdoor Dry-Bulb Temperature Weighting Factor
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahh, so pretty now.

@mjwitte
Copy link
Contributor

mjwitte commented Aug 1, 2020

This is good. Diffs on linux are due to develop moving with merge of #8177. Merging.

@mjwitte mjwitte merged commit 922f888 into develop Aug 1, 2020
@mjwitte mjwitte deleted the 8055-RunningMeanOutsideAirTempCtrlConstFlowRadSys branch August 10, 2020 15:49
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
IDDChange Code changes impact the IDD file (cannot be merged after IO freeze) NewFeature Includes code to add a new feature to EnergyPlus
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Constant Flow Radiant System Control Temperature Setpoint Reset on Running Mean Outside Air Temperature
8 participants