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

Fix ASHRAE simple method for vertical surfaces #7890

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
a9b7522
Merge develop into working branch.
ajscimone Mar 23, 2020
4b1350f
Remove left over git conflict string.
ajscimone Mar 23, 2020
345bcdd
Correct minimum Hcov In setting.
ajscimone Mar 24, 2020
f53aa07
Create/add unit tests for the calculation of coefficients from Ashrae…
ajscimone Mar 24, 2020
4ec9b5a
Merge branch 'develop' into fix-ashrae-simple-method-hconvin-dependence
ajscimone Mar 27, 2020
dea0356
Fix near zero condition on decision tree logic for convection coeffic…
ajscimone Mar 31, 2020
93d89ce
Refactor ashrae simple algorithm and add additional unit tests for ze…
ajscimone Apr 1, 2020
1a9582a
Clean up unit test comments for CalcASHRAESimpleIntConvCoeff.
ajscimone Apr 1, 2020
33e8f41
Merge branch 'develop' into fix-ashrae-simple-method-hconvin-dependence
ajscimone Apr 6, 2020
dbe9881
Fix unit test to account for horizontal surfaced flipped 180 degrees,…
ajscimone Apr 18, 2020
fb2f038
Replace if else style algorithm with original implementation.
ajscimone Apr 18, 2020
f4218c0
Merge branch 'develop' into fix-ashrae-simple-method-hconvin-dependence
ajscimone Apr 19, 2020
acab86f
Remove unneeded test for HConvIn.
ajscimone Apr 21, 2020
873b1cf
Add unit test for scenario where costilt is close to zero but not exa…
ajscimone Apr 21, 2020
ee4a575
Replace original algorithm which fails new unit tests with new if/els…
ajscimone Apr 21, 2020
cd73ed9
Merge branch 'develop' into fix-ashrae-simple-method-hconvin-dependence
ajscimone Apr 21, 2020
22e9f64
Clean up ordering of checks in the ashrae simple unit test.
ajscimone Apr 21, 2020
ee98a72
Modify ashrae simple algorithm to handle all tilted surface conditions.
ajscimone Apr 22, 2020
04b612a
Merge branch 'develop' into fix-ashrae-simple-method-hconvin-dependence
ajscimone Apr 26, 2020
c83e67b
Modify ashrae simple explanation table.
ajscimone Apr 28, 2020
101f597
Merge develop into branch.
ajscimone May 15, 2020
b00e5bf
Add unit test for HConvIn Dependence on vertical surfaces where the a…
ajscimone May 15, 2020
9bf737c
Remove unit test unneeded comment.
ajscimone May 15, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
89 changes: 48 additions & 41 deletions src/EnergyPlus/ConvectionCoefficients.cc
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@
#include <algorithm>
#include <cassert>
#include <cmath>
#include <limits>
Copy link
Member

Choose a reason for hiding this comment

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

Removing includes 👍

#include <string>

// ObjexxFCL Headers
Expand Down Expand Up @@ -3831,62 +3830,70 @@ namespace ConvectionCoefficients {
// NBSSIR 83-2655, National Bureau of Standards, "Surface Inside Heat Balances", pp 79.
// 2. ASHRAE Handbook of Fundamentals 1985, p. 23.2, Table 1.

// USE STATEMENTS:
// na

// Locals
// SUBROUTINE ARGUMENT DEFINITIONS:

// SUBROUTINE PARAMETER DEFINITIONS:
// na

// INTERFACE BLOCK SPECIFICATIONS:
// na

// DERIVED TYPE DEFINITIONS:
// na
// +---------------------+-----------+---------------------------------------------+------------------+-----------------+-------------+
// | Situation | DeltaTemp | CosTilt | cos(tilt)*deltaT | Convection Type | Coefficient |
// +---------------------+-----------+---------------------------------------------+------------------+-----------------+-------------+
// | Vertical Surface | N/A | -0.3827 to 0.3827 (67.5 to 112.5 degrees) | N/A | Normal | 3.076 |
// | Horizontal Surface | Positive | 0.9238 to 1.0 (0 to 22.5 degrees) | Positive | Enhanced | 4.043 |
// | Horizontal Surface | Positive | -0.9238 to -1.0 (157.5 to 180 degrees) | Negative | Reduced | 0.948 |
// | Horizontal Surface | Negative | 0.9239 to 1.0 (0 to 22.5 degrees) | Negative | Reduced | 0.948 |
// | Horizontal Surface | Negative | -0.9239 to -1.0 (157.5 to 180 degrees) | Positive | Enhanced | 4.040 |
// | Tilted Surface | Positive | 0.3827 to 0.9239 (22.5 to 67.5 degrees) | Positive | Enhanced | 3.870 |
// | Tilted Surface | Negative | -0.3827 to -0.9239 (157.5 to 157.5 degrees) | Positive | Enhanced | 3.870 |
// | Tilted Surface | Negative | 0.3827 to 0.9239 (22.5 to 67.5 degrees) | Negative | Reduced | 2.281 |
// | Tilted Surface | Positive | -0.3827 to -0.9239 (157.5 to 157.5 degrees) | Negative | Reduced | 2.281 |
// +---------------------+-----------+---------------------------------------------+------------------+-----------------+-------------+
Copy link
Member

Choose a reason for hiding this comment

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

In a function like this with a bunch of code paths, this is a great addition to help a developer understand what should be happening, and also the limitations of the function.


// SUBROUTINE LOCAL VARIABLE DECLARATIONS:
Real64 DeltaTemp = Tamb - Tsurf;

// Set HConvIn using the proper correlation based on DeltaTemp and Cosine of the Tilt of the Surface
if (std::abs(cosTilt) >= 0.9239) { // Horizontal Surface
if (DeltaTemp * cosTilt < 0.0) { // Horizontal, Reduced Convection
return 0.948;
} else if (DeltaTemp * cosTilt == 0.0) { // Vertical Surface
return 3.076;
} else /*if (DeltaTemp * cosTilt > 0.0)*/ { // Horizontal, Enhanced Convection
return 4.040;
}
} else { // Tilted Surface
if (DeltaTemp * cosTilt < 0.0) { // Tilted, Reduced Convection
return 2.281;
} else if (DeltaTemp * cosTilt == 0.0) { // Vertical Surface
return 3.076;
} else /*if (DeltaTemp * cosTilt > 0.0)*/ { // Tilted, Enhanced Convection
return 3.870;
if (std::abs(cosTilt) < 0.3827) { // Vertical Surface
return 3.076;
}
else {
Real64 DeltaTempCosTilt = (Tamb - Tsurf)*cosTilt;
Copy link
Member

Choose a reason for hiding this comment

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

A little whitespace around that asteriscus would've been nice but not a big deal.

if (std::abs(cosTilt) >= 0.9239) { // Horizontal Surface
if (DeltaTempCosTilt > 0.0){ //Enhanced Convection
return 4.040;
}
else if (DeltaTempCosTilt < 0.0){ // Reduced Convection
return 0.948;
}
else { // Zero DeltaTemp
return 3.076;
}
}
else { // tilted surface
if (DeltaTempCosTilt > 0.0){ // Enhanced Convection
return 3.870;
}
else if (DeltaTempCosTilt < 0.0){ // Reduced Convection
return 2.281;
}
else { // Zero DeltaTemp
return 3.076;
}
}
}

}

void CalcASHRAESimpleIntConvCoeff(int const SurfNum, // surface number for which coefficients are being calculated
Real64 const SurfaceTemperature, // Temperature of surface for evaluation of HcIn
Real64 const ZoneMeanAirTemperature // Mean Air Temperature of Zone
)
{

if (std::abs(Surface(SurfNum).CosTilt) >= 0.3827) { // Recalculate HConvIn
if (Surface(SurfNum).ExtBoundCond == DataSurfaces::KivaFoundation) {
SurfaceGeometry::kivaManager.surfaceConvMap[SurfNum].in = [](double Tsurf, double Tamb, double, double, double cosTilt) -> double {
return CalcASHRAESimpleIntConvCoeff(Tsurf, Tamb, cosTilt);
};
} else {
HConvIn(SurfNum) = CalcASHRAESimpleIntConvCoeff(SurfaceTemperature, ZoneMeanAirTemperature, Surface(SurfNum).CosTilt);
}
if (Surface(SurfNum).ExtBoundCond == DataSurfaces::KivaFoundation){
SurfaceGeometry::kivaManager.surfaceConvMap[SurfNum].in = [](double Tsurf, double Tamb, double, double, double cosTilt) -> double {
return CalcASHRAESimpleIntConvCoeff(Tsurf, Tamb, cosTilt);
};
}
else {
Copy link
Member

Choose a reason for hiding this comment

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

This else style is not the preferred form. } else { is preferred.

HConvIn(SurfNum) = CalcASHRAESimpleIntConvCoeff(SurfaceTemperature, ZoneMeanAirTemperature, Surface(SurfNum).CosTilt);
}

// Establish some lower limit to avoid a zero convection coefficient (and potential divide by zero problems)
if (HConvIn(SurfNum) < LowHConvLimit) HConvIn(SurfNum) = LowHConvLimit;
HConvIn(SurfNum) = max(HConvIn(SurfNum), LowHConvLimit);
}

Real64 CalcASHRAETARPNatural(Real64 const Tsurf, Real64 const Tamb, Real64 const cosTilt)
Expand Down
136 changes: 136 additions & 0 deletions tst/EnergyPlus/unit/ConvectionCoefficients.unit.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1242,3 +1242,139 @@ TEST_F(ConvectionCoefficientsFixture, CalcBeausoleilMorrisonMixedUnstableCeiling
convCoeff = CalcBeausoleilMorrisonMixedUnstableCeiling(deltaTemp, height, surfTemp, zoneNum);
EXPECT_NEAR(convCoeff, 9.999, tolerance);
}


TEST_F(ConvectionCoefficientsFixture, ConvectionCoefficientsTest_CalcASHRAESimpleIntConvCoeff)
{
// Unit test for the function CalcASHRAESimpleIntConvCoeff, used to determine the Convection Coefficient
// for the Ashrae Simple algorithm setting

Real64 Tsurf;
Real64 Tamb;
Real64 CosTilt;
Real64 ConvectionCoefficient;
Real64 ExpectedCoefficient;

// Scenario: Vertical Surface
// Hcov expected = 3.076
// Delta_T is not relevant for this calculation

Tsurf = 30.0;
Tamb = 20.0;
CosTilt = 0.0; // cos(90 degrees)
ExpectedCoefficient = 3.076;

ConvectionCoefficient = CalcASHRAESimpleIntConvCoeff(Tsurf, Tamb, CosTilt);
EXPECT_EQ(ConvectionCoefficient, ExpectedCoefficient);

//Scenario: Vertical Surface, CosTilt not exactly zero
// Hcov expected = 3.076

Tsurf = 19.0;
Tamb = 20.0;
CosTilt = 0.0001; // cos(90 degrees)
ExpectedCoefficient = 3.076;

ConvectionCoefficient = CalcASHRAESimpleIntConvCoeff(Tsurf, Tamb, CosTilt);
EXPECT_EQ(ConvectionCoefficient, ExpectedCoefficient);

//Scenario: Vertical Surface, Zero Delta T
// Hcov expected = 3.076

Tsurf = 23.0;
Tamb = 23.0;
CosTilt = 0; // cos(90 degrees)
ExpectedCoefficient = 3.076;

ConvectionCoefficient = CalcASHRAESimpleIntConvCoeff(Tsurf, Tamb, CosTilt);
EXPECT_EQ(ConvectionCoefficient, ExpectedCoefficient);

// Scenario: Horizontal Surface with reduced convection
// Hcov expected = 0.948
// A negative Delta_T is required for reduced convection

Tsurf = 30.0;
Tamb = 20.0;
CosTilt = 0.9239; // cos(22.5 degrees)
ExpectedCoefficient = 0.948;

ConvectionCoefficient = CalcASHRAESimpleIntConvCoeff(Tsurf, Tamb, CosTilt);
EXPECT_EQ(ConvectionCoefficient, ExpectedCoefficient);


//Scenario: Horizontal surface with enhanced convection:
// Hcov expected = 4.040
// A positive Delta_T is required for enhanced convection

Tsurf = 20.0;
Tamb = 30.0;
CosTilt = 0.9239; // cos(22.5 degrees)
ExpectedCoefficient = 4.040;

ConvectionCoefficient = CalcASHRAESimpleIntConvCoeff(Tsurf, Tamb, CosTilt);
EXPECT_EQ(ConvectionCoefficient, ExpectedCoefficient);

//Scenario: horizontal surface, enhanced convection
// 180 degree surface, negative Delta_T
// Hcov expected = 4.040

Tsurf = 30.0;
Tamb = 20.0;
CosTilt = -1; // cos(180 degrees)
ExpectedCoefficient = 4.040;

ConvectionCoefficient = CalcASHRAESimpleIntConvCoeff(Tsurf, Tamb, CosTilt);
EXPECT_EQ(ConvectionCoefficient, ExpectedCoefficient);

//Scenario: horizontal surface, reduced convection
// 180 degree surface, positive Delta_T
// Hcov expected = 0.948

Tsurf = 20.0;
Tamb = 30.0;
CosTilt = -1; // cos(180 degrees)
ExpectedCoefficient = 0.948;

ConvectionCoefficient = CalcASHRAESimpleIntConvCoeff(Tsurf, Tamb, CosTilt);
EXPECT_EQ(ConvectionCoefficient, ExpectedCoefficient);

//Scenario: tilted surface with reduced convection
// Hcov expected = 2.281
// A negative Delta_T is required for reduced convection

Tsurf = 30.0;
Tamb = 20.0;
CosTilt = 0.707; // cos(45 degrees)
ExpectedCoefficient = 2.281;

ConvectionCoefficient = CalcASHRAESimpleIntConvCoeff(Tsurf, Tamb, CosTilt);
EXPECT_EQ(ConvectionCoefficient, ExpectedCoefficient);

//Scenario: tilted surface with enhanced convection
// Hcov expected = 3.870

Tsurf = 20.0;
Tamb = 30.0;
CosTilt = 0.707; // cos(45 degrees)
ExpectedCoefficient = 3.870;

ConvectionCoefficient = CalcASHRAESimpleIntConvCoeff(Tsurf, Tamb, CosTilt);
EXPECT_EQ(ConvectionCoefficient, ExpectedCoefficient);
}

TEST_F(ConvectionCoefficientsFixture, ConvectionCoefficientsTest_HConvInDependence)
{
Real64 ConvectionCoefficient;
Real64 ExpectedCoefficient = 3.076;

DataSurfaces::Surface.allocate(1);
DataSurfaces::Surface(1).CosTilt = 0;

DataHeatBalance::HConvIn.allocate(1);

CalcASHRAESimpleIntConvCoeff(1, 20.0, 30.0);

ConvectionCoefficient = DataHeatBalance::HConvIn(1);

EXPECT_EQ(ConvectionCoefficient, ExpectedCoefficient);
}
Copy link
Member

Choose a reason for hiding this comment

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

Great set of unit tests. Kind of a shame that you missed one code path in all the possible configurations (line 3874 here), but again, not going to hold this up for that, this is good work.