Skip to content

Commit

Permalink
OutConvClass to enum class, fix unit tests
Browse files Browse the repository at this point in the history
  • Loading branch information
@mitchute
mitchute committed Jul 20, 2021
1 parent ac3cc02 commit c59ca83
Show file tree
Hide file tree
Showing 6 changed files with 58 additions and 53 deletions.
18 changes: 9 additions & 9 deletions src/EnergyPlus/ConvectionCoefficients.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5653,17 +5653,17 @@ void DynamicExtConvSurfaceClassification(EnergyPlusData &state, int const SurfNu
}

if (DeltaTemp < 0.0) {
state.dataSurface->SurfOutConvClassification(SurfNum) = ConvectionConstants::OutConvClass_RoofStable;
state.dataSurface->SurfOutConvClassification(SurfNum) = ConvectionConstants::OutConvClass::RoofStable;
} else {
state.dataSurface->SurfOutConvClassification(SurfNum) = ConvectionConstants::OutConvClass_RoofUnstable;
state.dataSurface->SurfOutConvClassification(SurfNum) = ConvectionConstants::OutConvClass::RoofUnstable;
}

} else {

if (Windward(Surface(SurfNum).CosTilt, Surface(SurfNum).Azimuth, surfWindDir)) {
state.dataSurface->SurfOutConvClassification(SurfNum) = ConvectionConstants::OutConvClass_WindwardVertWall;
state.dataSurface->SurfOutConvClassification(SurfNum) = ConvectionConstants::OutConvClass::WindwardVertWall;
} else {
state.dataSurface->SurfOutConvClassification(SurfNum) = ConvectionConstants::OutConvClass_LeewardVertWall;
state.dataSurface->SurfOutConvClassification(SurfNum) = ConvectionConstants::OutConvClass::LeewardVertWall;
}
}
}
Expand All @@ -5678,7 +5678,7 @@ void MapExtConvClassificationToHcModels(EnergyPlusData &state, int const SurfNum
// RE-ENGINEERED na

switch (state.dataSurface->SurfOutConvClassification(SurfNum)) {
case ConvectionConstants::OutConvClass_WindwardVertWall:
case ConvectionConstants::OutConvClass::WindwardVertWall:
state.dataSurface->SurfOutConvHfModelEq(SurfNum) = state.dataConvectionCoefficient->OutsideFaceAdaptiveConvectionAlgo.HWindWallWindwardEqNum;
if (state.dataSurface->SurfOutConvHfModelEq(SurfNum) == ConvectionConstants::HcExt_UserCurve) {
state.dataSurface->SurfOutConvHfUserCurveIndex(SurfNum) =
Expand All @@ -5690,7 +5690,7 @@ void MapExtConvClassificationToHcModels(EnergyPlusData &state, int const SurfNum
state.dataConvectionCoefficient->OutsideFaceAdaptiveConvectionAlgo.HNatVertWallUserCurveNum;
}
break;
case ConvectionConstants::OutConvClass_LeewardVertWall:
case ConvectionConstants::OutConvClass::LeewardVertWall:
state.dataSurface->SurfOutConvHfModelEq(SurfNum) = state.dataConvectionCoefficient->OutsideFaceAdaptiveConvectionAlgo.HWindWallLeewardEqNum;
if (state.dataSurface->SurfOutConvHfModelEq(SurfNum) == ConvectionConstants::HcExt_UserCurve) {
state.dataSurface->SurfOutConvHfUserCurveIndex(SurfNum) =
Expand All @@ -5702,7 +5702,7 @@ void MapExtConvClassificationToHcModels(EnergyPlusData &state, int const SurfNum
state.dataConvectionCoefficient->OutsideFaceAdaptiveConvectionAlgo.HNatVertWallUserCurveNum;
}
break;
case ConvectionConstants::OutConvClass_RoofStable:
case ConvectionConstants::OutConvClass::RoofStable:
state.dataSurface->SurfOutConvHfModelEq(SurfNum) = state.dataConvectionCoefficient->OutsideFaceAdaptiveConvectionAlgo.HWindHorizRoofEqNum;
if (state.dataSurface->SurfOutConvHfModelEq(SurfNum) == ConvectionConstants::HcExt_UserCurve) {
state.dataSurface->SurfOutConvHfUserCurveIndex(SurfNum) =
Expand All @@ -5714,7 +5714,7 @@ void MapExtConvClassificationToHcModels(EnergyPlusData &state, int const SurfNum
state.dataConvectionCoefficient->OutsideFaceAdaptiveConvectionAlgo.HNatStableHorizUserCurveNum;
}
break;
case ConvectionConstants::OutConvClass_RoofUnstable:
case ConvectionConstants::OutConvClass::RoofUnstable:
state.dataSurface->SurfOutConvHfModelEq(SurfNum) = state.dataConvectionCoefficient->OutsideFaceAdaptiveConvectionAlgo.HWindHorizRoofEqNum;
if (state.dataSurface->SurfOutConvHfModelEq(SurfNum) == ConvectionConstants::HcExt_UserCurve) {
state.dataSurface->SurfOutConvHfUserCurveIndex(SurfNum) =
Expand All @@ -5728,7 +5728,7 @@ void MapExtConvClassificationToHcModels(EnergyPlusData &state, int const SurfNum
break;
default:
ShowSevereError(state,
format("MapExtConvClassificationToHcModels: caught unknown outdoor surface classification:{}",
format("MapExtConvClassificationToHcModels: caught unknown outdoor surface classification: {}",
state.dataSurface->SurfOutConvClassification(SurfNum)));
}
}
Expand Down
12 changes: 8 additions & 4 deletions src/EnergyPlus/ConvectionConstants.hh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -144,10 +144,14 @@ int constexpr HcExt_AlamdariHammondStableHorizontal{324};
int constexpr HcExt_AlamdariHammondUnstableHorizontal{325};

// Parameters for classification of outside face of surfaces
constexpr int OutConvClass_WindwardVertWall{101};
constexpr int OutConvClass_LeewardVertWall{102};
constexpr int OutConvClass_RoofStable{103};
constexpr int OutConvClass_RoofUnstable{104};
enum class OutConvClass
{
Invalid = -1,
WindwardVertWall = 101,
LeewardVertWall = 102,
RoofStable = 103,
RoofUnstable = 104,
};

enum class ConvSurfDeltaT : int
{
Expand Down
9 changes: 5 additions & 4 deletions src/EnergyPlus/DataSurfaces.hh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1257,10 +1257,11 @@ struct SurfacesData : BaseGlobalStruct
Array1D<int> SurfIntConvCoeffIndex; // Interior Convection Coefficient pointer (different data structure) when being overridden
Array1D<int> SurfExtConvCoeffIndex; // Exterior Convection Coefficient pointer (different data structure) when being overridden
Array1D<ConvectionConstants::InConvClass>
SurfIntConvClassification; // current classification for inside face air flow regime and surface orientation
Array1D<int> SurfIntConvHcModelEq; // current convection model for inside face
Array1D<int> SurfIntConvHcUserCurveIndex; // current index to user convection model if used
Array1D<int> SurfOutConvClassification; // current classification for outside face wind regime and convection orientation
SurfIntConvClassification; // current classification for inside face air flow regime and surface orientation
Array1D<int> SurfIntConvHcModelEq; // current convection model for inside face
Array1D<int> SurfIntConvHcUserCurveIndex; // current index to user convection model if used
Array1D<ConvectionConstants::OutConvClass>
SurfOutConvClassification; // current classification for outside face wind regime and convection orientation
Array1D<int> SurfOutConvHfModelEq; // current convection model for forced convection at outside face
Array1D<int> SurfOutConvHfUserCurveIndex; // current index to user forced convection model if used
Array1D<int> SurfOutConvHnModelEq; // current Convection model for natural convection at outside face
Expand Down
14 changes: 7 additions & 7 deletions src/EnergyPlus/HeatBalanceSurfaceManager.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2037,13 +2037,13 @@ void AllocateSurfaceHeatBalArrays(EnergyPlusData &state)
"Average",
Surface(loop).Name);
if (Surface(loop).ExtBoundCond == ExternalEnvironment) {
SetupOutputVariable(state,
"Surface Outside Face Convection Classification Index",
OutputProcessor::Unit::None,
state.dataSurface->SurfOutConvClassification(loop),
"Zone",
"Average",
Surface(loop).Name);
// SetupOutputVariable(state,

This comment has been minimized.

Sign in to view

Copy link
@mitchute

mitchute Jul 20, 2021 •
edited
Loading

Author Collaborator

Do we have a plan for allowing enums in SetupOutputVars yet, or do we just label them as "not converted to enum due to SetupOutputVars usage," or similar? I thought there was a branch that proposed a solution on this but can't track it down.

This comment has been minimized.

Sign in to view

Copy link
@amirroth

amirroth Jul 20, 2021

Collaborator

There are two separate issues with enum and SetupOutputVariable.

The first issue is that output variables can also be used as EMS actuators. EMS can only actuate using numbers, not enumerations, and while it is legal to assign a class enum : int to an integer, the reverse is not true. There are three possible solutions to this problem.

  1. Don't use enums for variables that can also be EMS actuators.
  2. Don't overload output variables as EMS actuators.
  3. Create an output variable template that can be instantiated for different enums.

I think we are currently using option 1, but we may have also prototyped option 3.

The second issue has to do with enumerations that are not compact. This is more of an aesthetic/philosophical issue than an actual compilation/typing issue. Enumerations are not required to be compact, it's just cleaner and faster if they are. Also, the point of an enumerations is explicitly to NOT rely on its numeric value, with the possible exception of -1 for Error or Invalid or NotFound. Some solutions in order of preference would be:

  1. Compactify and output these as strings rather than arbitrary integers.
  2. Compactify and output as integers but also provide API functions that can convert integers to strings and vice versa.
  3. Leave uncompacted.
// "Surface Outside Face Convection Classification Index",
// OutputProcessor::Unit::None,
// state.dataSurface->SurfOutConvClassification(loop),
// "Zone",
// "Average",
// Surface(loop).Name);
SetupOutputVariable(state,
"Surface Outside Face Forced Convection Model Equation Index",
OutputProcessor::Unit::None,
Expand Down
2 changes: 1 addition & 1 deletion src/EnergyPlus/SurfaceGeometry.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1016,7 +1016,7 @@ namespace SurfaceGeometry {
state.dataSurface->SurfIntConvClassification(SurfNum) = ConvectionConstants::InConvClass::Invalid;
state.dataSurface->SurfIntConvHcModelEq(SurfNum) = 0;
state.dataSurface->SurfIntConvHcUserCurveIndex(SurfNum) = 0;
state.dataSurface->SurfOutConvClassification(SurfNum) = 0;
state.dataSurface->SurfOutConvClassification(SurfNum) = ConvectionConstants::OutConvClass::Invalid;
state.dataSurface->SurfOutConvHfModelEq(SurfNum) = 0;
state.dataSurface->SurfOutConvHfUserCurveIndex(SurfNum) = 0;
state.dataSurface->SurfOutConvHnModelEq(SurfNum) = 0;
Expand Down
56 changes: 28 additions & 28 deletions tst/EnergyPlus/unit/ConvectionCoefficients.unit.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -486,99 +486,99 @@ TEST_F(ConvectionCoefficientsFixture, DynamicIntConvSurfaceClassification)
state->dataHeatBalFanSys->MAT(1) = 30.0;

DynamicIntConvSurfaceClassification(*state, 1);
EXPECT_EQ(state->dataSurface->SurfIntConvClassification(1), ConvectionConstants::InConvClass_A3_VertWalls);
EXPECT_EQ(state->dataSurface->SurfIntConvClassification(1), ConvectionConstants::InConvClass::A3_VertWalls);

DynamicIntConvSurfaceClassification(*state, 2);
EXPECT_EQ(state->dataSurface->SurfIntConvClassification(2), ConvectionConstants::InConvClass_A3_StableTilted);
EXPECT_EQ(state->dataSurface->SurfIntConvClassification(2), ConvectionConstants::InConvClass::A3_StableTilted);

DynamicIntConvSurfaceClassification(*state, 3);
EXPECT_EQ(state->dataSurface->SurfIntConvClassification(3), ConvectionConstants::InConvClass_A3_UnstableTilted);
EXPECT_EQ(state->dataSurface->SurfIntConvClassification(3), ConvectionConstants::InConvClass::A3_UnstableTilted);

DynamicIntConvSurfaceClassification(*state, 4);
EXPECT_EQ(state->dataSurface->SurfIntConvClassification(4), ConvectionConstants::InConvClass_A3_UnstableHoriz);
EXPECT_EQ(state->dataSurface->SurfIntConvClassification(4), ConvectionConstants::InConvClass::A3_UnstableHoriz);

DynamicIntConvSurfaceClassification(*state, 5);
EXPECT_EQ(state->dataSurface->SurfIntConvClassification(5), ConvectionConstants::InConvClass_A3_StableHoriz);
EXPECT_EQ(state->dataSurface->SurfIntConvClassification(5), ConvectionConstants::InConvClass::A3_StableHoriz);

// vertical floor is currently not a valid case, so returns zero with a severe error
// DynamicIntConvSurfaceClassification(*state, 6);
// EXPECT_EQ(state->dataSurface->SurfIntConvClassification(6), 0);

DynamicIntConvSurfaceClassification(*state, 7);
EXPECT_EQ(state->dataSurface->SurfIntConvClassification(7), ConvectionConstants::InConvClass_A3_StableTilted);
EXPECT_EQ(state->dataSurface->SurfIntConvClassification(7), ConvectionConstants::InConvClass::A3_StableTilted);

DynamicIntConvSurfaceClassification(*state, 8);
EXPECT_EQ(state->dataSurface->SurfIntConvClassification(8), ConvectionConstants::InConvClass_A3_StableTilted);
EXPECT_EQ(state->dataSurface->SurfIntConvClassification(8), ConvectionConstants::InConvClass::A3_StableTilted);

DynamicIntConvSurfaceClassification(*state, 9);
EXPECT_EQ(state->dataSurface->SurfIntConvClassification(9), ConvectionConstants::InConvClass_A3_StableHoriz);
EXPECT_EQ(state->dataSurface->SurfIntConvClassification(9), ConvectionConstants::InConvClass::A3_StableHoriz);

DynamicIntConvSurfaceClassification(*state, 10);
EXPECT_EQ(state->dataSurface->SurfIntConvClassification(10), ConvectionConstants::InConvClass_A3_StableHoriz);
EXPECT_EQ(state->dataSurface->SurfIntConvClassification(10), ConvectionConstants::InConvClass::A3_StableHoriz);

DynamicIntConvSurfaceClassification(*state, 11);
EXPECT_EQ(state->dataSurface->SurfIntConvClassification(11), ConvectionConstants::InConvClass_A3_VertWalls);
EXPECT_EQ(state->dataSurface->SurfIntConvClassification(11), ConvectionConstants::InConvClass::A3_VertWalls);

DynamicIntConvSurfaceClassification(*state, 12);
EXPECT_EQ(state->dataSurface->SurfIntConvClassification(12), ConvectionConstants::InConvClass_A3_UnstableTilted);
EXPECT_EQ(state->dataSurface->SurfIntConvClassification(12), ConvectionConstants::InConvClass::A3_UnstableTilted);

DynamicIntConvSurfaceClassification(*state, 13);
EXPECT_EQ(state->dataSurface->SurfIntConvClassification(13), ConvectionConstants::InConvClass_A3_UnstableTilted);
EXPECT_EQ(state->dataSurface->SurfIntConvClassification(13), ConvectionConstants::InConvClass::A3_UnstableTilted);

DynamicIntConvSurfaceClassification(*state, 14);
EXPECT_EQ(state->dataSurface->SurfIntConvClassification(14), ConvectionConstants::InConvClass_A3_UnstableHoriz);
EXPECT_EQ(state->dataSurface->SurfIntConvClassification(14), ConvectionConstants::InConvClass::A3_UnstableHoriz);

DynamicIntConvSurfaceClassification(*state, 15);
EXPECT_EQ(state->dataSurface->SurfIntConvClassification(15), ConvectionConstants::InConvClass_A3_UnstableHoriz);
EXPECT_EQ(state->dataSurface->SurfIntConvClassification(15), ConvectionConstants::InConvClass::A3_UnstableHoriz);

// Case 2 - Zone air colder than surfaces
state->dataHeatBalFanSys->MAT(1) = 10.0;

DynamicIntConvSurfaceClassification(*state, 1);
EXPECT_EQ(state->dataSurface->SurfIntConvClassification(1), ConvectionConstants::InConvClass_A3_VertWalls);
EXPECT_EQ(state->dataSurface->SurfIntConvClassification(1), ConvectionConstants::InConvClass::A3_VertWalls);

DynamicIntConvSurfaceClassification(*state, 2);
EXPECT_EQ(state->dataSurface->SurfIntConvClassification(2), ConvectionConstants::InConvClass_A3_UnstableTilted);
EXPECT_EQ(state->dataSurface->SurfIntConvClassification(2), ConvectionConstants::InConvClass::A3_UnstableTilted);

DynamicIntConvSurfaceClassification(*state, 3);
EXPECT_EQ(state->dataSurface->SurfIntConvClassification(3), ConvectionConstants::InConvClass_A3_StableTilted);
EXPECT_EQ(state->dataSurface->SurfIntConvClassification(3), ConvectionConstants::InConvClass::A3_StableTilted);

DynamicIntConvSurfaceClassification(*state, 4);
EXPECT_EQ(state->dataSurface->SurfIntConvClassification(4), ConvectionConstants::InConvClass_A3_StableHoriz);
EXPECT_EQ(state->dataSurface->SurfIntConvClassification(4), ConvectionConstants::InConvClass::A3_StableHoriz);

DynamicIntConvSurfaceClassification(*state, 5);
EXPECT_EQ(state->dataSurface->SurfIntConvClassification(5), ConvectionConstants::InConvClass_A3_UnstableHoriz);
EXPECT_EQ(state->dataSurface->SurfIntConvClassification(5), ConvectionConstants::InConvClass::A3_UnstableHoriz);

// vertical floor is currently not a valid case, so returns zero with a severe error
// DynamicIntConvSurfaceClassification(*state, 6);
// EXPECT_EQ(state->dataSurface->SurfIntConvClassification(6), 0);

DynamicIntConvSurfaceClassification(*state, 7);
EXPECT_EQ(state->dataSurface->SurfIntConvClassification(7), ConvectionConstants::InConvClass_A3_UnstableTilted);
EXPECT_EQ(state->dataSurface->SurfIntConvClassification(7), ConvectionConstants::InConvClass::A3_UnstableTilted);

DynamicIntConvSurfaceClassification(*state, 8);
EXPECT_EQ(state->dataSurface->SurfIntConvClassification(8), ConvectionConstants::InConvClass_A3_UnstableTilted);
EXPECT_EQ(state->dataSurface->SurfIntConvClassification(8), ConvectionConstants::InConvClass::A3_UnstableTilted);

DynamicIntConvSurfaceClassification(*state, 9);
EXPECT_EQ(state->dataSurface->SurfIntConvClassification(9), ConvectionConstants::InConvClass_A3_UnstableHoriz);
EXPECT_EQ(state->dataSurface->SurfIntConvClassification(9), ConvectionConstants::InConvClass::A3_UnstableHoriz);

DynamicIntConvSurfaceClassification(*state, 10);
EXPECT_EQ(state->dataSurface->SurfIntConvClassification(10), ConvectionConstants::InConvClass_A3_UnstableHoriz);
EXPECT_EQ(state->dataSurface->SurfIntConvClassification(10), ConvectionConstants::InConvClass::A3_UnstableHoriz);

DynamicIntConvSurfaceClassification(*state, 11);
EXPECT_EQ(state->dataSurface->SurfIntConvClassification(11), ConvectionConstants::InConvClass_A3_VertWalls);
EXPECT_EQ(state->dataSurface->SurfIntConvClassification(11), ConvectionConstants::InConvClass::A3_VertWalls);

DynamicIntConvSurfaceClassification(*state, 12);
EXPECT_EQ(state->dataSurface->SurfIntConvClassification(12), ConvectionConstants::InConvClass_A3_StableTilted);
EXPECT_EQ(state->dataSurface->SurfIntConvClassification(12), ConvectionConstants::InConvClass::A3_StableTilted);

DynamicIntConvSurfaceClassification(*state, 13);
EXPECT_EQ(state->dataSurface->SurfIntConvClassification(13), ConvectionConstants::InConvClass_A3_StableTilted);
EXPECT_EQ(state->dataSurface->SurfIntConvClassification(13), ConvectionConstants::InConvClass::A3_StableTilted);

DynamicIntConvSurfaceClassification(*state, 14);
EXPECT_EQ(state->dataSurface->SurfIntConvClassification(14), ConvectionConstants::InConvClass_A3_StableHoriz);
EXPECT_EQ(state->dataSurface->SurfIntConvClassification(14), ConvectionConstants::InConvClass::A3_StableHoriz);

DynamicIntConvSurfaceClassification(*state, 15);
EXPECT_EQ(state->dataSurface->SurfIntConvClassification(15), ConvectionConstants::InConvClass_A3_StableHoriz);
EXPECT_EQ(state->dataSurface->SurfIntConvClassification(15), ConvectionConstants::InConvClass::A3_StableHoriz);
}

TEST_F(ConvectionCoefficientsFixture, EvaluateIntHcModelsFisherPedersen)
Expand Down

5 comments on commit c59ca83

@nrel-bot-2c
Copy link

Choose a reason for hiding this comment

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

conv-coeff-enums-2 (mitchute) - x86_64-Linux-Ubuntu-18.04-gcc-7.5: OK (3145 of 3146 tests passed, 694 test warnings)

Messages:\n

  • 695 tests had: AUD diffs.
  • 681 tests had: RDD diffs.
  • 2 tests had: ERR diffs.
  • 2 tests had: MTD diffs.
  • 1 test had: EIO diffs.
  • 1 test had: ESO small diffs.
  • 1 test had: MTR small diffs.
  • 1 test had: Table big diffs.

Failures:\n

regression Test Summary

  • Passed: 741
  • Failed: 1

Build Badge Test Badge

@nrel-bot
Copy link

Choose a reason for hiding this comment

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

conv-coeff-enums-2 (mitchute) - Win64-Windows-10-VisualStudio-16: OK (2355 of 2355 tests passed, 0 test warnings)

Build Badge Test Badge

@nrel-bot-2b
Copy link

Choose a reason for hiding this comment

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

conv-coeff-enums-2 (mitchute) - x86_64-Linux-Ubuntu-18.04-gcc-7.5-UnitTestsCoverage-Debug: OK (1660 of 1660 tests passed, 0 test warnings)

Build Badge Test Badge Coverage Badge

@nrel-bot-2c
Copy link

Choose a reason for hiding this comment

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

conv-coeff-enums-2 (mitchute) - x86_64-Linux-Ubuntu-18.04-gcc-7.5-IntegrationCoverage-Debug: OK (726 of 726 tests passed, 0 test warnings)

Build Badge Test Badge Coverage Badge

@nrel-bot-3
Copy link

Choose a reason for hiding this comment

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

conv-coeff-enums-2 (mitchute) - x86_64-MacOS-10.15-clang-11.0.0: OK (3105 of 3105 tests passed, 691 test warnings)

Messages:\n

  • 691 tests had: AUD diffs.
  • 677 tests had: RDD diffs.
  • 2 tests had: ERR diffs.
  • 2 tests had: MTD diffs.

Build Badge Test Badge

Please sign in to comment.