Skip to content

Fix #11158 - Add Cooling Coil Condenser Volume (Flow Rate) variables for Coil:Cooling:DX:TwoSpeed #11176

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
Myoldmopar merged 2 commits into from
Sep 4, 2025
Merged

Fix #11158 - Add Cooling Coil Condenser Volume (Flow Rate) variables for Coil:Cooling:DX:TwoSpeed #11176

Myoldmopar merged 2 commits into from
Sep 4, 2025

Conversation

@jmarrec
Copy link
Contributor

@jmarrec jmarrec commented Aug 27, 2025 •
edited
Loading

Pull request overview

Description of the purpose of this PR

  • Add Cooling Coil Condenser Volume (Flow Rate) variables for Coil:Cooling:DX:TwoSpeed

Verified that the defect file (User supplied, transitionned to 25.1.0 and saved on DevSupport) now includes the correct variables

image image

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

@jmarrec jmarrec self-assigned this Aug 27, 2025
@jmarrec jmarrec added Defect Includes code to repair a defect in EnergyPlus NotIDDChange Code does not impact IDD (can be merged after IO freeze) labels Aug 27, 2025
jmarrec
jmarrec commented Aug 27, 2025
View reviewed changes
src/EnergyPlus/DXCoils.cc
Comment on lines +5846 to +5864
if (thisDXCoil.CondensateCollectMode == CondensateCollectAction::ToTank) {
SetupOutputVariable(state,
"Cooling Coil Condensate Volume Flow Rate",
Constant::Units::m3_s,
thisDXCoil.CondensateVdot,
OutputProcessor::TimeStepType::System,
OutputProcessor::StoreType::Average,
thisDXCoil.Name);
SetupOutputVariable(state,
"Cooling Coil Condensate Volume",
Constant::Units::m3,
thisDXCoil.CondensateVol,
OutputProcessor::TimeStepType::System,
OutputProcessor::StoreType::Sum,
thisDXCoil.Name,
Constant::eResource::OnSiteWater,
OutputProcessor::Group::HVAC,
OutputProcessor::EndUseCat::Condensate);
}
Copy link
Contributor Author

@jmarrec jmarrec Aug 27, 2025

Choose a reason for hiding this comment

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

simple fix.

Copy link
Member

@Myoldmopar Myoldmopar Sep 4, 2025

Choose a reason for hiding this comment

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

SIMPLE!

@jmarrec jmarrec force-pushed the 11158_MissingVarDXTwoSpeed branch from 77bcf22 to 5e978a8 Compare August 27, 2025 16:48
jmarrec added 2 commits August 27, 2025 18:49
@jmarrec
Add a failing test for #11158
28c5866 
```
/home/julien/Software/Others/EnergyPlus/tst/EnergyPlus/unit/DXCoils.unit.cc:3455: Failure
Expected: (std::find(outVarNames.begin(), outVarNames.end(), varName)) != (outVarNames.end()), actual: 8-byte object <90-94 00-63 C1-56 00-00> vs 8-byte object <90-94 00-63 C1-56 00-00>
Did not find expected output variable: Cooling Coil Condensate Volume Flow Rate
/home/julien/Software/Others/EnergyPlus/tst/EnergyPlus/unit/DXCoils.unit.cc:3455: Failure
Expected: (std::find(outVarNames.begin(), outVarNames.end(), varName)) != (outVarNames.end()), actual: 8-byte object <90-94 00-63 C1-56 00-00> vs 8-byte object <90-94 00-63 C1-56 00-00>
Did not find expected output variable: Cooling Coil Condensate Volume
```
@jmarrec
Fix #11158 - Add Cooling Coil Condenser Volume (Flow Rate) variables …
72805b9 
…for Coil:Cooling:DX:TwoSpeed
@jmarrec jmarrec force-pushed the 11158_MissingVarDXTwoSpeed branch from 5e978a8 to 72805b9 Compare August 27, 2025 16:50
jmarrec
jmarrec commented Aug 27, 2025
View reviewed changes
tst/EnergyPlus/unit/DXCoils.unit.cc
ASSERT_EQ(-25.0, state->dataDXCoils->DXCoil(1).MinOATCompressor); // use default value at -25C
}

TEST_F(EnergyPlusFixture, CoilCoolingDXTwoSpeed_CondensateVariables)
Copy link
Contributor Author

@jmarrec jmarrec Aug 27, 2025

Choose a reason for hiding this comment

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

New test

tst/EnergyPlus/unit/DXCoils.unit.cc
" 0.0, !- Minimum Value of x",
" 1.0; !- Maximum Value of x",

"Coil:Cooling:DX:TwoSpeed,",
Copy link
Contributor Author

@jmarrec jmarrec Aug 27, 2025

Choose a reason for hiding this comment

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

TwoSpeed coil

tst/EnergyPlus/unit/DXCoils.unit.cc
" autosize, !- Low Speed Evaporative Condenser Air Flow Rate {m3/s}",
" autosize, !- Low Speed Evaporative Condenser Pump Rated Power Consumption {W}",
" , !- Supply Water Storage Tank Name",
" condensate_storage, !- Condensate Collection Water Storage Tank Name",
Copy link
Contributor Author

@jmarrec jmarrec Aug 27, 2025

Choose a reason for hiding this comment

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

With a Condensate Collection Water Storage

tst/EnergyPlus/unit/DXCoils.unit.cc
Comment on lines +3425 to +3431
EXPECT_NO_THROW(GetDXCoils(*state));
compare_err_stream("");

auto &dxCoil = state->dataDXCoils->DXCoil(1);

ASSERT_EQ("MAIN COOLING COIL 1", dxCoil.Name); // Cooling Coil Two Speed
EXPECT_ENUM_EQ(CondensateCollectAction::ToTank, dxCoil.CondensateCollectMode);
Copy link
Contributor Author

@jmarrec jmarrec Aug 27, 2025

Choose a reason for hiding this comment

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

Get the coil. SetupOutputVariable is inside GetDXCoils, so can't do it manually.

Ensure CondensateCollectMode is ok

tst/EnergyPlus/unit/DXCoils.unit.cc
Comment on lines +3433 to +3455
std::vector<std::string> outVarNames;
outVarNames.reserve(state->dataOutputProcessor->ddOutVars.size());
std::transform(state->dataOutputProcessor->ddOutVars.begin(),
state->dataOutputProcessor->ddOutVars.end(),
std::back_inserter(outVarNames),
[](auto const *var) { return var->name; });
std::vector<std::string> expectedVarNames = {
"Cooling Coil Total Cooling Rate",
"Cooling Coil Total Cooling Energy",
"Cooling Coil Sensible Cooling Rate",
"Cooling Coil Sensible Cooling Energy",
"Cooling Coil Latent Cooling Rate",
"Cooling Coil Latent Cooling Energy",
"Cooling Coil Electricity Rate",
"Cooling Coil Electricity Energy",
"Cooling Coil Runtime Fraction",
"Cooling Coil Condensate Volume Flow Rate",
"Cooling Coil Condensate Volume",
};
for (auto const &varName : expectedVarNames) {
EXPECT_NE(std::find(outVarNames.begin(), outVarNames.end(), varName), outVarNames.end())
<< "Did not find expected output variable: " << varName;
}
Copy link
Contributor Author

@jmarrec jmarrec Aug 27, 2025

Choose a reason for hiding this comment

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

Ensure we find the variables we expect.

Before fix, the Last two are missing

/home/julien/Software/Others/EnergyPlus/tst/EnergyPlus/unit/DXCoils.unit.cc:3455: Failure
Expected: (std::find(outVarNames.begin(), outVarNames.end(), varName)) != (outVarNames.end()), actual: 8-byte object <90-94 00-63 C1-56 00-00> vs 8-byte object <90-94 00-63 C1-56 00-00>
Did not find expected output variable: Cooling Coil Condensate Volume Flow Rate
/home/julien/Software/Others/EnergyPlus/tst/EnergyPlus/unit/DXCoils.unit.cc:3455: Failure
Expected: (std::find(outVarNames.begin(), outVarNames.end(), varName)) != (outVarNames.end()), actual: 8-byte object <90-94 00-63 C1-56 00-00> vs 8-byte object <90-94 00-63 C1-56 00-00>
Did not find expected output variable: Cooling Coil Condensate Volume

Copy link
Member

@Myoldmopar Myoldmopar Sep 4, 2025

Choose a reason for hiding this comment

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

Great test here.

@jmarrec jmarrec requested a review from Myoldmopar September 4, 2025 08:42
Myoldmopar
Myoldmopar approved these changes Sep 4, 2025
View reviewed changes
tst/EnergyPlus/unit/DXCoils.unit.cc
Comment on lines +3433 to +3455
std::vector<std::string> outVarNames;
outVarNames.reserve(state->dataOutputProcessor->ddOutVars.size());
std::transform(state->dataOutputProcessor->ddOutVars.begin(),
state->dataOutputProcessor->ddOutVars.end(),
std::back_inserter(outVarNames),
[](auto const *var) { return var->name; });
std::vector<std::string> expectedVarNames = {
"Cooling Coil Total Cooling Rate",
"Cooling Coil Total Cooling Energy",
"Cooling Coil Sensible Cooling Rate",
"Cooling Coil Sensible Cooling Energy",
"Cooling Coil Latent Cooling Rate",
"Cooling Coil Latent Cooling Energy",
"Cooling Coil Electricity Rate",
"Cooling Coil Electricity Energy",
"Cooling Coil Runtime Fraction",
"Cooling Coil Condensate Volume Flow Rate",
"Cooling Coil Condensate Volume",
};
for (auto const &varName : expectedVarNames) {
EXPECT_NE(std::find(outVarNames.begin(), outVarNames.end(), varName), outVarNames.end())
<< "Did not find expected output variable: " << varName;
}
Copy link
Member

@Myoldmopar Myoldmopar Sep 4, 2025

Choose a reason for hiding this comment

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

Great test here.

@Myoldmopar
Copy link
Member

Myoldmopar commented Sep 4, 2025

Guess what? This SIMPLE fix passes happily with develop pulled in right now. Merging this! Thanks @jmarrec

@Myoldmopar Myoldmopar merged commit c866b49 into develop Sep 4, 2025
9 checks passed
@Myoldmopar Myoldmopar deleted the 11158_MissingVarDXTwoSpeed branch September 4, 2025 15:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Myoldmopar Myoldmopar Myoldmopar approved these changes

Assignees

@jmarrec jmarrec

Labels

Defect Includes code to repair a defect in EnergyPlus NotIDDChange Code does not impact IDD (can be merged after IO freeze)

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Coil:Cooling:DX:TwoSpeed not outputting condensate (Coil:Cooling:DX:SingleSpeed works fine)

4 participants

@jmarrec @Myoldmopar @nrel-bot-2