Correction of Condensers Not Operating Based on Operation Scheme #10653

RKStrand · 2024-08-09T20:43:09Z

Pull request overview

Fixes PlantEquipmentOperation:OutdoorDryBulb does not work with some components #10634
It was noted (in test suite IDFs) that condensers were operating even though their operation scheme should have kept them from functioning. For example, in one file, when the outdoor dry-bulb temperature rose above 27C, only one condenser should have operated rather than both. While this may not be realistic, EnergyPlus should have been able to do this. Unfortunately, both cooling towers always ran, even when the temperature rose above the 27C value. This was because three of the four tower models never looked at the load that was assigned to it based on the operation scheme. This included the single speed, two speed, and variable speed cooling tower. The Merkel tower model did in fact use the assigned load. To fix this issue for the other cooling towers, a modified version of the algorithm from the Merkel model was implemented in the other models. Now, in the test file, the only cooling tower that operates when ODB is above 27C is the one that is supposed to and the other turns off. This logic was added as a new subroutine and a unit test was added to verify that the code is doing what it is supposed to do. Differences are expected in numerous files because before the operation schemes weren't being taken into account.

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.

This commit includes the fix to the problem for both condensers operating even when the operation scheme said that one of them should not be operating. It fixes this for all of the cooling tower models that were not set up to look at this. Results now should that the towers are operating when they are supposed to.

Something about the logical flag wasn't what the custom check wanted. This is an attempt to fix that.

Unit test was added to exercise the new subroutine that is being used to allow condensers to factor in condenser loop operation schemes.

RKStrand · 2024-08-09T20:44:07Z

Here is a graph that can be compared to the graph that was posted in the original defect. It shows clearly that the second cooling tower shuts down when ODB goes above 27C.

Hard to see perhaps in the actual graph, but in the data as soon as ODB is above 27C, the BigTower2 drops to zero heat transfer rate.

rraustad · 2024-08-09T21:01:49Z

So what was happening before? The condenser plant was exceeding the set point temperature? Or was the condenser plant set point met because the tower was not turning off based on OAT? From the results this looks correct. Operating a tower on DB temp does not seem practical. @EnergyArchmage ?

rraustad · 2024-08-09T21:07:30Z

In the graph is does look like tower 2 is still on when the OAT rises above 27 C. Tower 2 operation is definitely different than shown in the issue description.

RKStrand · 2024-08-09T21:23:14Z

@rraustad Yes, I agree--the graph makes it look like it is still operating when the ODB is above 27C. In hour 10, the ODB is 26.975C so both the first and second tower have a positive heat transfer rate. That's hard to see in the graph though because it is so close to 27C. In hour 11 when the ODB is well above 27C, the second tower is off and the first tower takes all of the load. I'll add the test file results here so you can see for yourself, but it is really hard to see on the graph.

eplusoutCondFix.csv

RKStrand · 2024-08-09T21:24:58Z

@rraustad to see what was happening before, take a quick look at the defect (#10634) where a graph about what was happening before is shown.

rraustad · 2024-08-09T21:35:56Z

Should the water flow through the tower also turn off? Or are these towers somehow in series.

RKStrand · 2024-08-09T22:00:00Z

Should the water flow through the tower also turn off? Or are these towers somehow in series.

Well, they are not in series. There's no delta T across the second tower, but there's no reason why this is going through the second tower and not the bypass branch. The file in question here is out of the test suite: CoolingTowerDryBulbRangeOp.idf

Corrected an issue with the fix where the mass flow rate when the load was zero was still positive in the defect idf when it should be zero. This is now corrected.

RKStrand · 2024-08-09T22:18:10Z

FYI: The non-zero flow rate when the load was zero problem has been corrected in the latest commit that was just made.

rraustad · 2024-08-09T23:07:01Z

src/EnergyPlus/CondenserLoopTowers.cc

+        if (std::abs(MyLoad) <= HVAC::SmallLoad) {
+            // tower doesn't need to do anything
+            this->OutletWaterTemp = state.dataLoopNodes->Node(this->WaterInletNodeNum).Temp;
+            this->WaterMassFlowRate = 0.0;


I think when there is a local change to plant flow rates, that change should be confirmed using this call. The point is, after the water flow is set to 0 here, there should be a call to the plant component flow control function SetComponentFlowRate. It is possible that the plant flow resolver would want flow through this branch (i.e., maybe there is no bypass and the pump is constant volume).

PlantUtilities::SetComponentFlowRate(state, this->WaterMassFlowRate, this->WaterInletNodeNum, this->WaterOutletNodeNum, this->plantLoc);

Should the order of these conditionals be switched? line 6461 and line 6469. Why do this twice? This begs an else if.

rraustad · 2024-08-09T23:49:08Z

tst/EnergyPlus/unit/CondenserLoopTowers.unit.cc

@@ -927,8 +928,8 @@ TEST_F(EnergyPlusFixture, CondenserLoopTowers_SingleSpeedSizing)
        outletNodeIndex = std::distance(state->dataLoopNodes->NodeID.begin(), outletNode);
    }
    // TODO: FIXME: This is failing. Actual is -10.409381032746095, expected is 30.


Is this comment still needed?

EnergyArchmage · 2024-08-10T16:08:10Z

It is definitely a good idea to update these old models. In addition to .MyLoad ( or CurLoad here), the operation schemes also set a bool .ON (usually called RunFlag) for each component. In general nowadays, the plant simulate calls should include both MyLoad and RunFlag.

To me, std::abs(MyLoad) <= HVAC::SmallLoad is a bit of hack compared to just examining RunFlag. If a machine is running it should have both RunFlag true and significant load in the right direction, ( MyLoad < SmallLoad ) where sign convention matters and cooling is negative. It could be possible that a positive MyLoad gets passed in and you don't want it to run in that case.

And yes, as @rraustad suggests, if the machine is not running it definitely needs to make a zero flow request via SetComponentFlowRate(). It may still have flow if in series with something else but that will at least make request for zero flow.

I would plot these variables for the towers -- to see how plant flow rates are responding and make sure the tower fan is off.

Output:Variable,*,Plant Component Distributed Demand Rate,detailed; !- HVAC Average [W]    
Output:Variable,*,System Node Requested Mass Flow Rate, detailed; !- HVAC Average [kg/s]
Output:Variable,*,System Node Mass Flow Rate,detailed; !- HVAC Average [kg/s]
Output:Variable,*,Cooling Tower Fan Electricity Rate,detailed; !- HVAC Average [W]

Various review comments were received. These were all addressed in this commit.

RKStrand · 2024-08-13T19:31:19Z

Thanks, @rraustad and @EnergyArchmage, for your comments. I believe that I have addressed them all with the most recent commit (the one right before I merged in develop). Specifically:

PlantUtilities::SetComponentFlowRate is now called after the flow is set to zero
The dual if statement has been converted to if-else if and the order of the checks switched
Strange "TODO" comment removed in a pre-existing unit test
The if check about load has been changed to avoid loads of the wrong sign and also check the run flag
The output variables were added as suggested--everything looks like what one would expect (when the second cooling tower isn't running, there is no flow and no fan energy); will post graph/data here shortly

I think that was everything to date. Did I miss anything or is there anything else that needs to be done here?

RKStrand · 2024-08-13T20:08:04Z

EnergyArchmage · 2024-08-14T13:21:12Z

src/EnergyPlus/CondenserLoopTowers.cc

@@ -6355,6 +6344,7 @@ namespace CondenserLoopTowers {
        // This subroutine is for passing results to the outlet water node.

        // set node information
+        PlantUtilities::SafeCopyPlantNode(state, this->WaterInletNodeNum, this->WaterOutletNodeNum);


@RKStrand hope you don't mind me jumping in here. When components are in series, this is needed to pass data down the branch to the next component.

Not at all, @EnergyArchmage! Thanks for making that change.

EnergyArchmage · 2024-08-14T13:21:58Z

This looks good to me!

rraustad · 2024-08-14T13:35:52Z

src/EnergyPlus/CondenserLoopTowers.cc


-        if (std::abs(MyLoad) <= HVAC::SmallLoad) {
+        if ((std::abs(MyLoad) <= HVAC::SmallLoad) || !RunFlag) {


I am wondering if just !RunFlag would meet the intent. I would think RunFlag would be false if (std::abs(MyLoad) <= HVAC::SmallLoad) but then I am not sure.

I wasn't sure either and given that the Merkel model already used MyLoad, I felt like having both would be "safer". My guess is that both of these flags will probably trip this IF at the same time, but I didn't see anything wrong with doing it this way which seemed like it would catch all conditions.

rraustad · 2024-08-14T13:39:27Z

src/EnergyPlus/CondenserLoopTowers.cc

@@ -5529,6 +5513,11 @@ namespace CondenserLoopTowers {
            CalcBasinHeaterPower(
                state, this->BasinHeaterPowerFTempDiff, this->BasinHeaterSchedulePtr, this->BasinHeaterSetPointTemp, this->BasinHeaterPower);
            return;
+        } else if (this->WaterMassFlowRate <= DataBranchAirLoopPlant::MassFlowTolerance || (MyLoad > HVAC::SmallLoad)) {


This is interesting. If this conditional is checked then the tower is running. I can't be sure if the water mass flow rate could really be this small if the tower is running but then you have the plant resolver making that determination. Also not sure of the load check.

I noticed that there was a basin heater check in checkMassFlowAndLoad similar to this. Does that make this new code unnecessary?

I left this alone because it was the one that was identified as working properly before the defect was logged. I'll admit, I did not see why it did this check (seemed confusing). However, I left it alone in the Merkel model because I didn't see a compelling reason to fix something that wasn't broken. You'll notice that the code here is slightly different than the code in checkMassFlowAndLoad--intentionally so because I trust the logic in checkMassFlowAndLoad and it makes more logical sense. So, I don't think the new code is unnecessary. It was somewhat based on what was done here in the Merkel model, with changes to the logic (by me and then improved with your suggestions).

rraustad · 2024-08-14T13:41:43Z

src/EnergyPlus/CondenserLoopTowers.cc

+
+        bool returnFlagSet = false;
+        this->checkMassFlowAndLoad(state, MyLoad, RunFlag, returnFlagSet);
+        if (returnFlagSet) return;


If you made this function a bool it would just be if (this->checkMassFlowAndLoad(state, MyLoad, RunFlag) return;

I think I was concerned that a function would then be setting other parameters and calling other routines, not just getting a bool result. I felt like that was a violation of some standard, but I'll admit that I don't know whether that is an E+ standard or something that I simply picked up over the years that maybe isn't completely true. @Myoldmopar, any thoughts on this?

Myoldmopar · 2024-08-14T19:43:51Z

I pulled develop and this is still happy. I'm going to mark this ready for merge, but hold off from actually merging while I await clean CI results on other branches.

Myoldmopar · 2024-08-16T14:59:13Z

Pulled develop in and everything is happy, merging this. Thanks @RKStrand

RKStrand added 3 commits August 9, 2024 13:05

10634 Address Custom Check Error

8f7c5de

Something about the logical flag wasn't what the custom check wanted. This is an attempt to fix that.

10634 Addition of Unit Test

712f23f

Unit test was added to exercise the new subroutine that is being used to allow condensers to factor in condenser loop operation schemes.

RKStrand added the Defect Includes code to repair a defect in EnergyPlus label Aug 9, 2024

RKStrand added this to the EnergyPlus 24.2 milestone Aug 9, 2024

RKStrand requested review from Myoldmopar, mjwitte and rraustad August 9, 2024 20:43

RKStrand assigned Myoldmopar and RKStrand Aug 9, 2024

10634 Mass Flow to Zero

eb0559c

Corrected an issue with the fix where the mass flow rate when the load was zero was still positive in the defect idf when it should be zero. This is now corrected.

rraustad reviewed Aug 9, 2024

View reviewed changes

RKStrand added 2 commits August 13, 2024 14:24

10634 Addresses Review Comments

92134b5

Various review comments were received. These were all addressed in this commit.

Merge branch 'develop' into 10634CondenserNotFollowingPlanOpScheme

c8531d1

RKStrand requested a review from rraustad August 13, 2024 19:31

add SafeCopyPlantNode() to cooling towers

4d61594

EnergyArchmage reviewed Aug 14, 2024

View reviewed changes

rraustad reviewed Aug 14, 2024

View reviewed changes

Myoldmopar merged commit 9a7218c into develop Aug 16, 2024
15 checks passed

Myoldmopar deleted the 10634CondenserNotFollowingPlanOpScheme branch August 16, 2024 14:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Correction of Condensers Not Operating Based on Operation Scheme #10653

Correction of Condensers Not Operating Based on Operation Scheme #10653

RKStrand commented Aug 9, 2024 •

edited

Loading

RKStrand commented Aug 9, 2024 •

edited

Loading

rraustad commented Aug 9, 2024

rraustad commented Aug 9, 2024

RKStrand commented Aug 9, 2024

RKStrand commented Aug 9, 2024

rraustad commented Aug 9, 2024 •

edited

Loading

RKStrand commented Aug 9, 2024

RKStrand commented Aug 9, 2024

rraustad Aug 9, 2024

rraustad Aug 9, 2024 •

edited

Loading

rraustad Aug 9, 2024

EnergyArchmage commented Aug 10, 2024

RKStrand commented Aug 13, 2024

RKStrand commented Aug 13, 2024

EnergyArchmage Aug 14, 2024

RKStrand Aug 14, 2024

EnergyArchmage commented Aug 14, 2024

rraustad Aug 14, 2024

RKStrand Aug 14, 2024

rraustad Aug 14, 2024

rraustad Aug 14, 2024

RKStrand Aug 14, 2024

rraustad Aug 14, 2024 •

edited

Loading

RKStrand Aug 14, 2024

Myoldmopar commented Aug 14, 2024

Myoldmopar commented Aug 16, 2024


		if (std::abs(MyLoad) <= HVAC::SmallLoad) {
		if ((std::abs(MyLoad) <= HVAC::SmallLoad) \|\| !RunFlag) {

Correction of Condensers Not Operating Based on Operation Scheme #10653

Correction of Condensers Not Operating Based on Operation Scheme #10653

Conversation

RKStrand commented Aug 9, 2024 • edited Loading

Pull request overview

Pull Request Author

Reviewer

RKStrand commented Aug 9, 2024 • edited Loading

rraustad commented Aug 9, 2024

rraustad commented Aug 9, 2024

RKStrand commented Aug 9, 2024

RKStrand commented Aug 9, 2024

rraustad commented Aug 9, 2024 • edited Loading

RKStrand commented Aug 9, 2024

RKStrand commented Aug 9, 2024

Choose a reason for hiding this comment

rraustad Aug 9, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

EnergyArchmage commented Aug 10, 2024

RKStrand commented Aug 13, 2024

RKStrand commented Aug 13, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

EnergyArchmage commented Aug 14, 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

rraustad Aug 14, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Myoldmopar commented Aug 14, 2024

Myoldmopar commented Aug 16, 2024

RKStrand commented Aug 9, 2024 •

edited

Loading

RKStrand commented Aug 9, 2024 •

edited

Loading

rraustad commented Aug 9, 2024 •

edited

Loading

rraustad Aug 9, 2024 •

edited

Loading

rraustad Aug 14, 2024 •

edited

Loading