SetPointManager Refactor #10537

amirroth · 2024-05-31T02:16:48Z

Object-oriented refactor of the SetPointManager module. The original module had a class hierarchy but instances of each class were contained in separate arrays resulting in a significant amount of redundant code. The new implementation uses a single array of pointers to objects of all SetPointManager sub-classes, enabling significant code reuse. Overall, almost 6,000 lines of code (65%) were removed from SetPointManager.[cc|hh]. This was an especially redundant module and so savings of this magnitude may not be possible elsewhere, but savings of about half that should be. We'll have to see.

Some highlights:

Changed input processing approach from IDF-based to epJSON-based, this enabled significant code savings in the GetInput routine.
In the new GetInput routine, there is code that is guarded by a PRESERVE_IDF_ORDER macro. This establishes a template for preserving IDF order in epJSON-based input code. The approach is the same as the one used in getObjectItem (thanks @mjwitte).
Streamlined default value handling getRealFieldValue and getAlphaFieldValue.
Moved some state variables to local.
Added duplicate name check and fixed one test file to comply.

CI is green except for a few diffs in some .bnd files. These are caused by the fact that SetPointManager input data is now read and processed only once. In some simulations it used to be processed twice, once in InternalGains and then again later in ManageHVAC. Double processing caused differences in the number of times LoopNode objects were referenced. I suspect that this is also why there wasn't a duplicate name check. When input is processed twice, every name is a duplicate.

amirroth · 2024-05-31T02:19:03Z

performance_tests/15zonePVAV.idf

@@ -7872,7 +7872,7 @@
    PACKAGED VAV WITH REHEAT Supply Equipment Outlet Node;  !- Setpoint Node or NodeList Name

  SetpointManager:MixedAir,
-    PACKAGED VAV WITH REHEAT_HeatC SAT Manager,  !- Name
+    PACKAGED VAV WITH REHEAT_HeatC SAT Manager MA,  !- Name


Duplicate SetPointManager names are no longer allowed.

I changed the labels on this PR to include IDDChange, because this is changing input requirements even though the idd itself isn't changing. Will need transition for this.

amirroth · 2024-05-31T02:19:30Z

src/EnergyPlus/AirflowNetwork/src/Elements.cpp

@@ -699,9 +699,6 @@ namespace AirflowNetwork {
        // Using/Aliasing
        auto &NumPrimaryAirSys = state.dataHVACGlobal->NumPrimaryAirSys;

-        // SUBROUTINE PARAMETER DEFINITIONS:
-        int constexpr CycFanCycComp(1); // fan cycles with compressor operation
-


This was not used.

amirroth · 2024-05-31T02:20:18Z

src/EnergyPlus/BoilerSteam.cc

@@ -338,8 +338,7 @@ namespace BoilerSteam {
            } else {
                // need call to EMS to check node
                bool FatalError = false; // but not really fatal yet, but should be.
-                EMSManager::CheckIfNodeSetPointManagedByEMS(
-                    state, this->BoilerOutletNodeNum, EMSManager::SPControlType::TemperatureSetPoint, FatalError);
+                EMSManager::CheckIfNodeSetPointManagedByEMS(state, this->BoilerOutletNodeNum, HVAC::CtrlVarType::Temp, FatalError);


Unified a few redundant types to HVAC::CtrlVarType.

amirroth · 2024-05-31T02:21:14Z

src/EnergyPlus/DataEnvironment.hh

@@ -62,6 +62,16 @@ struct EnergyPlusData;

 namespace DataEnvironment {

+    enum class GroundTempType


Made the different GroundTemp field groups into std::arrays indexed by this enum.

amirroth · 2024-05-31T02:24:10Z

src/EnergyPlus/InputProcessing/InputProcessor.cc

-        json const alphas = legacy_idd["alphas"];
-        if (alphas.find("fields") != alphas.end()) {
-            NumAlpha += alphas["fields"].size();
+    if (auto found = legacy_idd.find("alphas"); found != legacy_idd.end()) {


Eliminate redundant lookups.

amirroth · 2024-05-31T02:41:54Z

src/EnergyPlus/SetPointManager.cc

-    // The System Node Reset Humidity Setpoint Managers
-    for (SetPtMgrNum = 1; SetPtMgrNum <= state.dataSetPointManager->NumSystemNodeResetHumSetPtMgrs; ++SetPtMgrNum) {
-        state.dataSetPointManager->SystemNodeResetSetPtMgr(SetPtMgrNum + state.dataSetPointManager->NumSystemNodeResetTempSetPtMgrs).calculate(state);
+    for (auto *spm : state.dataSetPointManager->spms) {


Ahhhhhhhhhh.

amirroth · 2024-05-31T02:44:15Z

src/EnergyPlus/SetPointManager.cc

    // SUBROUTINE LOCAL VARIABLE DECLARATIONS:
-    auto &CondWaterSetPoint = state.dataSetPointManager->CondWaterSetPoint;


Bad use of shortcut refs.

amirroth · 2024-05-31T02:47:12Z

src/EnergyPlus/SetPointManager.hh

@@ -66,61 +68,40 @@ struct EnergyPlusData;

 namespace SetPointManager {

-    enum class CtrlNodeType


Moved to DataHVACGlobals.

amirroth · 2024-05-31T02:48:59Z

src/EnergyPlus/SetPointManager.hh

    bool ManagerOn = false;
    bool GetInputFlag = true; // First time, input is "gotten"

    bool InitSetPointManagersOneTimeFlag = true;
    bool InitSetPointManagersOneTimeFlag2 = true;
-    Real64 DCESPMDsn_EntCondTemp = 0.0;


Most of these were actually local variables.

amirroth · 2024-05-31T02:50:13Z

tst/EnergyPlus/unit/HVACControllers.unit.cc

@@ -133,16 +133,19 @@ TEST_F(EnergyPlusFixture, HVACControllers_ResetHumidityRatioCtrlVarType)

    GetSetPointManagerInputs(*state);
    // check specified control variable type is "HumidityRatio"
-    ASSERT_TRUE(compare_enums(SetPointManager::CtrlVarType::HumRat, state->dataSetPointManager->AllSetPtMgr(1).CtrlTypeMode));
+    ASSERT_EQ((int)HVAC::CtrlVarType::HumRat, (int)state->dataSetPointManager->spms(1)->ctrlVar);


This is better than compare_enums. Just use this from now on.

I disagree. IMHO The custom function is clearer, and doesn't require casting, so you don't have to remember how to do it properly. But that's not even the real point here.

The point is that compare_enums will not let you shoot yourself in the foot. You cannot compare different enums with it, it will not compile.

If your beef is with the way the error message is structured, we can work on that to improve it.

And I'd avoid C-like cast, so at least it should be a static_cast, which gets verbose.

An alternative would be define the C++23 std::to_underlying: https://en.cppreference.com/w/cpp/utility/to_underlying

That's dead simple

// std::to_underlying only in C++23 template <class T> inline constexpr typename std::underlying_type_t<T> to_underlying(T val) noexcept { return static_cast<typename std::underlying_type<T>::type>(val); }

But it'd still won't ensure you use the right types

Demo: https://godbolt.org/z/57PYfrs1Y

How about we figure a way to have EXPECT_ENUM_EQ and a EXPECT_ENUM_NE macros.

You could use them like

auto v1 = EnumClass::val1; EXPECT_ENUM_EQ(v1, EnumClass::val2); EXPECT_ENUM_NE(v1, EnumClass::val1);

And they'd format to

/path/tofile.cpp:107: Failure In comparing enums of type 'EnumClass', Expected equality of these values: v1 Which is: 0 EnumClass::val2 Which is: 1 /path/tofile.cpp:110: Failure In comparing enums of type 'EnumClass' Expected: (v1) != (EnumClass::val1) Actual: both are 0

or maybe just

/path/tofile.cpp:107: Failure Expected equality of these values: v1 Which is: EnumClass(0) EnumClass::val2 Which is: EnumClass(1) /path/tofile.cpp:110: Failure Expected: (v1) != (EnumClass::val1) actual: both are EnumClass(0)

I'd be happy to work on that!

My beef is not with the error message, it's with the compare_enums(X1, X2, false) "gotcha", so I like the EXPECT_ENUM_EQ/EXPECT_ENUM_NE solution. We don't have to do it in this PR. I will revert these changes and we can do this separately. Thanks.

I reverted.

rraustad · 2024-05-31T21:03:53Z

src/EnergyPlus/SetPointManager.cc

+{
+    auto found = state.dataSetPointManager->spmMap.find(Name);
+    return (found != state.dataSetPointManager->spmMap.end()) ? found->second : 0;
+} // GetSetPointManagerIndex()


So this is the reason all SPMs must have a unique name? Not that having unique names is a bad thing, or that users should want to have unique names across all SPMs, just that a name/type would always be unique if a unique name is used within each class. I am only thinking about this because of UnitarySystem now managing more parent object classes and whether there is anything I can do about not forcing the requirement of unique names (and the new getIndex function added to UnitarySystem in #10452). I think I am landing on unique names being a good thing going forward. Why would you name a PTAC the same as a CoilSystem? or a scheduled SPM the same as a mixed air SPM? In fact, it may be a good thing to force unique object names for every object in E+.

Well, there are at least three things going on here. First, previously name uniqueness was not forced on SPMs at all. Not within an SPM sub-class, not across sub-classes. And I am guessing this is because some created a situation in which SPM input data was read twice and couldn't think of a way to resolve this issue other than disabling the check.

Second, I don't think we actually need to enforce name uniqueness within the instances of a certain IDF object because epJSON does that already. It uses the object names as keys and you can't have duplicate keys.

Third, the "convention" within EnergyPlus right now is to enforce name uniqueness within a "parent" class for lack of a better term. So, there is name uniqueness across all curves, and all schedules, and all chillers, etc. This is what "Global Names" is used for and this is what I did here. We currently do not enforce name uniqueness across an entire IDF file. We can start to do that, I am not sure how much that will help. Name uniqueness within the same class hierarchy is helpful because ultimately you are going to want to handle members of that hierarchy in a uniform way (so you can delete 65% of the code!) and that means storing them in the same data structures. If you have multiple objects of the same name in the same data structure, things can get a bit messy/verbose because then you need a secondary key everywhere to tell one from the other. But you're not going to run into that problem for objects that are not in the same class hierarchy. Having a Schedule and a Fan with the same name is not a problem internally because those two objects will never be handled by the same piece of code.

jmarrec

Couple of nitpicks (I know, no one asked for my opinion, sorry).

jmarrec · 2024-06-03T08:10:04Z

src/EnergyPlus/SetPointManager.cc

-                                                                                                "MINIMUMMASSFLOWRATE"};
-
-constexpr std::array<std::string_view, static_cast<int>(ControlStrategy::Num)> strategyNamesUC = {
+constexpr std::array<std::string_view, (int)HVAC::CtrlVarType::Num> ctrlVarTypeNames = {


Im curious why change the static_cast to a C-style cast?

C++ style cast are checked by the compiler, more restrictive, and can be searched for easily.

(Never use a C-style cast in C++ is my rule)

Your opinion is more than welcome. I don't mark you as a reviewer because I figured you have enough other things to do (as do I, but that's a different story), but if you have the bandwidth and interest.

As for this, this is the only idiom (enum as an array index) where I use C-style casts, and it's mostly for readability. My presumption is that all enums are implemented as ints by the compiler. In my 30+ years of programming, I have never seen them implemented any other way, so the assumption is that this upcast is safe even without a check. Furthermore, if you are using an enum as a std::array index, you are already explicitly saying that you are using the enum as an it.

Note, I agree with you about the EXPECT_EQ thing. That's a different idiom, and using int casts there was a bridge too far. But I don't see a problem with them here in this idiom.

I understand the rationale.

For completeness, The default is that C++ enum class uses integer, C enums are unsigned. In other words:

enum e1 {}; <=> enum e1: unsigned {} enum class e2 {}; <=> enum e2: int (};

If you use the enum value for indexing into a std::array (and you can only do that because we do not have the related conversion compiler warnings enabled), you're actually saying myarray[static_cast<std::size_t>(myEnumValue)].

Which is probably fine in E+ since we typically go from Invalid = -1 to something small anyways, except if you were use the Invalid = -1 we have, and do myarray[MyEnum::Invalid]. static_cast<size_t>(-1) gives 18,446,744,073,709,551,616, which is definitely going to raise an out-of-bounds array indexing for our enums, but in a debug build only. In release, this is UB.

There are actually two kinds of enums in EnergyPlus. Those that come from the IDF file, and those that are used only internally. The ones used internally should never be set to Invalid and probably don't even need an Invalid value. The ones that come from EnergyPlus probably also don't need an Invalid value either because @rraustad loves to remind me, those get trapped in the epJson parser and never get to EnergyPlus proper.

You can go ahead and enable the compiler warnings. In fact, I see that somehow a -w snuck into flags.cmake in energypluslib.dir which you probably want to get rid of while you are at it.

jmarrec · 2024-06-03T08:20:29Z

src/EnergyPlus/SetPointManager.hh

-        DefineSZMinHumSetPointManager() : NumZones(0), NumCtrlNodes(0), SetPt(0.0)
-        {
-        }
+        int schedNum = 0;

        void calculate(EnergyPlusData &state);


Suggested change

void calculate(EnergyPlusData &state);

void calculate(EnergyPlusData &state) override;

Same comment applies to all of them deriving from SPMBase.

Your code works, but marking it override has two advantages:

It's clearer that's what your doing, and clearer is good.

The compiler will be nice enough to yell at you if you mess up the function signature and there's no such parent function to override.

(I do realize in this specific case since your parent virtual function is a pure virtual one, so if you messed it up the compiler would still yell at you, but still)

TIL why it's good to use override. It always seemed nonsensical to me. Thanks!

jmarrec · 2024-06-03T08:21:48Z

src/EnergyPlus/SetPointManager.hh

    enum class SupplyFlowTempStrategy
    {
        Invalid = -1,
        MaxTemp,
        MinTemp,
        Num
    };
+


Suggested change

Trailing whitespaces in that file.

jmarrec · 2024-06-03T08:43:16Z

src/EnergyPlus/SetPointManager.hh

-    Real64 TotEnergy = 0;         // Total energy consumptions at this time step
-    Real64 TotEnergyPre = 0;      // Total energy consumptions at the previous time step
-
+    Array1D<SetPointManager::SPMBase *> spms;


I'm pretty sure you picked a raw pointer on purpose after consideration, but in the off chance not, I'd prefer using std::unique_ptr<SPMBase>. (And maybe a std::array or EPVector to contain it?)

"But performance" you may say. well,

https://quick-bench.com/q/9xIjZoQqSyiz-uhlVFTFpN7fsn0

Locally I get more similar results, but still, my point is that this does not hinder performance.

I'm pretty sure you picked a raw pointer on purpose after consideration, but in the off chance not, I'd prefer using std::unique_ptr<SPMBase>. (And maybe a std::array or EPVector to contain it?)

I did use a raw pointer on consideration. What is the consideration for std::unique_pointer?EnergyPlus is not a traditional scientific program, but it does have the same simple memory management pattern as scientific programs do. You allocate everything at the beginning, then you compute, then you deallocate. There's no need for reference counting and garbage collection in that model. Just add delete loops to clear_state. To me, std::unique_pointer is just one more box in C++-library bingo.

Also, I'm out on EPVector. I no longer see the point and in fact I am convinced that a container that can be accessed as both a 0-based and 1-based array is only a source of bugs. I could change to std::vector and I have done that in some modules that are more self-contained than SPM (e.g., OutputProcessor). It will probably take another day or so to dig up the corner cases that arise. Is that worth it for this PR?

"But performance" you may say. well,

https://quick-bench.com/q/9xIjZoQqSyiz-uhlVFTFpN7fsn0

Locally I get more similar results, but still, my point is that this does not hinder performance.

I will not say "but performance". I will say "C++ bingo".

Regarding EPVector, the original intent was as a shim to move toward std::vector and std::array and not as something we should keep around forever. With EPVector, we wanted something that would work well enough with Array1D (which allows 0-based and 1-based indexing and is the reason that EPVector does it) that switching wouldn't break everything.

New code should definitely use std::vector/array unless there's a compelling reason to go another direction. At the time that EPVector was introduced, there were a lot of problematic cases (slicing etc.) that are less present now. Anything that's an EPVector now should (hopefully) just need the indexing addressed in order to be switched.

I agree. I think this is why I changed my mind about it. I was pro EPVector earlier because there was just a lot of Array1D madness that we needed to move away from. But now that we've moved away from a lot of it, the 0-based/1-based duality has become somewhat of a bug rather than a feature for external references (I was bitten by it a few times during the Fans transition). FWIW I actually think that Array1D also allows mixed indexing to some degree and we should disable it.

I also agree that we need to move to 0-based indices incrementally and I was thinking of doing that here, but it took me a little while to actually work the kinks out of this module and by the time I was done I was too tired to deal with that type of change. I will probably go back and do it in a separate pass.

There used to be way too much slicing going on, that's for sure. Let's not add any new Array1D objects if we can help it. With Fortran indexing being what it is (negative indices and all that), disabling either operator() or operator[] on Array1D might be harder than just moving to EPVector and then on to std::vector.

I was thinking of disabling operator [] on it. I don't think it's used in more than a handful of places, maybe I'm wrong.

src/EnergyPlus/SetPointManager.hh

Myoldmopar · 2024-06-03T16:56:54Z

Lots of fantastic stuff here. And my god, NET -6000 lines of code is amazing.

I appreciate your commentary @jmarrec, some of which led to commits and other things which are tabled for later.

I am happy to apply Clang Format and get that cleaned, but as @mjwitte noted, this will need a transition rule. It will need to, at a minimum, alert the user about the unique name requirement, and at a maximum, actually rename things. I'm guessing the former is sufficient. @mjwitte do you know of an example of that already in place in one of the transition versions? Or should I just try it out from scratch?

amirroth · 2024-06-03T16:59:46Z

Lots of fantastic stuff here. And my god, NET -6000 lines of code is amazing.

I appreciate your commentary @jmarrec, some of which led to commits and other things which are tabled for later.

I am happy to apply Clang Format and get that cleaned, but as @mjwitte noted, this will need a transition rule. It will need to, at a minimum, alert the user about the unique name requirement, and at a maximum, actually rename things. I'm guessing the former is sufficient. @mjwitte do you know of an example of that already in place in one of the transition versions? Or should I just try it out from scratch?

I have one more quick commit based on @jmarrec comments.

There is already a duplicate name error printed.

Myoldmopar · 2024-06-03T17:04:02Z

There is already a duplicate name error printed.

You are referring to the EnergyPlus code itself failing, right? Should we also alert the user during the transition process? This is really a change of behavior from 24.1 to 24.2, so I think the idea is to let them know about the change while they are updating their input files. Of course if someone makes a brand new file in 24.2, we'd also warn them that it is invalid when the execute that file.

I'm not strongly opinionated here, but happy to chip in on a transition warning if it is warranted.

amirroth · 2024-06-03T17:09:14Z

There is already a duplicate name error printed.

You are referring to the EnergyPlus code itself failing, right? Should we also alert the user during the transition process? This is really a change of behavior from 24.1 to 24.2, so I think the idea is to let them know about the change while they are updating their input files. Of course if someone makes a brand new file in 24.2, we'd also warn them that it is invalid when the execute that file.

I'm not strongly opinionated here, but happy to chip in on a transition warning if it is warranted.

Yes, I was referring to EnergyPlus itself. I don't know about the other thing. If anything, we should probably have been alerting the user to the fact that duplicate names were not being check-for previously since that seems to be the "rule" for pretty much every other "family" of objects in EnergyPlus.

mjwitte · 2024-06-03T21:23:53Z

I am happy to apply Clang Format and get that cleaned, but as @mjwitte noted, this will need a transition rule. It will need to, at a minimum, alert the user about the unique name requirement, and at a maximum, actually rename things. I'm guessing the former is sufficient. @mjwitte do you know of an example of that already in place in one of the transition versions? Or should I just try it out from scratch?

I can't think of a similar transition. The good thing is that SPM names are never used elsewhere in the input except possibly for the two output variables for SetpointManager:WarmestTemperatureFlow.

jmarrec · 2024-06-03T23:33:36Z

@Myoldmopar I made a working implementation of detecting non unique SPM names accross all types and throwing a fatal if not in #10542
(Don't ask me why I did that, I don't know)

amirroth · 2024-06-04T00:08:03Z

@Myoldmopar I made a working implementation of detecting non unique SPM names accross all types and throwing a fatal if not in #10542
(Don't ask me why I did that, I don't know)

Because you’re a sicko, that’s why.

SPM - Fortran Transition - throw if SPM names are not unique

Myoldmopar · 2024-06-04T14:15:38Z

Alright, thanks to @jmarrec this now has the transition rules in place to enforce (warn) uniqueness during transition workflows. I just applied Clang Format, so CI should be clean. There has been some good conversation here about the overall architecture and philosophy, and that can continue for a bit while we wait on things to get confirmed.

amirroth · 2024-06-04T14:44:33Z

Alright, thanks to @jmarrec this now has the transition rules in place to enforce (warn) uniqueness during transition workflows. I just applied Clang Format, so CI should be clean. There has been some good conversation here about the overall architecture and philosophy, and that can continue for a bit while we wait on things to get confirmed.

We're not discussing architecture per se, we're discussing use of various C++ features and idioms. Specifically:

Use of EXPECT_TRUE(compare_enums(e1, e2)) vs. EXPECT_EQ((int)e1, (int)e2). I reverted my changes on this one and agree that we should preserve enum-typed comparison here although maybe with a cleaner template that doesn't have the compare_enums(e1, e2, false) gotcha.
Benefit of override keyword. Added.
Use of unchecked (int) "upcasts" vs. checked static_cast<int> "upcasts" in use of enums as std::array indices. Not backing down on this one at this point.
Use of raw pointers and delete loops in clear_state vs. unique_ptr wrappers. Not backing down on this one either right now. I've made my thoughts clear about this feature. I understand its utility in general but EnergyPlus specifically doesn't need it. At best, it's an emotional support idiom. At worst, it's C++-library bingo.
Continued use of Array1D vs. transition to std::vector when these refactoring changes are made. This is a gray one. I've been transitioning to vector when I feel like the module is sufficiently self contained and there aren't too many external indices (e.g., OutputProcessor, although @JasonGlazer found one last week!). If I think that I may have to chase down a bunch of if (xNum > 0) tests then I leave it as Array1D (e.g., Fans). The situation with SPM fell in the middle and towards the end I just got too tired to tackle this part.

jmarrec · 2024-06-10T13:06:16Z

I merged develop in and resolved the conflicts due to the merge of

#10545 - Provide facilities for testing enums in a safe way (EXPECT_ENUM_EQ / EXPECT_ENUM_NE) #10550

Myoldmopar · 2024-06-10T14:48:04Z

OK, I think this one is ready to go. I'll let CI wrap up since it is almost done, but it looks clean, just BND diffs. Speak up quick if there is anything else here!

Myoldmopar · 2024-06-10T19:03:19Z

OK fine, merging! Thanks @amirroth. And reviewers.

amirroth added 22 commits May 20, 2024 18:15

Initial refactor

15f4ab8

Merge remote-tracking branch 'origin/develop' into SPM

1232bca

Fix some CI errors

67c60e9

Merge remote-tracking branch 'origin/develop' into SPM

56a71ef

Fix a few more CI issues

2214793

Yet more CI fixes

28b93ad

Whittling away

5a27688

Whittle whittle

1278001

Preserve node creation order (and my sanity)

d4a9e04

Fix MZMinHum and IdealCondenserEnteringTemp managers

820e201

Fix SZCooling manager

69814d2

Merge remote-tracking branch 'origin/develop' into SPM

ca2b2ec

Fix SZReheat and FollowSystemNode inputs

13d368f

Fix FollowSysNode and SZMaxHum managers

dd777b4

Fix ReturnWaterTemp manager

7c178b1

Fix MZHeating and MZCooling managers

565f943

Preserve IDF object order within object type

af3bcb1

Merge remote-tracking branch 'origin/develop' into SPM

27ce476

Quick cleanup, fix epJson input and CondenserEnteringTemp manager

598e217

Forgot to commit the unit test changes

f65aa7f

Fix warnings

fb62e5b

Merge remote-tracking branch 'origin/develop' into SPM

3c16819

amirroth requested review from dareumnam and Myoldmopar May 31, 2024 02:16

amirroth added DoNotPublish Includes changes that shouldn't be reported in the changelog Refactoring Includes code changes that don't change the functionality of the program, just perform refactoring labels May 31, 2024

amirroth added this to the EnergyPlus 24.2 milestone May 31, 2024

amirroth assigned amirroth and unassigned amirroth May 31, 2024

amirroth commented May 31, 2024

View reviewed changes

mjwitte added IDDChange Code changes impact the IDD file (cannot be merged after IO freeze) and removed DoNotPublish Includes changes that shouldn't be reported in the changelog labels May 31, 2024

amirroth added 2 commits May 31, 2024 16:16

Revert compare_enum changes

42b7e48

Merge remote-tracking branch 'origin/develop' into SPM

0426356

rraustad reviewed May 31, 2024

View reviewed changes

jmarrec reviewed Jun 3, 2024

View reviewed changes

SetPointManager.hh

f14032f

amirroth added 2 commits June 3, 2024 14:58

Merge remote-tracking branch 'origin/develop' into SPM

1fef327

Merge remote-tracking branch 'origin/develop' into SPM

b42460d

SPM - Fortran Transition - throw if SPM names are not unique

298d280

jmarrec mentioned this pull request Jun 3, 2024

SPM - Fortran Transition - throw if SPM names are not unique #10542

Merged

20 tasks

Myoldmopar added 2 commits June 4, 2024 09:06

Merge pull request #10542 from NREL/SPM-transition

866845f

SPM - Fortran Transition - throw if SPM names are not unique

Apply Clang Format

368ca38

Myoldmopar self-assigned this Jun 5, 2024

jmarrec mentioned this pull request Jun 5, 2024

Provide facilities for testing enums in a safe way (EXPECT_ENUM_EQ / EXPECT_ENUM_NE) #10545

Closed

3 tasks

Merge branch 'develop' into SPM and resolve conflicts

07b0600

Myoldmopar merged commit 016f0c8 into develop Jun 10, 2024
15 checks passed

Myoldmopar deleted the SPM branch June 10, 2024 19:03

		@@ -62,6 +62,16 @@ struct EnergyPlusData;

		namespace DataEnvironment {

		enum class GroundTempType

		// SUBROUTINE LOCAL VARIABLE DECLARATIONS:
		auto &CondWaterSetPoint = state.dataSetPointManager->CondWaterSetPoint;

		@@ -66,61 +68,40 @@ struct EnergyPlusData;

		namespace SetPointManager {

		enum class CtrlNodeType

	void calculate(EnergyPlusData &state);
	void calculate(EnergyPlusData &state) override;

SetPointManager Refactor #10537

SetPointManager Refactor #10537

Conversation

amirroth commented May 31, 2024

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

jmarrec May 31, 2024 • edited Loading

Choose a reason for hiding this comment

jmarrec May 31, 2024 • edited Loading

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

jmarrec 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

jmarrec Jun 3, 2024 • edited Loading

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 Jun 3, 2024

amirroth commented Jun 3, 2024

Myoldmopar commented Jun 3, 2024

amirroth commented Jun 3, 2024

mjwitte commented Jun 3, 2024

jmarrec commented Jun 3, 2024

amirroth commented Jun 4, 2024

Myoldmopar commented Jun 4, 2024

amirroth commented Jun 4, 2024

jmarrec commented Jun 10, 2024

Myoldmopar commented Jun 10, 2024

Myoldmopar commented Jun 10, 2024

jmarrec May 31, 2024 •

edited

Loading

jmarrec May 31, 2024 •

edited

Loading

jmarrec Jun 3, 2024 •

edited

Loading