Skip to content

Allow water heater desuperheater with Coil:Cooling:WaterToAirHeatPump:VariableSpeedEquationFit #10945

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 4 commits into from
Mar 14, 2025
Merged

Allow water heater desuperheater with Coil:Cooling:WaterToAirHeatPump:VariableSpeedEquationFit #10945

Myoldmopar merged 4 commits into from
Mar 14, 2025

Conversation

@yzhou601
Copy link
Contributor

@yzhou601 yzhou601 commented Feb 20, 2025 •
edited
Loading

Pull request overview

  • Fixes #ISSUENUMBERHERE

Description of the purpose of this PR

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

@yzhou601 yzhou601 self-assigned this Feb 20, 2025
@yzhou601 yzhou601 added the NewFeature Includes code to add a new feature to EnergyPlus label Feb 20, 2025
@yzhou601 yzhou601 mentioned this pull request Feb 20, 2025
Merged
20 tasks
@yzhou601 yzhou601 added this to the EnergyPlus 25.1 milestone Feb 20, 2025
@yzhou601
Copy link
Contributor Author

yzhou601 commented Feb 26, 2025

Local test files comparing the results w and w/o desuperheater:
image
Desuperheater coil is working, the heating energy was subtracted from water-to-air cooling coil source energy.
There's a storage tank connected to desuperheater serving as a preheat tank in the test file, and the temperature of these two tanks look like:
image
The results look good to me.
@Myoldmopar Let me know if we need to add a new E+ IDF file to the repository or is the unit test sufficient, if so it's ready for review.

@github-actions
Copy link

github-actions bot commented Feb 26, 2025

⚠️ Regressions detected on macos-14 for commit ca5aaac

Regression Summary
  • ESO Big Diffs: 1

@github-actions
Copy link

github-actions bot commented Feb 26, 2025

⚠️ Regressions detected on macos-14 for commit 183087c

Regression Summary
  • ESO Big Diffs: 1

@joseph-robertson joseph-robertson mentioned this pull request Feb 27, 2025
Closed
5 tasks
yzhou601
yzhou601 commented Feb 28, 2025
View reviewed changes
src/EnergyPlus/VariableSpeedCoils.cc
state.dataHeatBal->HeatReclaimVS_DXCoil(DXCoilNum).AvailCapacity = state.dataVariableSpeedCoils->QSource;
state.dataHeatBal->HeatReclaimVS_Coil(DXCoilNum).AvailCapacity = state.dataVariableSpeedCoils->QSource;
if (state.dataHeatBal->HeatReclaimVS_Coil(DXCoilNum).WaterHeatingDesuperheaterReclaimedHeatTotal > 0.0)
state.dataVariableSpeedCoils->QSource -= state.dataHeatBal->HeatReclaimVS_Coil(DXCoilNum).WaterHeatingDesuperheaterReclaimedHeatTotal;
Copy link
Contributor Author

@yzhou601 yzhou601 Feb 28, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

The CI diffs are caused by changing the QSource to subtract the heat being reclaimed for air-to-air DX coils, I did this for single-speed water-to-air coils in #7251, and I felt there's no reason that the air source DX coils not to subtract the reclaimed heat. Let me know if this makes sense.

@yzhou601 yzhou601 marked this pull request as ready for review February 28, 2025 18:36
@yzhou601 yzhou601 requested a review from Myoldmopar February 28, 2025 18:36
@shorowit shorowit mentioned this pull request Mar 4, 2025
Merged
14 tasks
@Myoldmopar
Copy link
Member

Myoldmopar commented Mar 14, 2025

I'm good with this. Glad it got wrapped up since we had already gotten the IDD change in. I'll give it a quick sanity check locally with latest develop now.

@Myoldmopar
Copy link
Member

Myoldmopar commented Mar 14, 2025

All good, merging. Thanks @yzhou601

@Myoldmopar Myoldmopar merged commit 4d3263a into develop Mar 14, 2025
9 checks passed
@Myoldmopar Myoldmopar deleted the Desuperheater_multispeed_watertoair_equationfit branch March 14, 2025 18:20
@Myoldmopar
Copy link
Member

Myoldmopar commented Mar 14, 2025

OK, so this PR was passing 100% before merging. I pulled develop in and re-ran release testing, and it passed 100%. However, after the merge, a couple failures popped up in the debug tests. I'm rebuilding for debug now and will see.

@yzhou601 yzhou601 mentioned this pull request Apr 9, 2025
Open
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Myoldmopar Myoldmopar Awaiting requested review from Myoldmopar

Assignees

@Myoldmopar Myoldmopar

@yzhou601 yzhou601

Labels

NewFeature Includes code to add a new feature to EnergyPlus

Projects

None yet

Milestone

EnergyPlus 25.1

Development

Successfully merging this pull request may close these issues.

5 participants

@yzhou601 @Myoldmopar @nrel-bot-2 @nrel-bot-2c