Skip to content

Conversation

mjwitte
Copy link
Contributor

@mjwitte mjwitte commented Apr 23, 2025

Pull request overview

  • Fixes EvaporativeFluidCooler:TwoSpeed not populating Cooling Towers and Fluid Coolers table in Equipment Summary #10997
  • For EvaporativeFluidCooler:SingleSpeed and EvaporativeFluidCooler:TwoSpeed update the following input fields:
    • Design Entering Water Temperature: make autosizable, default to autosize (takes the value from Sizing:Plant)
    • Design Entering Air Temperature: add default of 35.0C (same as the hard-coded value for the StandardDesignCapacity option)
    • Design Entering Air Wet-bulb Temperature: add default of 25.6C (same as the hard-coded value for the StandardDesignCapacity option)
    • No transition necessary, just new defaults.

Testfile status (with this branch)

Filename Equipment Type autosized? Sizing:Plant Object? Present in Output? All Data Filled?
ASHRAE901_ApartmentHighRise_STD2019_Denver EvaporativeFluidCooler:TwoSpeed Y Y Y Y
CoolingTower_MerkelVariableSpeed CoolingTower:VariableSpeed:Merkel Y Y Y Y
CoolingTower_SingleSpeed_MultiCell CoolingTower:SingleSpeed Y Y Y Y
CoolingTower_TwoSpeed CoolingTower:TwoSpeed N N Y Y
CoolingTower_VariableSpeed CoolingTower:VariableSpeed Y Y Y Y
CoolingTower CoolingTower:SingleSpeed N N Y Y
EvaporativeFluidCooler_TwoSpeed EvaporativeFluidCooler:TwoSpeed Y Y Y Y
EvaporativeFluidCooler EvaporativeFluidCooler:SingleSpeed N N Y Y
FluidCooler FluidCooler:SingleSpeed N N Y Y
FluidCoolerTwoSpeed FluidCooler:TwoSpeed Mixed Y Y Y

Pull Request Author

  • 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

  • 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

@mjwitte mjwitte added the Defect Includes code to repair a defect in EnergyPlus label Apr 23, 2025
Copy link

⚠️ Regressions detected on macos-14 for commit baf7c82

Regression Summary
  • Audit: 3
  • Table String Diffs: 3
  • Table Big Diffs: 2

@nrel-bot-2
Copy link

@mjwitte @Myoldmopar it has been 28 days since this pull request was last updated.

@nrel-bot-2c
Copy link

@mjwitte @Myoldmopar it has been 34 days since this pull request was last updated.

@mjwitte mjwitte added the RPD label Jul 7, 2025
Copy link

⚠️ Regressions detected on macos-14 for commit cbdb3ec

Regression Summary
  • Audit: 3
  • Table String Diffs: 3

Copy link

⚠️ Regressions detected on macos-14 for commit c0248a9

Regression Summary
  • Audit: 3
  • Table String Diffs: 3

Copy link

⚠️ Regressions detected on macos-14 for commit 688b97f

Regression Summary
  • Audit: 3
  • Table String Diffs: 3
  • Table Big Diffs: 1

@mjwitte mjwitte added this to the EnergyPlus 25.2 IO Freeze milestone Aug 1, 2025
@mjwitte mjwitte added the IDDChange Code changes impact the IDD file (cannot be merged after IO freeze) label Aug 5, 2025
@mjwitte mjwitte changed the title Fix Evaporative Fluid Cooler Table Reports Fix Evaporative Fluid Cooler Table Reports and Add Design Entering Temperature Defaults Aug 5, 2025
@mjwitte mjwitte marked this pull request as ready for review August 5, 2025 18:00
@mjwitte
Copy link
Contributor Author

mjwitte commented Aug 5, 2025

EvaporativeFluidCooler.idf
Develop:
image
image

This branch:
image
image

@mjwitte
Copy link
Contributor Author

mjwitte commented Aug 5, 2025

EvaporativeFluidCooler_TwoSpeed.idf
Develop:
image
image

This branch:
image
image

Copy link
Contributor Author

@mjwitte mjwitte left a comment

Choose a reason for hiding this comment

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

Code walkthru

Real64 tmpHighSpeedAirFlowRate = this->HighSpeedAirFlowRate;

int PltSizCondNum = state.dataPlnt->PlantLoop(this->plantLoc.loopNum).PlantSizNum;
if (PltSizCondNum > 0) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Get relevant values from sizing:plant if present.

bool LowSpeedEvapFluidCoolerUAWasAutoSized = false; // true if low speed UA set to autosize on input
Real64 LowSpeedEvapFluidCoolerUASizingFactor = 0.0; // sizing factor for low speed UA []
Real64 DesignEnteringWaterTemp = 0.0; // Entering water temperature at design conditions
Real64 DesignExitWaterTemp = -999; // Leaving water temperature at design conditions [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.

Add this to the struct

Copy link
Contributor

Choose a reason for hiding this comment

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

Is that a new glycol limit?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed to DataSizing::AutoSize

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed to DataSizing::AutoSize

Oh, but it's not supposed to be that. -999 was a lazy way to track if it was set along the way. I guess I'll keep it this way for now and make a todo for later.

Copy link
Contributor

Choose a reason for hiding this comment

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

No need for a ToDo unless that would cover all places that use this method. See ReportCoilSelection for a slew of these real values, e.g., -999.0

DesignEnteringAirWetBulb = this->DesignEnteringAirWetBulbTemp;
}
if (state.dataSize->PlantSizData(PltSizCondNum).ExitTemp <= DesignEnteringAirWetBulb) {
if (this->DesignExitWaterTemp <= DesignEnteringAirWetBulb) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Use this->DesignExitWaterTemp throughout.

this->DesignEnteringWaterTemp = this->inletConds.WaterTemp;
this->inletConds.AirTemp = 35.0; // 95F design inlet air dry-bulb temp
this->inletConds.AirWetBulb = 25.6; // 78F design inlet air wet-bulb temp
this->DesignEnteringAirWetBulbTemp = this->inletConds.AirWetBulb;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Set this->DesignEnteringAirWetBulbTemp in all sizing options so it can be reported.

Real64 OutWaterTemp; // outlet water temperature [C]
this->SimSimpleEvapFluidCooler(state, par1, par2, UA, OutWaterTemp);
Real64 const CoolingOutput = Cp * par1 * (this->inletConds.WaterTemp - OutWaterTemp);
this->SimSimpleEvapFluidCooler(state, par1, par2, UA, this->DesignExitWaterTemp);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Set this->DesignExitWaterTemp in all sizing options so it can be reported.

// create predefined report
std::string equipName = this->Name;
OutputReportPredefined::PreDefTableEntry(state, state.dataOutRptPredefined->pdchMechType, equipName, this->EvapFluidCoolerType);
OutputReportPredefined::PreDefTableEntry(state,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Add a bunch of missing table outputs.

Copy link

github-actions bot commented Aug 5, 2025

⚠️ Regressions detected on macos-14 for commit d1e000f

Regression Summary
  • Audit: 3
  • Table String Diffs: 3
  • Table Big Diffs: 1

@mjwitte
Copy link
Contributor Author

mjwitte commented Aug 5, 2025

Diffs as expected:

* Diffs by Type *:
{
  "Audit": [
    "ASHRAE901_ApartmentHighRise_STD2019_Denver",
    "EvaporativeFluidCooler",
    "EvaporativeFluidCooler_TwoSpeed"
  ],
  "Table Big Diffs": [
    "EvaporativeFluidCooler"
  ],
  "Table String Diffs": [
    "ASHRAE901_ApartmentHighRise_STD2019_Denver",
    "EvaporativeFluidCooler",
    "EvaporativeFluidCooler_TwoSpeed"
  ]

@Myoldmopar Myoldmopar self-assigned this Aug 27, 2025
@mjwitte mjwitte requested a review from rraustad September 10, 2025 17:39
@mjwitte
Copy link
Contributor Author

mjwitte commented Sep 10, 2025

@rraustad Would appreciate your review here, especially for the new IDD defaults.

this->DesignExitWaterTemp = state.dataSize->PlantSizData(PltSizCondNum).ExitTemp;
if (this->DesignEnteringWaterTemp == DataSizing::AutoSize) {
this->DesignEnteringWaterTemp =
state.dataSize->PlantSizData(PltSizCondNum).ExitTemp + state.dataSize->PlantSizData(PltSizCondNum).DeltaT;
Copy link
Contributor

Choose a reason for hiding this comment

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

Documentation above says The design entering water temperature must be greater than the design entering air temperature. Does that need to be checked when autosizing? I do see at line 1381 where exit water temp must be greater than entering air WB so maybe that is sufficient when autosizing. getInput does this:

            if (thisEFC.DesignEnteringWaterTemp <= thisEFC.DesignEnteringAirWetBulbTemp) {
                ShowSevereError(state,
                                format("{} = \"{}\", {} must be greater than {}.",
                                       state.dataIPShortCut->cCurrentModuleObject,
                                       AlphArray(1),
                                       state.dataIPShortCut->cNumericFieldNames(9),
                                       state.dataIPShortCut->cNumericFieldNames(11)));
                ErrorsFound = true;
            }
            if (thisEFC.DesignEnteringAirTemp <= thisEFC.DesignEnteringAirWetBulbTemp) {
                ShowSevereError(state,
                                format("{} = \"{}\", {} must be greater than {}.",
                                       state.dataIPShortCut->cCurrentModuleObject,
                                       AlphArray(1),
                                       state.dataIPShortCut->cNumericFieldNames(10),
                                       state.dataIPShortCut->cNumericFieldNames(11)));
                ErrorsFound = true;
            }

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Documentation above says The design entering water temperature must be greater than the design entering air temperature. Does that need to be checked when autosizing? I do see at line 1381 where exit water temp must be greater than entering air WB so maybe that is sufficient when autosizing. getInput does this:

@rraustad Hmm, not sure if this is ok or not. I may make some revisions in a follow-up PR.

@mitchute I've merged in develop. Assuming CI comes back as expected. This can merge.

N21, \field Design Entering Air Wet-bulb Temperature
\type real
\units C
\default 25.6
Copy link
Contributor

Choose a reason for hiding this comment

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

I think for defaults these are reasonably extreme. The user can adjust as they see fit.

@rraustad
Copy link
Contributor

Corrections to table reports, as shown in above examples, look much better.

Copy link
Contributor

@rraustad rraustad left a comment

Choose a reason for hiding this comment

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

Other than any adjustments based on limited comments I think this is ready.

Copy link

⚠️ Regressions detected on ubuntu-24.04 for commit 02471ca

Regression Summary
  • Audit: 3
  • Table String Diffs: 3
  • Table Big Diffs: 1
  • ESO Small Diffs: 1

Copy link

⚠️ Regressions detected on macos-14 for commit 02471ca

Regression Summary
  • Audit: 3
  • Table String Diffs: 3
  • Table Big Diffs: 1

@mjwitte
Copy link
Contributor Author

mjwitte commented Sep 26, 2025

@mitchute CI is all green, diffs as before. Merge at will.

@mitchute mitchute merged commit ca324c8 into develop Sep 26, 2025
11 checks passed
@mitchute mitchute deleted the rpdTables3 branch September 26, 2025 16:11
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 IDDChange Code changes impact the IDD file (cannot be merged after IO freeze) RPD

Projects

None yet

Development

Successfully merging this pull request may close these issues.

EvaporativeFluidCooler:TwoSpeed not populating Cooling Towers and Fluid Coolers table in Equipment Summary

6 participants