Fix #10817 - Avoid crash in FluidCooler/EvaporativeFluidCooler when Water flow rate autosized and no Sizing:Plant #10855

jmarrec · 2024-12-13T11:38:30Z

Pull request overview

Fixes Crash in Fluid Cooler Code #10817

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.

…ng it to an empty (invalid / default initialized) FluidCooler!

…t loop This makes no difference in test results, but it was bothering me that they were on the demand side as it's not logical

…ater flow rate autosized and no Sizing:Plant

jmarrec

I have noticed more problems, but they will likely result in diffs in results so I didn't had them right now, as I'd like CI to report green.

jmarrec · 2024-12-13T11:39:33Z

tst/EnergyPlus/unit/FluidCoolers.unit.cc

@@ -356,10 +356,10 @@ TEST_F(EnergyPlusFixture, SizeFunctionTestWhenPlantSizingIndexIsZero)
    auto &thisFluidCooler = state->dataFluidCoolers->SimpleFluidCooler(FluidCoolerNum);

    state->dataPlnt->PlantLoop.allocate(FluidCoolerNum);
-    state->dataFluidCoolers->SimpleFluidCooler.allocate(FluidCoolerNum);


Fix an existing test. When I tried to reuse it, I realized that the allocate after GetInput was resetting it to an empty (invalid / default initialized) FluidCooler

jmarrec · 2024-12-13T11:39:41Z

tst/EnergyPlus/unit/FluidCoolers.unit.cc

    state->dataFluidCoolers->SimpleFluidCooler(FluidCoolerNum).plantLoc.loopNum = 1;
    state->dataPlnt->PlantLoop(FluidCoolerNum).PlantSizNum = 0;

+    EXPECT_EQ("DRY COOLER", thisFluidCooler.Name);


This would fail, before

jmarrec · 2024-12-13T11:40:42Z

tst/EnergyPlus/unit/FluidCoolers.unit.cc

@@ -389,10 +389,10 @@ TEST_F(EnergyPlusFixture, ExerciseSingleSpeedFluidCooler)

    FluidCoolerspecs *ptr = FluidCoolerspecs::factory(*state, DataPlant::PlantEquipmentType::FluidCooler_SingleSpd, "DRY COOLER");

-    PlantLocation pl{1, EnergyPlus::DataPlant::LoopSideLocation::Demand, 1, 1};
+    PlantLocation pl{1, EnergyPlus::DataPlant::LoopSideLocation::Supply, 1, 1};


Unrelated, but place the fluid coolers on the supply side of the plant loop.

This makes no difference in test results, but it was bothering me that all of the FluidCooler.unit.cc / EvaporativeFluidCooler.unit.cc had the fluid coolers on the demand side

jmarrec · 2024-12-13T11:40:57Z

tst/EnergyPlus/unit/FluidCoolers.unit.cc

+TEST_F(EnergyPlusFixture, FluidCooler_SizeWhenPlantSizingIndexIsZeroAndAutosized)
+{
+    // Test for #10817
+    std::string const idf_objects = delimited_string({
+        "   FluidCooler:SingleSpeed,",
+        "     Dry Cooler,              !- Name",
+        "     Dry Cooler Inlet Node,   !- Water Inlet Node Name",
+        "     Dry Cooler Outlet Node,  !- Water Outlet Node Name",
+        "     NominalCapacity,         !- Performance Input Method",
+        "     Autosize,                !- Design Air Flow Rate U-factor Times Area Value {W/K}",
+        "     58601,                   !- Nominal Capacity {W}",
+        "     50,                      !- Design Entering Water Temperature {C}",
+        "     35,                      !- Design Entering Air Temperature {C}",
+        "     25,                      !- Design Entering Air Wetbulb Temperature {C}",
+        "     Autosize,                !- Design Water Flow Rate {m3/s}",
+        "     Autosize,                !- Design Air Flow Rate {m3/s}",
+        "     Autosize;                !- Design Air Flow Rate Fan Power {W}",
+    });
+
+    ASSERT_TRUE(process_idf(idf_objects));
+
+    GetFluidCoolerInput(*state);
+    int FluidCoolerNum(1);
+
+    state->dataPlnt->PlantLoop.allocate(FluidCoolerNum);
+    state->dataPlnt->PlantLoop(FluidCoolerNum).PlantSizNum = 0;
+
+    auto &thisFluidCooler = state->dataFluidCoolers->SimpleFluidCooler(FluidCoolerNum);
+    thisFluidCooler.plantLoc.loopNum = 1;
+
+    // Necessary to trigger the crash from #
+    state->dataPlnt->PlantFirstSizesOkayToFinalize = false;
+
+    EXPECT_TRUE(thisFluidCooler.DesignWaterFlowRateWasAutoSized);
+    EXPECT_TRUE(thisFluidCooler.HighSpeedFanPowerWasAutoSized);
+    EXPECT_TRUE(thisFluidCooler.HighSpeedAirFlowRateWasAutoSized);
+    EXPECT_TRUE(thisFluidCooler.HighSpeedFluidCoolerUAWasAutoSized);
+
+    thisFluidCooler.size(*state);
+}


New test, crashes before fix

jmarrec · 2024-12-13T11:41:24Z

tst/EnergyPlus/unit/EvaporativeFluidCoolers.unit.cc

+TEST_F(EnergyPlusFixture, EvapFluidCooler_SizeWhenPlantSizingIndexIsZeroAndAutosized)
+{
+    // Test for #10817
+    std::string const idf_objects = delimited_string({
+        "EvaporativeFluidcooler:SingleSpeed,",
+        "  Big EvaporativeFluidCooler,  !- Name",
+        "  Condenser EvaporativeFluidcooler Inlet Node,  !- Water Inlet Node Name",
+        "  Condenser EvaporativeFluidcooler Outlet Node,  !- Water Outlet Node Name",
+        "  Autosize,                !- Design Air Flow Rate {m3/s}",
+        "  Autosize,                !- Design Air Flow Rate Fan Power {W}",
+        "  0.002208,                !- Design Spray Water Flow Rate {m3/s}",
+        "  UFactorTimesAreaAndDesignWaterFlowRate,  !- Performance Input Method",
+        "  ,                        !- Outdoor Air Inlet Node Name",
+        "  ,                        !- Heat Rejection Capacity and Nominal Capacity Sizing Ratio",
+        "  ,                        !- Standard Design Capacity {W}",
+        "  Autosize,                !- Design Air Flow Rate U-factor Times Area Value {W/K}",
+        "  Autosize,                !- Design Water Flow Rate {m3/s}",
+        "  ,                        !- User Specified Design Capacity {W}",
+        "  46.11,                   !- Design Entering Water Temperature {C}",
+        "  35,                      !- Design Entering Air Temperature {C}",
+        "  25.6;                    !- Design Entering Air Wet-bulb Temperature {C}",
+    });
+
+    ASSERT_TRUE(process_idf(idf_objects));
+
+    EvapFluidCoolerSpecs *ptr =
+        EvapFluidCoolerSpecs::factory(*state, DataPlant::PlantEquipmentType::EvapFluidCooler_SingleSpd, "BIG EVAPORATIVEFLUIDCOOLER");
+
+    state->dataPlnt->PlantLoop.allocate(1);
+    state->dataPlnt->PlantLoop(1).PlantSizNum = 0;
+    ptr->plantLoc.loopNum = 1;
+
+    // Necessary to trigger the crash from #
+    state->dataPlnt->PlantFirstSizesOkayToFinalize = false;
+
+    EXPECT_TRUE(ptr->DesignWaterFlowRateWasAutoSized);
+    EXPECT_TRUE(ptr->HighSpeedAirFlowRateWasAutoSized);
+    EXPECT_TRUE(ptr->HighSpeedFanPowerWasAutoSized);
+    EXPECT_TRUE(ptr->HighSpeedEvapFluidCoolerUAWasAutoSized);
+
+    ptr->SizeEvapFluidCooler(*state);
+}


Realized the same was happening for EvaporativeFluidCooler:XX objects, so added a test there too

jmarrec · 2024-12-13T11:42:27Z

src/EnergyPlus/EvaporativeFluidCoolers.cc

            if (PltSizCondNum > 0) {
+
+                // Check when the user specified Condenser/Evaporative Fluid Cooler water design setpoint
+                // temperature is less than design inlet air wet bulb temperature
+                Real64 DesignEnteringAirWetBulb = 0;


Fix is to basically ensure the check is done INSIDE the if(PltSizCondNum > 0)

If PltSizCondNum is not > 0, it'll throw anyways

jmarrec · 2024-12-13T11:42:57Z

src/EnergyPlus/EvaporativeFluidCoolers.cc

@@ -1380,28 +1403,6 @@ namespace EvaporativeFluidCoolers {
                    ShowFatalError(state, "Autosizing of evaporative fluid cooler condenser flow rate requires a loop Sizing:Plant object.");


Throw when PltSizCondNum is not > 0 is here.

jmarrec · 2024-12-13T11:43:18Z

src/EnergyPlus/FluidCoolers.cc

+    // This is to trap when the user specified Condenser/Fluid Cooler water design setpoint temperature is less than design inlet air dry bulb
+    // temperature
+    auto ensureSizingPlantExitTempIsNotLessThanDesignEnteringAirTemp = [this, &state, PltSizCondNum]() {
+        if (state.dataSize->PlantSizData(PltSizCondNum).ExitTemp <= this->DesignEnteringAirTemp && state.dataPlnt->PlantFirstSizesOkayToFinalize) {
+            ShowSevereError(state, format("Error when autosizing the UA value for fluid cooler = {}.", this->Name));
+            ShowContinueError(state,
+                              format("Design Loop Exit Temperature ({:.2R} C) must be greater than design entering air dry-bulb temperature "
+                                         "({:.2R} C) when autosizing the fluid cooler UA.",
+                                         state.dataSize->PlantSizData(PltSizCondNum).ExitTemp,
+                                         this->DesignEnteringAirTemp));
+                ShowContinueError(state,
+                                  "It is recommended that the Design Loop Exit Temperature = design inlet air dry-bulb temp plus the Fluid Cooler "
+                                  "design approach temperature (e.g., 4 C).");
+                ShowContinueError(state,
+                                  "If using HVACTemplate:Plant:ChilledWaterLoop, then check that input field Condenser Water Design Setpoint must be "
+                              "> design inlet air dry-bulb temp if autosizing the Fluid Cooler.");
+            ShowFatalError(state, "Review and revise design input values as appropriate.");
+        }
+    };


Fluid cooler had the same block 4 times, so I reuse a lambda instead

jmarrec · 2024-12-13T11:43:50Z

src/EnergyPlus/FluidCoolers.cc

+    // This is to trap when the user specified Condenser/Fluid Cooler water design setpoint temperature is less than design inlet air dry bulb
+    // temperature
+    auto ensureSizingPlantExitTempIsNotLessThanDesignEnteringAirTemp = [this, &state, PltSizCondNum]() {
+        if (state.dataSize->PlantSizData(PltSizCondNum).ExitTemp <= this->DesignEnteringAirTemp && state.dataPlnt->PlantFirstSizesOkayToFinalize) {


I don't think it's necessary to delay until PlantFirstSizesOkayToFinalize is true, but I didn't want to change the existing behavior

``` rg Fluidcool -l | xargs sed -i 's/Fluidcooler/FluidCooler/g' ```

…low Rate (m3/s)

…nd used to report Range & Approach)

github-actions · 2024-12-13T13:27:12Z

⚠️ Regressions detected on macos-14 for commit `f5ebb77`

Regression Summary

Table Big Diffs: 6

jmarrec · 2024-12-16T08:57:28Z

Regressions are all expected. The ASHRAE901_OfficeLarge_STD2019_Denver variations have an autosized Design Water Flow Rate, so the Design Leaving Water Temperature was already being correctly calculated, so no diff here (and approach/range), only the m3/s sig digits:

Before:

After:

FluidCooler.idf

Design Water Flow Rate is harcoded to 0.001388, so the Design Leaving Water Temperature was zero before and approach & range were wrong

before:

After:

FluidCoolerTwoSpeed

Same here as the ASHRAE901, the Design Water Flow Rate is autosized so the Design Leaving Water Temperature was correctly calcualted

before:

After:

nrel-bot-2 · 2024-12-23T00:19:54Z

@jmarrec it has been 7 days since this pull request was last updated.

nrel-bot-2 · 2025-01-02T00:25:27Z

@jmarrec it has been 10 days since this pull request was last updated.

nrel-bot-2 · 2025-01-10T00:28:49Z

@jmarrec it has been 8 days since this pull request was last updated.

jmarrec added 7 commits December 13, 2024 10:25

Highlight an issue in an existing test

316d5ae

Fix problem in existing test: the allocate after GetInput was resetti…

a003e07

…ng it to an empty (invalid / default initialized) FluidCooler!

Rename test for clarity

61c4c5d

Unrelated, but place the fluid coolers on the SUPPLY side of the plan…

93c4e0b

…t loop This makes no difference in test results, but it was bothering me that they were on the demand side as it's not logical

Add tests for #10817

dee7e5b

Add test: EvaporativeFluidCooler also had the same issue as #10817

6932358

Fix #10817 - Avoid crash in FluidCooler/EvaporativeFluidCooler when W…

9e8586b

…ater flow rate autosized and no Sizing:Plant

jmarrec added Defect Includes code to repair a defect in EnergyPlus NotIDDChange Code does not impact IDD (can be merged after IO freeze) labels Dec 13, 2024

jmarrec self-assigned this Dec 13, 2024

jmarrec commented Dec 13, 2024

View reviewed changes

jmarrec added 4 commits December 13, 2024 13:38

Dry up code via a lambda

b02725a

Rename Fluidcooler -> FluidCooler in examples files for easier grepping

23fa180

``` rg Fluidcool -l | xargs sed -i 's/Fluidcooler/FluidCooler/g' ```

Increase num sig digits from default=2 to 6 to report the **Water** F…

a179c0d

…low Rate (m3/s)

Calculate DesignLeavingWaterTemp in all cases, since it's reported (a…

4aaa7c0

…nd used to report Range & Approach)

jmarrec force-pushed the 10817_CrashFluidCooler branch from 7284c44 to 4aaa7c0 Compare December 13, 2024 12:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix #10817 - Avoid crash in FluidCooler/EvaporativeFluidCooler when Water flow rate autosized and no Sizing:Plant #10855

Fix #10817 - Avoid crash in FluidCooler/EvaporativeFluidCooler when Water flow rate autosized and no Sizing:Plant #10855

jmarrec commented Dec 13, 2024

jmarrec left a comment

jmarrec Dec 13, 2024

jmarrec Dec 13, 2024

jmarrec Dec 13, 2024

jmarrec Dec 13, 2024

jmarrec Dec 13, 2024

jmarrec Dec 13, 2024

jmarrec Dec 13, 2024

jmarrec Dec 13, 2024

jmarrec Dec 13, 2024

github-actions bot commented Dec 13, 2024

jmarrec commented Dec 16, 2024 •

edited

Loading

nrel-bot-2 commented Dec 23, 2024

nrel-bot-2 commented Jan 2, 2025

nrel-bot-2 commented Jan 10, 2025

		@@ -1380,28 +1403,6 @@ namespace EvaporativeFluidCoolers {
		ShowFatalError(state, "Autosizing of evaporative fluid cooler condenser flow rate requires a loop Sizing:Plant object.");

Fix #10817 - Avoid crash in FluidCooler/EvaporativeFluidCooler when Water flow rate autosized and no Sizing:Plant #10855

Are you sure you want to change the base?

Fix #10817 - Avoid crash in FluidCooler/EvaporativeFluidCooler when Water flow rate autosized and no Sizing:Plant #10855

Conversation

jmarrec commented Dec 13, 2024

Pull request overview

Pull Request Author

Reviewer

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

Choose a reason for hiding this comment

github-actions bot commented Dec 13, 2024

⚠️ Regressions detected on macos-14 for commit f5ebb77

jmarrec commented Dec 16, 2024 • edited Loading

nrel-bot-2 commented Dec 23, 2024

nrel-bot-2 commented Jan 2, 2025

nrel-bot-2 commented Jan 10, 2025

⚠️ Regressions detected on macos-14 for commit `f5ebb77`

jmarrec commented Dec 16, 2024 •

edited

Loading