Skip to content

Conversation

@jmarrec
Copy link
Contributor

@jmarrec jmarrec commented Jan 21, 2025

Pull request overview

Pull Request Author

Add to this list or remove from it as applicable. This is a simple templated set of guidelines.

  • Title of PR should be user-synopsis style (clearly understandable in a standalone changelog context)
  • Label the PR with at least one of: Defect, Refactoring, NewFeature, Performance, and/or DoNoPublish
  • Pull requests that impact EnergyPlus code must also include unit tests to cover enhancement or defect repair
  • Author should provide a "walkthrough" of relevant code changes using a GitHub code review comment process
  • If any diffs are expected, author must demonstrate they are justified using plots and descriptions
  • If changes fix a defect, the fix should be demonstrated in plots and descriptions
  • If any defect files are updated to a more recent version, upload new versions here or on DevSupport
  • If IDD requires transition, transition source, rules, ExpandObjects, and IDFs must be updated, and add IDDChange label
  • If structural output changes, add to output rules file and add OutputChange label
  • If adding/removing any LaTeX docs or figures, update that document's CMakeLists file dependencies

Reviewer

This will not be exhaustively relevant to every PR.

  • Perform a Code Review on GitHub
  • If branch is behind develop, merge develop and build locally to check for side effects of the merge
  • If defect, verify by running develop branch and reproducing defect, then running PR and reproducing fix
  • If feature, test running new feature, try creative ways to break it
  • CI status: all green or justified
  • Check that performance is not impacted (CI Linux results include performance check)
  • Run Unit Test(s) locally
  • Check any new function arguments for performance impacts
  • Verify IDF naming conventions and styles, memos and notes and defaults
  • If new idf included, locally check the err file and other outputs

```
[ RUN      ] EnergyPlusFixture.CondenserLoopTowers_VSCoolingTower_OutputReport
/home/julien/Software/Others/EnergyPlus2/tst/EnergyPlus/unit/CondenserLoopTowers.unit.cc:4925: Failure
Expected equality of these values:
  expectedDesOutletWaterTemp
    Which is: 27.84
  VSTower.DesOutletWaterTemp
    Which is: 0
/home/julien/Software/Others/EnergyPlus2/tst/EnergyPlus/unit/CondenserLoopTowers.unit.cc:4926: Failure
Expected equality of these values:
  expectedDesInletWaterTemp
    Which is: 33.98
  VSTower.DesInletWaterTemp
    Which is: 0
/home/julien/Software/Others/EnergyPlus2/tst/EnergyPlus/unit/CondenserLoopTowers.unit.cc:4952: Failure
Expected equality of these values:
  fmt::format("{:.2f}", expectedDesignInletWB)
    Which is: "23.28"
  OutputReportPredefined::RetrievePreDefTableEntry(*state, orp.pdchCTFCDesInletAirWBT, TowerName)
    Which is: "0.00"
/home/julien/Software/Others/EnergyPlus2/tst/EnergyPlus/unit/CondenserLoopTowers.unit.cc:4955: Failure
Expected equality of these values:
  fmt::format("{:.2f}", expectedDesOutletWaterTemp)
    Which is: "27.84"
  OutputReportPredefined::RetrievePreDefTableEntry(*state, orp.pdchCTFCLevWaterSPTemp, TowerName)
    Which is: "0.00"
```
| Kept           | Removed           |
|----------------|-------------------|
| DesignInletWB  | DesInletAirWBTemp |
| DesignApproach | DesApproach       |
| DesignRange    | DesRange          |
…esInletWaterTemp

All other CT types do it properly already
@jmarrec jmarrec added Defect Includes code to repair a defect in EnergyPlus NotIDDChange Code does not impact IDD (can be merged after IO freeze) OutputChange Code changes output in such a way that it cannot be merged after IO freeze labels Jan 21, 2025
@jmarrec jmarrec self-assigned this Jan 21, 2025
Comment on lines -250 to -252
Real64 DesInletAirWBTemp = 0.0; // design tower outlet air wet-bulb temperature (C)
Real64 DesApproach = 0.0; // design tower approach temperature (deltaC)
Real64 DesRange = 0.0; // design tower range temperature (deltaC)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Remove duplicated Variables on CoolingTower struct. I kept only the ones defined first in the file.

Kept Removed
DesignInletWB DesInletAirWBTemp
DesignApproach DesApproach
DesignRange DesRange

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Real64 DesignInletWB = 0.0; // Design inlet air wet-bulb temperature (C)
Real64 DesignApproach = 0.0; // Design approach (outlet water temp minus inlet air wet-bulb temp (C)
Real64 DesignRange = 0.0; // Design range temperature (inlet water temp minus outlet water temp (C)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The report was zero for "Cooling Tower Design Inlet Wet-Bulb Temperature" because of the fact that two variables exited for the same quantity.

DesignInletWB was the one that stored the CT:VariableSpeed input:

tower.DesignInletWB = NumArray(1);

But the other one was reported

OutputReportPredefined::PreDefTableEntry(state, state.dataOutRptPredefined->pdchCTFCDesInletAirWBT, this->Name, this->DesInletAirWBTemp);

Comment on lines +1347 to +1349
// set tower design water outlet and inlet temperatures
tower.DesOutletWaterTemp = tower.DesignInletWB + tower.DesignApproach;
tower.DesInletWaterTemp = tower.DesOutletWaterTemp + tower.DesignRange;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These two variables were properly calculated for all CT Types except CoolingTower:VariableSpeed.

This fixes the "Leaving Water Setpoint Temperature" displaying as zero

Comment on lines +4569 to +4580
" CoolingTower:VariableSpeed,",
" CoolingTower Variable Speed, !- Name",
" Tower Inlet Node, !- Water Inlet Node Name",
" Tower Outlet Node, !- Water Outlet Node Name",
" CoolToolsCrossFlow, !- Model Type",
" , !- Model Coefficient Name",
" 23.28, !- Design Inlet Air Wet-Bulb Temperature {C}",
" 4.56, !- Design Approach Temperature {deltaC}",
" 6.14, !- Design Range Temperature {deltaC}",
" 0.02, !- Design Water Flow Rate {m3/s}",
" 10.0, !- Design Air Flow Rate {m3/s}",
" 1000.0, !- Design Fan Power {W}",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unit test input

Comment on lines 4942 to 4959
VSTower.SizeTower(*state); // This calls PreDefTableEntry when state.dataPlnt->PlantFinalSizesOkayToReport == true

auto &orp = *state->dataOutRptPredefined;
std::string const TowerName = VSTower.Name;
EXPECT_EQ("CoolingTower:VariableSpeed", OutputReportPredefined::RetrievePreDefTableEntry(*state, orp.pdchCTFCType, TowerName));
EXPECT_EQ("WATER", OutputReportPredefined::RetrievePreDefTableEntry(*state, orp.pdchCTFCFluidType, TowerName));
EXPECT_EQ(fmt::format("{:.2f}", expectedDesingRange), OutputReportPredefined::RetrievePreDefTableEntry(*state, orp.pdchCTFCRange, TowerName));
EXPECT_EQ(fmt::format("{:.2f}", expectedDesingApproach),
OutputReportPredefined::RetrievePreDefTableEntry(*state, orp.pdchCTFCApproach, TowerName));
EXPECT_EQ("1000.00", OutputReportPredefined::RetrievePreDefTableEntry(*state, orp.pdchCTFCDesFanPwr, TowerName));
EXPECT_EQ(fmt::format("{:.2f}", expectedDesignInletWB),
OutputReportPredefined::RetrievePreDefTableEntry(*state, orp.pdchCTFCDesInletAirWBT, TowerName));
EXPECT_EQ("0.02", OutputReportPredefined::RetrievePreDefTableEntry(*state, orp.pdchCTFCDesWaterFlowRate, TowerName));
EXPECT_EQ(fmt::format("{:.2f}", expectedDesOutletWaterTemp),
OutputReportPredefined::RetrievePreDefTableEntry(*state, orp.pdchCTFCLevWaterSPTemp, TowerName));
EXPECT_EQ("COOLINGTOWER LOOP", OutputReportPredefined::RetrievePreDefTableEntry(*state, orp.pdchCTFCCondLoopName, TowerName));
EXPECT_EQ("COOLINGTOWER SUPPLY EQUIPMENT BRANCH 1",
OutputReportPredefined::RetrievePreDefTableEntry(*state, orp.pdchCTFCCondLoopBranchName, TowerName));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Check that the report is ok.

Before fix:

[ RUN      ] EnergyPlusFixture.CondenserLoopTowers_VSCoolingTower_OutputReport
/home/julien/Software/Others/EnergyPlus2/tst/EnergyPlus/unit/CondenserLoopTowers.unit.cc:4925: Failure
Expected equality of these values:
  expectedDesOutletWaterTemp
    Which is: 27.84
  VSTower.DesOutletWaterTemp
    Which is: 0
/home/julien/Software/Others/EnergyPlus2/tst/EnergyPlus/unit/CondenserLoopTowers.unit.cc:4926: Failure
Expected equality of these values:
  expectedDesInletWaterTemp
    Which is: 33.98
  VSTower.DesInletWaterTemp
    Which is: 0
/home/julien/Software/Others/EnergyPlus2/tst/EnergyPlus/unit/CondenserLoopTowers.unit.cc:4952: Failure
Expected equality of these values:
  fmt::format("{:.2f}", expectedDesignInletWB)
    Which is: "23.28"
  OutputReportPredefined::RetrievePreDefTableEntry(*state, orp.pdchCTFCDesInletAirWBT, TowerName)
    Which is: "0.00"
/home/julien/Software/Others/EnergyPlus2/tst/EnergyPlus/unit/CondenserLoopTowers.unit.cc:4955: Failure
Expected equality of these values:
  fmt::format("{:.2f}", expectedDesOutletWaterTemp)
    Which is: "27.84"
  OutputReportPredefined::RetrievePreDefTableEntry(*state, orp.pdchCTFCLevWaterSPTemp, TowerName)
    Which is: "0.00"

@jmarrec jmarrec changed the title 10889 cooling tower report wbt Fix #10889 - Cooling Tower Design Inlet Air Wet-Blub temperature is 25 C in idf however is 0C in eplustbl.htm Jan 21, 2025
@jmarrec jmarrec requested a review from Myoldmopar January 21, 2025 08:16
@jmarrec
Copy link
Contributor Author

jmarrec commented Jan 21, 2025

Using https://github.com/NREL/EnergyPlusDevSupport/blob/master/DefectFiles/10000s/10889/in.idf

Before:

image


After (I didn't increase the water flow rate precision from 2 to 6 digits (yet?)):

image

@github-actions
Copy link

⚠️ Regressions detected on macos-14 for commit 31a5812

Regression Summary
  • Table Big Diffs: 236
  • Table Small Diffs: 2

tower.DesApproach = NumArray(15);
if (tower.DesApproach == DataSizing::AutoSize || tower.DesApproach == 0) {
tower.DesApproach = 3.9;
tower.DesignApproach = NumArray(15);
Copy link
Contributor Author

@jmarrec jmarrec Jan 21, 2025

Choose a reason for hiding this comment

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

This is CoolingTower:SingleSpeed, Range and Approach where stored in the wrong variable as well

OutputReportPredefined::PreDefTableEntry(state, state.dataOutRptPredefined->pdchCTFCRange, this->Name, this->DesignRange);
OutputReportPredefined::PreDefTableEntry(state, state.dataOutRptPredefined->pdchCTFCApproach, this->Name, this->DesignApproach);

Take 5ZoneBoilerOutsideAirReset.idf, which has a CT with defaulted Range and Approach.

Develop:

image

This PR:

image

tower.DesApproach = NumArray(23);
if (tower.DesApproach == DataSizing::AutoSize || tower.DesApproach == 0) {
tower.DesApproach = 3.9;
tower.DesignApproach = NumArray(23);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is CoolingTower:TwoSpeed, same thing, Approach and Range are incorrectly reported before fix

Comment on lines +1647 to +1654
tower.DesignApproach = NumArray(19);
if (tower.DesignApproach == DataSizing::AutoSize || tower.DesignApproach == 0) {
tower.DesignApproach = 3.9;
tower.TowerInletCondsAutoSize = true;
}
tower.DesRange = NumArray(20);
if (tower.DesRange == DataSizing::AutoSize || tower.DesRange == 0) {
tower.DesRange = 5.5;
tower.DesignRange = NumArray(20);
if (tower.DesignRange == DataSizing::AutoSize || tower.DesignRange == 0) {
tower.DesignRange = 5.5;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is VariableSpeedMerkel, also Range and Approach not reported correctly

@github-actions
Copy link

⚠️ Regressions detected on macos-14 for commit 273d522

Regression Summary
  • Table Big Diffs: 236
  • Table Small Diffs: 2

@jmarrec
Copy link
Contributor Author

jmarrec commented Jan 21, 2025

The output changes are expected for files with a CoolingTower in it.

First, sampling one file with each CT type, using the query

SELECT RowName, ColumnName, Units, Value FROM TabularDataWithStrings
  WHERE ReportName = 'EquipmentSummary'
    AND TableName = 'Cooling Towers and Fluid Coolers'

And comparing develop (ori) to this PR (new):

CoolingTower:SingleSpeed - 5ZoneBoilerOutsideAirReset

RowName ColumnName Units ori new
CENTRAL TOWER Type CoolingTower:SingleSpeed CoolingTower:SingleSpeed
CENTRAL TOWER Range C 0.00 5.50
CENTRAL TOWER Approach C 0.00 3.90
CENTRAL TOWER Design Water Flow Rate m3/s 0.00 0.002179

CoolingTower:TwoSpeed - CoolingTower_TwoSpeed

RowName ColumnName Units ori new
BIG TOWER1 Type CoolingTower:TwoSpeed CoolingTower:TwoSpeed
BIG TOWER2 Type CoolingTower:TwoSpeed CoolingTower:TwoSpeed
BIG TOWER1 Range C 0.00 5.50
BIG TOWER2 Range C 0.00 5.50
BIG TOWER1 Approach C 0.00 3.90
BIG TOWER2 Approach C 0.00 3.90
BIG TOWER1 Design Water Flow Rate m3/s 0.00 0.001076
BIG TOWER2 Design Water Flow Rate m3/s 0.00 0.001076

CoolingTower:VariableSpeed - 2ZoneDataCenterHVAC_wEconomizer

RowName ColumnName Units ori new
COOLING TOWER Type CoolingTower:VariableSpeed CoolingTower:VariableSpeed
COOLING TOWER Design Inlet Air Wet-Bulb Temperature C 0.00 25.60
COOLING TOWER Design Water Flow Rate m3/s 0.11 0.109717
COOLING TOWER Leaving Water Setpoint Temperature C 0.00 29.50

CoolingTower:VariableSpeed:Merkel - CoolingTower_MerkelVariableSpeed

  • No diff because Merkel DOES NOT REPORT TO THE HTML

HVACTemplate:Plant:Tower - HAMT_DailyProfileReport

RowName ColumnName Units ori new
MAIN TOWER Type CoolingTower:SingleSpeed CoolingTower:SingleSpeed
MAIN TOWER Range C 0.00 5.50
MAIN TOWER Approach C 0.00 3.90
MAIN TOWER Design Water Flow Rate m3/s 0.00 0.000327

@jmarrec
Copy link
Contributor Author

jmarrec commented Jan 21, 2025

Sampling 10 random reported diff files

HVACTemplate:Plant:Tower - CoolingTower_FluidBypass

RowName ColumnName Units ori new
TOWERWATERSYS COOLTOWER Type CoolingTower:SingleSpeed CoolingTower:SingleSpeed
TOWERWATERSYS COOLTOWER Range C 0.00 5.50
TOWERWATERSYS COOLTOWER Approach C 0.00 3.90
TOWERWATERSYS COOLTOWER Design Water Flow Rate m3/s 0.26 0.261332

HVACTemplate:Plant:Tower - PythonPluginLrgOff_GridStorageSmoothing

RowName ColumnName Units ori new
TOWERWATERSYS COOLTOWER Type CoolingTower:SingleSpeed CoolingTower:SingleSpeed
TOWERWATERSYS COOLTOWER Range C 0.00 5.50
TOWERWATERSYS COOLTOWER Approach C 0.00 3.90
TOWERWATERSYS COOLTOWER Design Water Flow Rate m3/s 0.21 0.208903

HVACTemplate:Plant:Tower - 5ZoneCoolBeam

RowName ColumnName Units ori new
CENTRAL TOWER Type CoolingTower:SingleSpeed CoolingTower:SingleSpeed
CENTRAL TOWER Range C 0.00 5.50
CENTRAL TOWER Approach C 0.00 3.90
CENTRAL TOWER Design Water Flow Rate m3/s 0.00 0.000933

HVACTemplate:Plant:Tower - LgOffVAV

RowName ColumnName Units ori new
BIG TOWER Type CoolingTower:SingleSpeed CoolingTower:SingleSpeed
BIG TOWER Range C 0.00 5.50
BIG TOWER Approach C 0.00 3.90
BIG TOWER Design Water Flow Rate m3/s 0.04 0.044671

HVACTemplate:Plant:Tower - 5ZoneWarmestMultDDSizVAV

RowName ColumnName Units ori new
CENTRAL TOWER Type CoolingTower:SingleSpeed CoolingTower:SingleSpeed
CENTRAL TOWER Range C 0.00 5.50
CENTRAL TOWER Approach C 0.00 3.90
CENTRAL TOWER Design Water Flow Rate m3/s 0.00 0.002265

HVACTemplate:Plant:Tower - TermReheatZoneExh

RowName ColumnName Units ori new
BIG TOWER Type CoolingTower:SingleSpeed CoolingTower:SingleSpeed
BIG TOWER Range C 0.00 5.50
BIG TOWER Approach C 0.00 3.90
BIG TOWER Design Water Flow Rate m3/s 0.00 0.001100

HVACTemplate:Plant:Tower - ASHRAE901_OfficeLarge_STD2019_Denver_Chiller205

RowName ColumnName Units ori new
CENTRAL TOWER Type FluidCooler:TwoSpeed FluidCooler:TwoSpeed
TOWERWATERSYS COOLTOWER 1 Type CoolingTower:VariableSpeed CoolingTower:VariableSpeed
TOWERWATERSYS COOLTOWER 2 Type CoolingTower:VariableSpeed CoolingTower:VariableSpeed
TOWERWATERSYS COOLTOWER 1 Design Inlet Air Wet-Bulb Temperature C 0.00 20.00
TOWERWATERSYS COOLTOWER 2 Design Inlet Air Wet-Bulb Temperature C 0.00 20.00
TOWERWATERSYS COOLTOWER 1 Design Water Flow Rate m3/s 0.04 0.041082
TOWERWATERSYS COOLTOWER 2 Design Water Flow Rate m3/s 0.04 0.041082
TOWERWATERSYS COOLTOWER 1 Leaving Water Setpoint Temperature C 0.00 25.56
TOWERWATERSYS COOLTOWER 2 Leaving Water Setpoint Temperature C 0.00 25.56

HVACTemplate:Plant:Tower - 5ZoneFPIU

RowName ColumnName Units ori new
CENTRAL TOWER Type CoolingTower:SingleSpeed CoolingTower:SingleSpeed
CENTRAL TOWER Range C 0.00 5.50
CENTRAL TOWER Approach C 0.00 3.90
CENTRAL TOWER Design Water Flow Rate m3/s 0.00 0.002003

HVACTemplate:Plant:Tower - DualDuctConstVolDamper

RowName ColumnName Units ori new
BIG TOWER Type CoolingTower:SingleSpeed CoolingTower:SingleSpeed
BIG TOWER Range C 0.00 5.50
BIG TOWER Approach C 0.00 3.90
BIG TOWER Design Water Flow Rate m3/s 0.00 0.001100

HVACTemplate:Plant:Tower - Fault_ChillerSWTSensorOffset_RefBldgLargeOfficeNew2004

RowName ColumnName Units ori new
TOWERWATERSYS COOLTOWER Type CoolingTower:SingleSpeed CoolingTower:SingleSpeed
TOWERWATERSYS COOLTOWER Range C 0.00 5.50
TOWERWATERSYS COOLTOWER Approach C 0.00 3.90
TOWERWATERSYS COOLTOWER Design Water Flow Rate m3/s 0.21 0.211608

@jmarrec jmarrec changed the title Fix #10889 - Cooling Tower Design Inlet Air Wet-Blub temperature is 25 C in idf however is 0C in eplustbl.htm Fix #10889 - Fix reporting of Cooling Towers in Equipment Summary - Cooling Towers and Fluid Coolers Jan 21, 2025
@jmarrec jmarrec force-pushed the 10889_CoolingTower_ReportWBT branch from 294b282 to c2fee76 Compare January 21, 2025 13:32
@github-actions
Copy link

⚠️ Regressions detected on macos-14 for commit b012ea3

Regression Summary
  • Table Big Diffs: 236
  • Table Small Diffs: 2

Copy link
Member

@Myoldmopar Myoldmopar left a comment

Choose a reason for hiding this comment

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

Wow. What a find. Thanks @jmarrec, this is a really great set of fixes. Testing it now.

@Myoldmopar
Copy link
Member

I found a unit test failure when running locally. I ended up having to add an init_state call to the new unit test to get the schedule input structures in place. I'm going to let CI run to make sure it's happy up here before merging.

@github-actions
Copy link

github-actions bot commented Feb 4, 2025

⚠️ Regressions detected on macos-14 for commit ff8cd43

Regression Summary
  • Table Big Diffs: 236
  • Table Small Diffs: 2

@Myoldmopar
Copy link
Member

All good here, and same minor regressions as before. Merging, thanks @jmarrec

@Myoldmopar Myoldmopar merged commit d8b3c59 into develop Feb 5, 2025
9 checks passed
@Myoldmopar Myoldmopar deleted the 10889_CoolingTower_ReportWBT branch February 5, 2025 15:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Defect Includes code to repair a defect in EnergyPlus NotIDDChange Code does not impact IDD (can be merged after IO freeze) OutputChange Code changes output in such a way that it cannot be merged after IO freeze

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Cooling Tower Design Inlet Air Wet-Blub temperature is 25 C in idf however is 0C in eplustbl.htm

6 participants