Remove `select_case_var`s & `enum class` cleanup #9385

dareumnam · 2022-04-14T15:09:17Z

Pull request overview

Remove select_case_vars in
- Autosizing\CoolingAirFlowSizing
- Autosizing\HeatingAirFlowSizing.cc
- Autosizing\HeatingCapacitySizing.cc
- DataHeatBalance.cc
- Datasizing.cc
- DesiccantDehumidifiers.cc
- DXCoils.cc
- FanCoilUnits.cc
- Furnaces.cc
- HeatBalanceSurfaceManager.cc
- HeatingCoils.cc
- Humidifiers.cc
enum class cleanup

NOTE: ENHANCEMENTS MUST FOLLOW A SUBMISSION PROCESS INCLUDING A FEATURE PROPOSAL AND DESIGN DOCUMENT PRIOR TO SUBMITTING CODE

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.

…mclasses

…ntDehumidifiers

…mclasses

amirroth · 2022-04-17T20:28:42Z

src/EnergyPlus/Autosizing/CoolingAirFlowSizing.cc

-                auto const SELECT_CASE_var(this->zoneEqSizing(this->curZoneEqNum).SizingMethod(DataHVACGlobals::CoolingAirflowSizing));
-                if ((SELECT_CASE_var == DataSizing::SupplyAirFlowRate) || (SELECT_CASE_var == DataSizing::None) ||
-                    (SELECT_CASE_var == DataSizing::FlowPerFloorArea)) {
+                switch (this->zoneEqSizing(this->curZoneEqNum).SizingMethod(DataHVACGlobals::CoolingAirflowSizing)) {


I'm not going to harp on this because I want this stuff merged, but this is a good candidate for
auto & thisZoneEqSizing = this->zoneEqSizing(this->curZoneEqNum); etc.

Actually, given what we are seeing in the debug builds and the desire to move to EPVector I will be harping on this more. 😁

amirroth · 2022-04-17T20:30:36Z

src/EnergyPlus/Autosizing/SystemAirFlowSizing.cc

-                            auto const SELECT_CASE_var(this->curDuctType);
-                            if (SELECT_CASE_var == DataHVACGlobals::Main) {
+                            switch (this->curDuctType) {
+                            case DataHVACGlobals::Main: {


Main, Other, and default are all the same case. Can be combined.

amirroth · 2022-04-17T20:37:24Z

src/EnergyPlus/Autosizing/SystemAirFlowSizing.cc

                            }
                        }
+                    } else {
+                        switch (this->curDuctType) {


And also here.

amirroth · 2022-04-17T20:38:20Z

src/EnergyPlus/DXCoils.cc

+            state, DXCoilNum, CompressorOperation::On, FirstHVACIteration, PartLoadRatio, FanOpMode, CompCycRatio, _, AirFlowRatio, MaxCap);
+    } break;
+    case CoilVRF_Heating: {
+        CalcDXHeatingCoil(state, DXCoilNum, PartLoadRatio, FanOpMode, AirFlowRatio, MaxCap);


Should this be VRF?

The VRF heating coil model re-uses the old DX heating coil calc routine. The VRF cooling coil has a few different equations so that coil type has it's own calc routine although we could probably incorporate new routine equations into the old if needed.

amirroth · 2022-04-17T20:39:51Z

src/EnergyPlus/DXCoils.cc

+
+    } break;
+    default: {
+        ShowSevereError(state, "Error detected in DX Coil=" + std::string{CompName});


Mmmmm ... could this be an assert? Wouldn't getInput have caught this?

amirroth · 2022-04-17T20:46:22Z

src/EnergyPlus/DXCoils.cc

-                               state.dataDXCoils->DXCoil(DXCoilNum).DXCoilType + " \"" + state.dataDXCoils->DXCoil(DXCoilNum).Name +
-                                   "\" - Requested enhanced dehumidification mode not available.");
-            }
+    if (state.dataDXCoils->DXCoil(DXCoilNum).DXCoilType_Num == CoilDX_CoolingTwoStageWHumControl) {


auto & dxCoil = state.dataDXCoils->DXCoils(DXCoilNum);

amirroth · 2022-04-17T20:51:30Z

src/EnergyPlus/DataHeatBalance.cc

+    default: {
+        if (state.dataSurface->Surface(numSurf).ExtBoundCond > 0) { // interzone partition
+            // use companion surface in adjacent zone
+            switch (state.dataSurface->Surface(state.dataSurface->Surface(numSurf).ExtBoundCond).Class) {


Can probably make this into a lookup std::array.

amirroth · 2022-04-17T20:57:05Z

src/EnergyPlus/DesiccantDehumidifiers.cc

-                            Par[1] = 1.0;
-                        } else {
-                            Par[1] = 0.0;
+            switch (DesicDehum(DesicDehumNum).RegenCoilType_Num) {


auto & desicDehum = state.dataDesicDehum->DesicDehum(DesicDehumNum); is a much more useful shortcut than auto & DesicDehum = state.dataDesicDehum->DesicDehum;. In general, don't have shortcut references to arrays, use them for individual elements.

amirroth · 2022-04-17T20:57:48Z

src/EnergyPlus/DesiccantDehumidifiers.cc

+                        Par[1] = 0.0;
+                    }
+                    Par[2] = RegenCoilLoad;
+                    General::SolveRoot(


📌 Replace this templated function with actual lambdas. There is an issue about this.

amirroth · 2022-04-17T20:58:55Z

src/EnergyPlus/HeatingCoils.cc

-            } else if (SELECT_CASE_var == Coil_HeatingDesuperheater) {
-                coilObjClassName = "Coil:Heating:Desuperheater";
-            }
+        switch (state.dataHeatingCoils->HeatingCoil(CoilNum).HCoilType_Num) {


📌 Should be able to get these from the constexpr std::array<std::string_view> once these are converted to enum class.

amirroth · 2022-04-17T21:00:31Z

src/EnergyPlus/ZoneTempPredictorCorrector.hh

-        SPE = 1, // Specific people object
-        OBJ = 2, // People object average
-        PEO = 3, // People number average
+        NO,  // No multiple people objects


These names!

Myoldmopar · 2022-04-19T18:25:21Z

Tested this with develop too and everything is good. There are a few comments here from @amirroth that should be kept for the next round so that this can merge in. Is that OK @amirroth or should we address more things in this branch?

dareumnam · 2022-04-19T19:18:07Z

@Myoldmopar @amirroth I'll make a separate follow-up branch to pick up all comments here.

Myoldmopar · 2022-04-19T19:24:41Z

Sounds like a plan to me @dareumnam, let's get this merged in, thanks!

amirroth · 2022-04-19T19:40:33Z

src/EnergyPlus/HeatingCoils.cc

-                    state, state.dataOutRptPredefined->pdchHeatCoilNomCap, HeatingCoil(CoilNum).Name, HeatingCoil(CoilNum).NominalCapacity);
-                PreDefTableEntry(state, state.dataOutRptPredefined->pdchHeatCoilNomEff, HeatingCoil(CoilNum).Name, HeatingCoil(CoilNum).Efficiency);
-            }
+        switch (HeatingCoil(CoilNum).HCoilType_Num) {


auto & heatingCoil = state.dataHeatingCoils->HeatingCoil(CoilNum);

amirroth · 2022-04-19T19:43:39Z

src/EnergyPlus/UserDefinedComponents.cc

-                            } else if (SELECT_CASE_var == "NO") {
-                                state.dataUserDefinedComponents->UserCoil(CompLoop).PlantIsConnected = false;
-                            }
+                        if (cAlphaArgs(8) == "YES") {


📌 There should probably be a function for this like bool getBoolValue(std::string_view str).

amirroth · 2022-04-19T19:44:53Z

Tested this with develop too and everything is good. There are a few comments here from @amirroth that should be kept for the next round so that this can merge in. Is that OK @amirroth or should we address more things in this branch?

Please don't hold this up.

dareumnam added 9 commits April 7, 2022 09:53

enum classes

37c664e

task2 cleanup

b479871

removing select_case_vars in objects in Autosizing folder

c558174

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

4f5fc57

…mclasses

removing select_case_vars in DataHeatBalance, Datasizing, and Desicca…

2fc7371

…ntDehumidifiers

DXCoils

56dd037

fix warnings

17c748c

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

255a8bb

…mclasses

removing select_case_vars 2

90842a7

dareumnam added the Refactoring Includes code changes that don't change the functionality of the program, just perform refactoring label Apr 14, 2022

dareumnam added this to the EnergyPlus 22.2 milestone Apr 14, 2022

dareumnam self-assigned this Apr 14, 2022

amirroth reviewed Apr 17, 2022

View reviewed changes

src/EnergyPlus/Autosizing/SystemAirFlowSizing.cc

}

}

} else {

switch (this->curDuctType) {

Copy link

Collaborator

amirroth Apr 17, 2022

Choose a reason for hiding this comment

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

And also here.

amirroth reviewed Apr 17, 2022

View reviewed changes

Myoldmopar merged commit c2a7f60 into develop Apr 19, 2022

Myoldmopar deleted the enumclasses branch April 19, 2022 19:24

amirroth reviewed Apr 19, 2022

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove `select_case_var`s & `enum class` cleanup #9385

Remove `select_case_var`s & `enum class` cleanup #9385

dareumnam commented Apr 14, 2022

amirroth Apr 17, 2022

amirroth Apr 19, 2022

amirroth Apr 17, 2022

amirroth Apr 17, 2022

amirroth Apr 17, 2022

rraustad Apr 18, 2022

amirroth Apr 17, 2022

amirroth Apr 17, 2022

amirroth Apr 17, 2022

amirroth Apr 17, 2022

amirroth Apr 17, 2022

amirroth Apr 17, 2022

amirroth Apr 17, 2022

Myoldmopar commented Apr 19, 2022

dareumnam commented Apr 19, 2022

Myoldmopar commented Apr 19, 2022

amirroth Apr 19, 2022

amirroth Apr 19, 2022

amirroth commented Apr 19, 2022

Remove select_case_vars & enum class cleanup #9385

Remove select_case_vars & enum class cleanup #9385

Conversation

dareumnam commented Apr 14, 2022

Pull request overview

Pull Request Author

Reviewer

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Myoldmopar commented Apr 19, 2022

dareumnam commented Apr 19, 2022

Myoldmopar commented Apr 19, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

amirroth commented Apr 19, 2022

Remove `select_case_var`s & `enum class` cleanup #9385

Remove `select_case_var`s & `enum class` cleanup #9385