Global Boilers and Chillers #7968

mitchute · 2020-05-05T15:29:31Z

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.

Myoldmopar · 2020-05-06T01:26:31Z

I didn't wait on the build to finish before pushing, but I think that takes care of it. The includes went circular. DataPlant.hh was including the plant topology, and deep down in the plant topology classes, it included the big Data.hh file. I moved those to forward declarations and that shouldn't happen anymore for plant components now.

Myoldmopar · 2020-05-06T13:29:19Z

There we go, that's nice and green

mitchute · 2020-05-07T14:29:29Z

The windows unit test failure passes on my VM. I won't worry about it any further at this point.

mjwitte · 2020-05-07T14:50:40Z

@mitchute Yeah, these have been showing up randomly on the Windows CI lately.
unknown file: error: C++ exception with description "InitializeMeters: Could not open file eplusout.mtd for output (write)." thrown in the test body.

Myoldmopar · 2020-05-07T15:28:09Z

I think once we get the mtd file converted over to the new OutputFiles structure, these will go away.

mitchute

Another step towards controlling the global state of E+. A nice pattern is beginning to emerge here.

mitchute · 2020-05-11T19:17:01Z

src/EnergyPlus/BoilerSteam.hh


        void onInitLoopEquip(EnergyPlusData &EP_UNUSED(state), const PlantLocation &EP_UNUSED(calledFromLocation)) override;

-        static PlantComponent *factory(std::string const &objectName);
+        static PlantComponent *factory(BoilerSteamData &boilers, std::string const &objectName);


Boiler and chiller data classes now being passed into the factory and GetInput methods.

That's interesting. I didn't think of that side-effect of this. We'll be passing a reference to the state around to all the factories. It makes me wonder if the factories will eventually become part of the state data classes. Pondering, pondering...

mitchute · 2020-05-11T19:19:14Z

src/EnergyPlus/Boilers.cc

@@ -233,75 +201,76 @@ namespace Boilers {
            // ErrorsFound will be set to True if problem was found, left untouched otherwise
            GlobalNames::VerifyUniqueBoilerName(
                DataIPShortCuts::cCurrentModuleObject, DataIPShortCuts::cAlphaArgs(1), ErrorsFound, DataIPShortCuts::cCurrentModuleObject + " Name");
-            Boiler(BoilerNum).Name = DataIPShortCuts::cAlphaArgs(1);
-            Boiler(BoilerNum).TypeNum = DataPlant::TypeOf_Boiler_Simple;
+            auto &thisBoiler = boilers.Boiler(BoilerNum);


Adding the state class reference to the front of everything was starting to get cumbersome to look at. auto & added where possible.

mitchute · 2020-05-11T19:21:10Z

src/EnergyPlus/Boilers.hh

-    // MODULE VARIABLE DECLARATIONS:
-    extern int NumBoilers;          // Number of boilers
-    extern bool GetBoilerInputFlag; // Boiler input flag, false if input is processed
+    enum class TempMode


New enum to get rid of some of these externs. The other boilers don't appear to need anything like this, so I left it in the header file.

mitchute · 2020-05-11T19:25:09Z

src/EnergyPlus/ChillerAbsorption.cc


-    int const waterIndex(1);
+    const char * fluidNameWater = "WATER";


Better than string literals all over the place? 🤷‍♂️

The following was suggested,

constexpr char * fluidName = "Water";

but it throws warnings on my machine.

warning: ISO C++11 does not allow conversion from string literal to 'char *const' [-Wwritable-strings]

I think it's better than string literals. Using a pointer to a constant character makes good sense until we come up with anything better. The warning is because a string literal is by definition a constant character string. It cannot be changed. The way you've declared it here, it's all fine, you have a pointer to a const char (const char *). The constexpr way was trying to make it a constant pointer to a non-constant char (char * const), which won't work because the string literal is constant.

mitchute · 2020-05-11T19:26:17Z

src/EnergyPlus/ChillerAbsorption.cc


-        BLASTAbsorber.allocate(numBlastAbsorbers);
+        chillers.BLASTAbsorber.allocate(chillers.numBlastAbsorbers);


Missed auto & here. Add it?

No, the allocate call just returns a reference to the original object, no need to capture it (in our use cases, anyway).

mitchute · 2020-05-11T19:30:49Z

src/EnergyPlus/CostEstimateManager.cc

-    // Functions
-
-    void SimCostEstimate()
+    void SimCostEstimate(EnergyPlusData &state)


A few new &state references in places.

mitchute · 2020-05-11T19:35:10Z

src/EnergyPlus/Data/EnergyPlusData.hh

@@ -68,6 +79,107 @@ namespace EnergyPlus {
 //    }
 //};

+    struct BoilersData : BaseGlobalStruct {


Most of these boilers and chiller data classes only contain three members: a getInput flag, an integer numObjects member, and a 1D array of objects. These should probably get consolidated later.

This is going to prove to be exceedingly common as we move through plant components.

mitchute · 2020-05-11T19:36:18Z

src/EnergyPlus/Plant/DataPlant.hh

@@ -261,6 +261,23 @@ namespace DataPlant {
    extern int PlantManageSubIterations; // tracks plant iterations to characterize solver
    extern int PlantManageHalfLoopCalls; // tracks number of half loop calls

+    // Enum classes


New enums I mentioned earlier.

mitchute · 2020-05-11T19:37:22Z

src/EnergyPlus/PlantChillers.cc

-        GTChiller.deallocate();
-        ConstCOPChiller.deallocate();
-    }
+    Real64 constexpr KJtoJ(1000.0); // convert Kjoules to joules


constexpr for numerical constants. There's probably something similar for the string constants.

mitchute · 2020-05-11T19:39:20Z

src/EnergyPlus/StateManagement.cc

@@ -250,16 +247,8 @@ void EnergyPlus::clearAllStates()
    AirflowNetworkBalanceManager::clear_state();
    BaseboardElectric::clear_state();
    BaseboardRadiator::clear_state();
-    Boilers::clear_state();


All of this now happens over in the state data class.

Myoldmopar

This is just a great effort. The only thing that I really want to fix up before merging is the pointer usage in the unit test code. It will be a very quick change to just make it a reference instead. It could be nice to propagate that same change elsewhere in the unit tests, but that doesn't have to be here. I may make those changes though.

Myoldmopar · 2020-05-13T14:09:51Z

src/EnergyPlus/BoilerSteam.hh


        void onInitLoopEquip(EnergyPlusData &EP_UNUSED(state), const PlantLocation &EP_UNUSED(calledFromLocation)) override;

-        static PlantComponent *factory(std::string const &objectName);
+        static PlantComponent *factory(BoilerSteamData &boilers, std::string const &objectName);


That's interesting. I didn't think of that side-effect of this. We'll be passing a reference to the state around to all the factories. It makes me wonder if the factories will eventually become part of the state data classes. Pondering, pondering...

Myoldmopar · 2020-05-13T14:12:57Z

src/EnergyPlus/Boilers.hh

-    // MODULE VARIABLE DECLARATIONS:
-    extern int NumBoilers;          // Number of boilers
-    extern bool GetBoilerInputFlag; // Boiler input flag, false if input is processed
+    enum class TempMode


Myoldmopar · 2020-05-13T14:15:31Z

src/EnergyPlus/ChillerAbsorption.cc


-    int const waterIndex(1);
+    const char * fluidNameWater = "WATER";


I think it's better than string literals. Using a pointer to a constant character makes good sense until we come up with anything better. The warning is because a string literal is by definition a constant character string. It cannot be changed. The way you've declared it here, it's all fine, you have a pointer to a const char (const char *). The constexpr way was trying to make it a constant pointer to a non-constant char (char * const), which won't work because the string literal is constant.

Myoldmopar · 2020-05-13T14:16:54Z

src/EnergyPlus/ChillerAbsorption.cc

            ShowSevereError("No " + DataIPShortCuts::cCurrentModuleObject + " equipment specified in input file");
            // See if load distribution manager has already gotten the input
            ErrorsFound = true;
        }

-        if (allocated(BLASTAbsorber)) return;
+        if (allocated(chillers.BLASTAbsorber)) return;


You are welcome to change variable names like BLASTAbsorber if you want. You can ping me or others if you want an idea, but in this case, I think it would be just more clear to just say chillers.absorptionChillers or something. No big deal for now though, carry on!

Myoldmopar · 2020-05-13T14:20:05Z

tst/EnergyPlus/unit/ChillerAbsorption.unit.cc

@@ -1784,42 +1783,43 @@ TEST_F(EnergyPlusFixture, ChillerAbsorption_Calc)
    Real64 AbsChillEvapLoad;
    bool AbsChillRunFlag = true;
    // check chiller inputs
-    EXPECT_EQ(BLASTAbsorber(AbsChillNum).NomCap, 100000.0);
-    EXPECT_EQ(BLASTAbsorber(AbsChillNum).FlowMode, ChillerAbsorption::LeavingSetPointModulated);
+    auto thisChiller = &state.dataChillerAbsorbers.BLASTAbsorber(AbsChillNum);


This is creating a pointer. While I'm not afraid of pointers, I think it would be good to avoid this pattern too broadly. I would suggest making the subtle change:

auto thisChiller = &state.dataChillerAbsorbers.BLASTAbsorber(AbsChillNum); to auto & thisChiller = state.dataChillerAbsorbers.BLASTAbsorber(AbsChillNum);

This way you are creating a reference instead of a pointer.

Myoldmopar · 2020-05-13T14:21:14Z

tst/EnergyPlus/unit/ChillerConstantCOP.unit.cc


    DataPlant::PlantLoop(1).Name = "ChilledWaterLoop";
    DataPlant::PlantLoop(1).FluidName = "ChilledWater";
    DataPlant::PlantLoop(1).FluidIndex = 1;
    DataPlant::PlantLoop(1).PlantSizNum = 1;
    DataPlant::PlantLoop(1).FluidName = "WATER";
-    DataPlant::PlantLoop(1).LoopSide(1).Branch(1).Comp(1).Name = ConstCOPChiller(1).Name;
+    DataPlant::PlantLoop(1).LoopSide(1).Branch(1).Comp(1).Name = state.dataPlantChillers.ConstCOPChiller(1).Name;


Creating another reference to this chiller is a great way to clean up this test like you did with the absorber.

Myoldmopar · 2020-05-13T14:26:10Z

src/EnergyPlus/Data/EnergyPlusData.hh

@@ -140,21 +252,73 @@ namespace EnergyPlus {
        }
    };

+    struct PlantChillersData : BaseGlobalStruct {


Yes. This class should be split into four different ones. Sorta. Each one will again only have a num integer, getinput bool, and an array of objects. I think I see where this is going for reals.

Myoldmopar

All good here. Thanks for the quick fixes.

Myoldmopar · 2020-05-13T17:49:56Z

src/EnergyPlus/ChillerAbsorption.cc

@@ -120,7 +120,7 @@ namespace ChillerAbsorption {
            chillers.getInput = false;
        }
        // Now look for this particular object
-        for (auto &thisAbs : chillers.BLASTAbsorber) {
+        for (auto &thisAbs : chillers.absorptionChillers) {


Yeah, that's nicer 😄

Myoldmopar · 2020-05-13T17:50:13Z

tst/EnergyPlus/unit/ChillerAbsorption.unit.cc

-    auto thisChiller = &state.dataChillerAbsorbers.BLASTAbsorber(AbsChillNum);
-    EXPECT_EQ(thisChiller->NomCap, 100000.0);
-    EXPECT_EQ(thisChiller->FlowMode, DataPlant::FlowMode::LEAVINGSETPOINTMODULATED);
+    auto &thisChiller = state.dataChillerAbsorbers.absorptionChillers(AbsChillNum);


Yes. Thanks!

Myoldmopar · 2020-05-13T17:51:18Z

@mitchute I then #7972 will merge in shortly, then I'll pull develop into this branch locally and run regressions. Assuming all is well I think this will go in directly.

Myoldmopar · 2020-05-14T14:10:48Z

Pulled develop in and ran regressions overnight. All good here. Merging.

move boilers data to EnergyPlusData

f016b5c

mitchute added the Refactoring Includes code changes that don't change the functionality of the program, just perform refactoring label May 5, 2020

mitchute added this to the EnergyPlus 9.4.0 milestone May 5, 2020

mitchute self-assigned this May 5, 2020

mitchute added 3 commits May 5, 2020 11:14

steam boilers data to EnergyPlusData

052d189

absorption chiller data to EnergyPlusData

8af176e

broken - moving FlowMode enum from boiler to DataPlant

2ef9854

mitchute changed the title ~~Global Boilers~~ Global Boilers and Chillers May 5, 2020

Decircularize

d70b626

Fix unit test flowmode usage

7aeb723

mitchute added 10 commits May 6, 2020 08:40

move condenser type and flow mode chiller options to enum in DataPlant

8068254

ChillerElectricEIR data to EnergyPlusData

87c9dcc

ChillerExhaustAbsorption data to EnergyPlusData

d5cceaf

cleanup boiler naming

7cbbf21

ChillerGasAbsorption data to EnergyPlusData

a33d108

ChillerIndirectAbsorption data to EnergyPlusData

c7d7fca

ChillerReformulatedEIR data to EnergyPlusData

11fc1cc

move PlantChiller FlowMode and CondenserType to enums in DataPlant

c9335d5

Merge branch 'develop' into global_boiler

76bd1c1

PlantChillers data to EnergyPlusData

f2df6db

mitchute added 2 commits May 11, 2020 12:29

Merge branch 'develop' into global_boiler

79140ce

fixup string literals

02ccc64

mitchute commented May 11, 2020

View reviewed changes

mitchute mentioned this pull request May 13, 2020

Global Baseboards #7989

Merged

20 tasks

Myoldmopar requested changes May 13, 2020

View reviewed changes

mitchute added 2 commits May 13, 2020 08:40

Merge branch 'develop' into global_boiler

b6dab27

cleanup pointer, cleanup references, minor renaming

b9dfdee

Myoldmopar approved these changes May 13, 2020

View reviewed changes

Myoldmopar merged commit be87902 into develop May 14, 2020

Myoldmopar deleted the global_boiler branch May 14, 2020 14:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Global Boilers and Chillers #7968

Global Boilers and Chillers #7968

mitchute commented May 5, 2020

Myoldmopar commented May 6, 2020

Myoldmopar commented May 6, 2020

mitchute commented May 7, 2020

mjwitte commented May 7, 2020

Myoldmopar commented May 7, 2020

mitchute left a comment

mitchute May 11, 2020

Myoldmopar May 13, 2020

mitchute May 11, 2020

mitchute May 11, 2020

Myoldmopar May 13, 2020

mitchute May 11, 2020

Myoldmopar May 13, 2020

mitchute May 11, 2020

Myoldmopar May 11, 2020

mitchute May 11, 2020

mitchute May 11, 2020

Myoldmopar May 11, 2020

mitchute May 11, 2020

mitchute May 11, 2020

mitchute May 11, 2020

Myoldmopar left a comment

Myoldmopar May 13, 2020

Myoldmopar May 13, 2020

Myoldmopar May 13, 2020

Myoldmopar May 13, 2020

Myoldmopar May 13, 2020

Myoldmopar May 13, 2020

Myoldmopar May 13, 2020

Myoldmopar left a comment

Myoldmopar May 13, 2020

Myoldmopar May 13, 2020

Myoldmopar commented May 13, 2020

Myoldmopar commented May 14, 2020


		int const waterIndex(1);
		const char * fluidNameWater = "WATER";


		BLASTAbsorber.allocate(numBlastAbsorbers);
		chillers.BLASTAbsorber.allocate(chillers.numBlastAbsorbers);

Global Boilers and Chillers #7968

Global Boilers and Chillers #7968

Conversation

mitchute commented May 5, 2020

Pull Request Author

Reviewer

Myoldmopar commented May 6, 2020

Myoldmopar commented May 6, 2020

mitchute commented May 7, 2020

mjwitte commented May 7, 2020

Myoldmopar commented May 7, 2020

mitchute left a 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

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 left a 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 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Myoldmopar commented May 13, 2020

Myoldmopar commented May 14, 2020