Surface Outside Face Convection Heat Gain Rate was fixed when rain flag is true & to be altered for OSCM as well #7504

dareumnam · 2019-09-16T20:52:57Z

Pull request overview

Fixes Surface outside face heat balance reporting is confusing when rain flag is true #6939 reporting for Surface Outside Face Convection (and Per Area) when raining
Also fixes Surface Outside Face Convection when using OtherSideConditionsModel (OSCM), the per Area value was correct and remains unchanged.
HeatBalanceSurfaceManager.cc & HeatBalanceSurfaceManager.hh
- Add a function for QdotConvOutRepPerArea to separate cases
  - If Optr is set, use OSCM boundary data
  - If Optr is not set,
    - If it is raining, (IsRain = True): Surface Outside Face Convection Heat Gain Rate calculation uses outdoor wet bulb temp.
    - Otherwise (IsRain = False): Surface Outside Face Convection Heat Gain Rate calculation uses outdoor dry bulb temp.
Add a unit test to HeatBalanceSurfaceManager.unit.cc
Expected Diffs:
- ESO Diffs: .csv output files of regression tests
  - that have Output: Variable of Surface Outside Face Convection Heat Gain Rate or Surface Outside Face Convection Heat Gain Rate per Area &&
  - that use a weather file that has at least one rainy day
  - this is likely to introduce small diffs in the Per Area output, because the surface area is no longer multiplied in and then divided back out.

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.

…alse when calculates Surface Outside Face Convection Heat Gain Rate and Surface Outside Face Convection Heat Gain Rate per Area

…nto Issue6939

…ode lines with the function

…ol error

mjwitte

Current code fixes the defect as expected (will retest after changes).
See comments below.

mjwitte · 2019-09-20T14:55:02Z

src/EnergyPlus/HeatBalanceSurfaceManager.cc

+    Real64 GetQdotConvOutRep(int SurfNum)
+    {
+        if (IsRain) {
+            return -Surface(SurfNum).Area * HcExtSurf(SurfNum) * (TH(1, 1, SurfNum) - Surface(SurfNum).OutWetBulbTemp);


It would be more efficient to leave area out of this, using this function to set QdotConvOutRepPerArea, then QdotConvOutRep = QdotConvOutRepPerArea * ...Area

mjwitte · 2019-09-20T14:59:45Z

src/EnergyPlus/HeatBalanceSurfaceManager.cc

@@ -5707,17 +5707,29 @@ namespace HeatBalanceSurfaceManager {
            }

            // fill in reporting values for outside face
-            QdotConvOutRep(SurfNum) = -Surface(SurfNum).Area * HcExtSurf(SurfNum) * (TH(1, 1, SurfNum) - Surface(SurfNum).OutDryBulbTemp);
+
+            QdotConvOutRep(SurfNum) = GetQdotConvOutRep(SurfNum);

            if (Surface(SurfNum).OSCMPtr > 0) { // Optr is set above in this case, use OSCM boundary data
                QdotConvOutRepPerArea(SurfNum) = -OSCM(OPtr).HConv * (TH(1, 1, SurfNum) - OSCM(OPtr).TConv);


It's wrong here that QdotConvOutRepPerArea is altered for OSCM but not QdotConvOutRep (not your error but another one here).

I would move this if into the new function as well. Then this area of code becomes 2 lines:

QdotConvOutRepPerArea(SurfNum) = GetQdotConvOutRep(SurfNum); QdotConvOutRep(SurfNum) = QdotConvOutRepPerArea(SurfNum) * Surface(SurfNum).Area;

Please update the title and description to mention fixing this for OSCM.

@mjwitte Thank you for all the reviews. I will fix these as you suggested.

mjwitte · 2019-09-20T15:08:09Z

src/EnergyPlus/HeatBalanceSurfaceManager.cc

+        } else {
+            return -Surface(SurfNum).Area * HcExtSurf(SurfNum) * (TH(1, 1, SurfNum) - Surface(SurfNum).OutDryBulbTemp);
+        }
+        return Real64();


Is this line necessary? @Myoldmopar ?

mjwitte · 2019-09-20T15:44:07Z

tst/EnergyPlus/unit/HeatBalanceSurfaceManager.unit.cc

+    DataEnvironment::IsRain = true;
+    Real64 ExpectedQconv1 = -58.197 * 1000 * (6.71793958923051 - 6.66143784594778);
+
+    EXPECT_EQ(ExpectedQconv1, GetQdotConvOutRep(1));


For Real64 calcs like this, we prefer EXPECT_NEAR with a small tolerance reasonable for the expected result magnitude: EXPECT_NEAR(ExpectedQconv1, GetQdotConvOutRep(1), 0.01);

…or OSCM

…nto Issue6939

mjwitte

@dareumnam Thanks - changes look good. I made one more small change adding const to the SurfNum agrument in the new function. Once CI completes, this will be merged.

mjwitte · 2019-09-20T19:18:41Z

src/EnergyPlus/HeatBalanceSurfaceManager.cc

        } // ...end of DO loop over all surface (actually heat transfer surfaces)
    }

+    Real64 GetQdotConvOutRepPerArea(int const SurfNum) 
+    {
+        int OPtr = Surface(SurfNum).OSCPtr;


@dareumnam Some integration tests are failing. Here's the problem - this should be .OSCMPtr.

@mjwitte Oops, thank you for letting me know. I'll fix it and push the branch again.

mjwitte · 2019-09-20T20:24:43Z

That's looking much better now - 2 files with small eso diffs. I'll let CI run its course here.

dareumnam · 2019-09-20T20:34:28Z

I am guessing that these ESO diffs are coming from .csv output files that have Surface Outside Face Convection Heat Gain Rate (Per Area) of rainy days (hopefully). I'll wait and see the CI testing results.

mjwitte · 2019-09-20T21:50:58Z

Yep, the diffs are in Surface Outside Face Convection Heat Gain Rate, on the order of e-11 or less.

dareumnam added 8 commits September 12, 2019 16:20

Fix HeatBalanceSurfaceManager.cc to separate cases IsRain = true or f…

53df3e9

…alse when calculates Surface Outside Face Convection Heat Gain Rate and Surface Outside Face Convection Heat Gain Rate per Area

add unit test version 1

70ec219

Merge remote-tracking branch 'upstream/develop' into Issue6939

d1187b9

changed the code using inline function

e9b10cd

Merge remote-tracking branch 'upstream/develop' into Issue6939

32bcb51

update the code and the unit test

1a22e53

Clear a line

a69eb9e

Merge remote-tracking branch 'upstream/develop' into Issue6939

68e9661

dareumnam added the Defect Includes code to repair a defect in EnergyPlus label Sep 16, 2019

dareumnam requested a review from Myoldmopar September 16, 2019 20:52

dareumnam self-assigned this Sep 16, 2019

dareumnam and others added 13 commits September 16, 2019 14:58

Clean up repeated code lines

9b0ce92

Fix the mistake while reducing the repeated code lines

4a3d59d

Return to original code for QdotConvOutRep PER AREA

ecdca54

Test for CI testing without my unit test

61010bf

Merge branch 'Issue6939' of https://github.com/dareumnam/EnergyPlus i…

c77ee22

…nto Issue6939

Test for CI testing without the unit test & replacement of repeated c…

b31cf1f

…ode lines with the function

The unit test was fixed to get rid of the error

d8ae313

clean up lines

46338d0

clean up

13d4f7d

Remove inline function to get rid of lnk2019 unresolved external symb…

9e0bd6c

…ol error

One more commit to make all 7 CI testing run

c64e448

Merge remote-tracking branch 'upstream/develop' into Issue6939

14ac948

Merge remote-tracking branch 'remotes/origin/develop' into Issue6939

65cf866

mjwitte requested changes Sep 20, 2019

View reviewed changes

dareumnam added 3 commits September 20, 2019 09:56

Merge remote-tracking branch 'upstream/develop' into Issue6939

5e08856

Fixed the code and the unit test that QdotConvOutRep can be altered f…

ba26532

…or OSCM

Unit test was fixed to use GetQdotConvOutRepPerArea function directly

6f76eab

dareumnam changed the title ~~Fix #6939 - Surface outside face heat balance reporting when rain flag is true~~ Surface Outside Face Convection Heat Gain Rate was fixed when rain flag is true & to be altered for OSCM as well Sep 20, 2019

Merge branch 'Issue6939' of https://github.com/dareumnam/EnergyPlus i…

55495ab

…nto Issue6939

6939 make new function argument const

cfcfed1

mjwitte approved these changes Sep 20, 2019

View reviewed changes

mjwitte reviewed Sep 20, 2019

View reviewed changes

Fix the variable name error for OSCMPtr

69827fd

mjwitte merged commit 3d373b9 into NREL:develop Sep 21, 2019

dareumnam deleted the Issue6939 branch October 11, 2019 17:23

shorowit mentioned this pull request Mar 16, 2021

Incorrect exterior surface convective heat transfer rate reporting during rain under active Present Weather Observation override in Version 8.9 #8621

Closed

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Surface Outside Face Convection Heat Gain Rate was fixed when rain flag is true & to be altered for OSCM as well #7504

Surface Outside Face Convection Heat Gain Rate was fixed when rain flag is true & to be altered for OSCM as well #7504

dareumnam commented Sep 16, 2019 •

edited by mjwitte

Loading

mjwitte left a comment

mjwitte Sep 20, 2019

mjwitte Sep 20, 2019

dareumnam Sep 20, 2019

mjwitte Sep 20, 2019

mjwitte Sep 20, 2019

mjwitte left a comment

mjwitte Sep 20, 2019

dareumnam Sep 20, 2019

mjwitte commented Sep 20, 2019

dareumnam commented Sep 20, 2019 •

edited

Loading

mjwitte commented Sep 20, 2019

Surface Outside Face Convection Heat Gain Rate was fixed when rain flag is true & to be altered for OSCM as well #7504

Surface Outside Face Convection Heat Gain Rate was fixed when rain flag is true & to be altered for OSCM as well #7504

Conversation

dareumnam commented Sep 16, 2019 • edited by mjwitte Loading

Pull request overview

Pull Request Author

Reviewer

mjwitte left a comment

Choose a reason for hiding this comment

mjwitte Sep 20, 2019

Choose a reason for hiding this comment

mjwitte Sep 20, 2019

Choose a reason for hiding this comment

dareumnam Sep 20, 2019

Choose a reason for hiding this comment

mjwitte Sep 20, 2019

Choose a reason for hiding this comment

mjwitte Sep 20, 2019

Choose a reason for hiding this comment

mjwitte left a comment

Choose a reason for hiding this comment

mjwitte Sep 20, 2019

Choose a reason for hiding this comment

dareumnam Sep 20, 2019

Choose a reason for hiding this comment

mjwitte commented Sep 20, 2019

dareumnam commented Sep 20, 2019 • edited Loading

mjwitte commented Sep 20, 2019

dareumnam commented Sep 16, 2019 •

edited by mjwitte

Loading

dareumnam commented Sep 20, 2019 •

edited

Loading