Addition of Standard Effective Temperature (SET) as an Output Variable #7431

RKStrand · 2019-07-30T13:44:14Z

Pull request overview

Based on discussion on unmethours.com, there was a request to add Standard Effective Temperature (SET) as an output variable. This is calculated as part of the Pierce two-node thermal comfort model. This work fulfills this request and it is now possible to select SET as an output variable in EnergyPlus. This includes code changes, a unit test, documentation updates, and a modified test suite input file to show the use of this new output variable.

Work Checklist

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)
At least one of the following appropriate labels must be added to this PR to be consumed into the changelog:
- Defect: This pull request repairs a github defect issue Standard Effective Temperature (SET) outputs from Energyplus not available #7174

Review Checklist

This will not be exhaustively relevant to every PR.

Functional code review (it has to work!)
If defect, results of running current develop vs this branch should exhibit the fix
CI status: all green or justified
Performance: CI Linux results include performance check
Unit Test(s)
C++ checks:
- Argument types
- If any virtual classes, ensure virtual destructor included, other things
IDD changes n/a
Required documentation updates?

This commit contains the code necessary to produce the SET variable for the Pierce Two-Node Thermal Comfort model. If uses wish to have SET for their run, they simply need to select Pierce as an option in the People statement and then add syntax to produce the Zone Thermal Comfort Pierce Model Standard Effective Temperature output variable.

This commit includes a new unit test for this work, documentation changes (I/O Ref) to describe the new output variable, and some other minor code cleanups.

Addition of syntax to an input file to show the new output variable.

mjwitte · 2019-08-22T19:35:41Z

tst/EnergyPlus/unit/ThermalComfort.unit.cc

+    OutputProcessor::TimeValue.allocate(2);
+    DataGlobals::DDOnlySimulation = true;
+
+    ManageSimulation();


Would prefer a much more lightweight unit test. Seems you could just have zone, people, and schedules in the idf snippet and then just call the functions to process zone and people data and then CalcThermalComfortPierce.

Moderately important. This should get your schedule in place, if done before processing the people:

ScheduleManager::ProcessScheduleInput(); ScheduleManager::ScheduleInputProcessed = true;

@mjwitte Ok, it's now much more lightweight. Doesn't call anything except the Pierce subroutine. Sets every manually in the unit test now. This should be up in GitHub as of now.

mjwitte · 2019-08-22T19:37:41Z

src/EnergyPlus/ThermalComfort.cc

@@ -467,6 +467,12 @@ namespace ThermalComfort {
                                    "Zone",
                                    "State",
                                    People(Loop).Name);
+                SetupOutputVariable("Zone Thermal Comfort Pierce Model Standard Effective Temperature",
+                                    OutputProcessor::Unit::None,


OutputProcessor::Unit::C ??

mjwitte · 2019-08-22T19:54:11Z

tst/EnergyPlus/unit/ThermalComfort.unit.cc

+
+    ASSERT_TRUE(process_idf(idf_objects));
+
+    OutputProcessor::TimeValue.allocate(2);


And when you merge in develop, this line will fail. Simply delete it.

Will delete it.

Two modifications were requested during the review process. One was a simple correction of the units for this new variable. The other was a revision (rewriting) of the unit test to make it more efficient.

For some reason, this did not commit properly the first time?

rraustad · 2019-08-28T02:42:11Z

New report variable added but no changes to table reports.

…4-AddSETOutput

rraustad · 2019-08-28T17:15:45Z

Final code and unit test review, removed clear_state access from unit test
Compiled and reviewed IO Ref., small edits, no new figures to add to CMake.
Ran sequential unit tests in release mode without failure
Ran example file, no warnings in err file. Same for annual run.
Verified new report variable added to existing example file (TermReheatSurfTC.idf)
Pulled develop and uploaded final changes
Plotted new report variable for design days

rraustad

Minor editorial mods to IO Ref, correct unit test, uploaded changes.

rraustad · 2019-08-28T17:17:31Z

This is ready to merge once CI has a chance to process changes.

rraustad · 2019-08-28T17:22:32Z

@RKStrand can you comment on the SET being that much above the summer design day zone temp in the figure above (e.g., occupants have on too many clothes, etc.)? The summer zone humidity ratio is actually lower than winter and zone temp is not that much different. No other code changes will be requested, just wondering if this is working as expected (i.e., if new issue should be opened).

RKStrand · 2019-08-28T19:38:22Z

@rraustad I see what you are saying in the results, but I think this is actually what one would expect. SET includes both radiative and convective aspects as well as a bunch of other things. I would expect that SET would exhibit somewhat similar behavior to MAT but the surface temperatures in the space will dampen the response and make SET more closely related to outside air temperature (since that will in turn affect inside surface temperatures). So, I think that what you are seeing is actually correct--SET is lower than MAT for the most part for winter (because MRT is lower as a result of the cold exterior conditions) and SET is generally higher than MAT for summer. The "heavy" surfaces are damping the response of SET so when MAT changes rapidly like it does at the end of the work day in the summer data MAT can flip so that it is higher than SET because MAT in that situation is actually higher temporarily for that time. Basically, if one were to graph MAT, MRT, and SET, I would guess that in general SET should track between these two for the most part. Hopefully that makes sense, but if it doesn't, please let me know. Overall, I think what you are seeing is the right trend and I don't think there is a problem here that needs to be logged as a defect. Thanks for asking and for diligently checking results to see if there are additional problems!

rraustad · 2019-08-28T20:01:07Z

Followup graph based on results discussion.

rraustad · 2019-08-30T14:48:13Z

I can't think of a reason why this should not merge except for a complete CI assessment...

RKStrand · 2019-08-30T14:58:58Z

@Myoldmopar This has met with approval from @rraustad but it sounds like there wasn't a complete CI result? Does it need to be relaunched or something?

Myoldmopar · 2019-08-30T15:09:22Z

The CI machine itself needed to be relaunched. And it has been done. CI results should be coming in soon.

rraustad · 2019-08-30T17:28:46Z

Diff's from Coil Sizing report fixes #7435 are creeping in here. Merged that one into develop 17 hrs ago.

rraustad · 2019-08-30T17:32:17Z

Well, I was just going to merge this anyway since I understood the diff's. But it can't hurt to have a fresh clean CI result.

RKStrand · 2019-08-30T17:36:11Z

@rraustad Oops--sorry to get in the way. Should have left it alone. But as you pointed out, a fresh CI can't (shouldn't) hurt.

Myoldmopar · 2019-08-30T17:39:51Z

We've had an issue with the CI box and had to take it down. I'm working to get some extra support stood back up but this is likely to need to wait until after the weekend.

rraustad · 2019-08-31T16:13:37Z

CI machines now match with expected results.

RKStrand added 4 commits July 29, 2019 14:01

Unit test, doc changes, and minor edits

578a7d9

This commit includes a new unit test for this work, documentation changes (I/O Ref) to describe the new output variable, and some other minor code cleanups.

Merge branch 'develop' into 7174-AddSETOutput

0dd2603

Input file changes to reflect new output variable

72b1efa

Addition of syntax to an input file to show the new output variable.

RKStrand requested a review from Nigusse July 30, 2019 13:44

RKStrand self-assigned this Jul 30, 2019

RKStrand added Defect Includes code to repair a defect in EnergyPlus EnergyPlus labels Jul 30, 2019

mjwitte added this to the EnergyPlus 9.2.0 milestone Aug 5, 2019

mjwitte reviewed Aug 22, 2019

View reviewed changes

RKStrand added 3 commits August 22, 2019 15:35

Merge branch 'develop' into 7174-AddSETOutput

4575d44

Code revisions in response to review comments

d3602d5

Two modifications were requested during the review process. One was a simple correction of the units for this new variable. The other was a revision (rewriting) of the unit test to make it more efficient.

Missing commit changes

439a4f0

For some reason, this did not commit properly the first time?

rraustad added the OutputChange Code changes output in such a way that it cannot be merged after IO freeze label Aug 28, 2019

mjwitte added NewFeature Includes code to add a new feature to EnergyPlus and removed Defect Includes code to repair a defect in EnergyPlus EnergyPlus OutputChange Code changes output in such a way that it cannot be merged after IO freeze labels Aug 28, 2019

rraustad added 2 commits August 28, 2019 13:11

Remove unit test clear_state, update IO Ref.

09ee3b4

Merge branch 'develop' of https://github.com/NREL/EnergyPlus into 717…

6fec4b2

…4-AddSETOutput

rraustad approved these changes Aug 28, 2019

View reviewed changes

Merge branch 'develop' into 7174-AddSETOutput

4aae846

RKStrand added 2 commits August 30, 2019 12:30

Merge branch 'develop' into 7174-AddSETOutput

93f8d20

Merge branch 'develop' into 7174-AddSETOutput

8a5217b

rraustad merged commit d3cdd51 into develop Aug 31, 2019

rraustad deleted the 7174-AddSETOutput branch August 31, 2019 16:13

rraustad mentioned this pull request Aug 31, 2019

Correction to UnitarySystem when HumRat control node is different from Temp control node #7412

Merged

18 tasks

mjwitte mentioned this pull request Dec 11, 2019

Standard Effective Temperature (SET) outputs from Energyplus not available #7174

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Addition of Standard Effective Temperature (SET) as an Output Variable #7431

Addition of Standard Effective Temperature (SET) as an Output Variable #7431

RKStrand commented Jul 30, 2019 •

edited by mjwitte

Loading

mjwitte Aug 22, 2019

mjwitte Aug 23, 2019

RKStrand Aug 26, 2019

mjwitte Aug 22, 2019

mjwitte Aug 22, 2019

RKStrand Aug 22, 2019

rraustad commented Aug 28, 2019 •

edited

Loading

rraustad commented Aug 28, 2019

rraustad left a comment

rraustad commented Aug 28, 2019

rraustad commented Aug 28, 2019 •

edited

Loading

RKStrand commented Aug 28, 2019

rraustad commented Aug 28, 2019

rraustad commented Aug 30, 2019

RKStrand commented Aug 30, 2019

Myoldmopar commented Aug 30, 2019

rraustad commented Aug 30, 2019

rraustad commented Aug 30, 2019

RKStrand commented Aug 30, 2019

Myoldmopar commented Aug 30, 2019

rraustad commented Aug 31, 2019


		ASSERT_TRUE(process_idf(idf_objects));

		OutputProcessor::TimeValue.allocate(2);

Addition of Standard Effective Temperature (SET) as an Output Variable #7431

Addition of Standard Effective Temperature (SET) as an Output Variable #7431

Conversation

RKStrand commented Jul 30, 2019 • edited by mjwitte Loading

Pull request overview

Work Checklist

Review Checklist

mjwitte Aug 22, 2019

Choose a reason for hiding this comment

mjwitte Aug 23, 2019

Choose a reason for hiding this comment

RKStrand Aug 26, 2019

Choose a reason for hiding this comment

mjwitte Aug 22, 2019

Choose a reason for hiding this comment

mjwitte Aug 22, 2019

Choose a reason for hiding this comment

RKStrand Aug 22, 2019

Choose a reason for hiding this comment

rraustad commented Aug 28, 2019 • edited Loading

rraustad commented Aug 28, 2019

rraustad left a comment

Choose a reason for hiding this comment

rraustad commented Aug 28, 2019

rraustad commented Aug 28, 2019 • edited Loading

RKStrand commented Aug 28, 2019

rraustad commented Aug 28, 2019

rraustad commented Aug 30, 2019

RKStrand commented Aug 30, 2019

Myoldmopar commented Aug 30, 2019

rraustad commented Aug 30, 2019

rraustad commented Aug 30, 2019

RKStrand commented Aug 30, 2019

Myoldmopar commented Aug 30, 2019

rraustad commented Aug 31, 2019

RKStrand commented Jul 30, 2019 •

edited by mjwitte

Loading

rraustad commented Aug 28, 2019 •

edited

Loading

rraustad commented Aug 28, 2019 •

edited

Loading