Skip to content

Add Coil:*:WaterToAirHeatPump:EquationFit as valid AirflowNetwork:Distribution:Component:Coil #10972

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 5 commits into from
Mar 17, 2025
Merged

Add Coil:*:WaterToAirHeatPump:EquationFit as valid AirflowNetwork:Distribution:Component:Coil #10972

Myoldmopar merged 5 commits into from
Mar 17, 2025

Conversation

@lymereJ
Copy link
Collaborator

@lymereJ lymereJ commented Mar 5, 2025 •
edited
Loading

Pull request overview

Follow-up to #10791, but focuses on Coil:*:WaterToAirHeatPump:EquationFit coils.

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

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

@lymereJ lymereJ added NewFeature Includes code to add a new feature to EnergyPlus AirflowNetwork Related primarily on airflow-network portions of the codebase labels Mar 5, 2025
@lymereJ lymereJ added this to the EnergyPlus 25.1 milestone Mar 5, 2025
lymereJ
lymereJ commented Mar 5, 2025
View reviewed changes
src/EnergyPlus/AirflowNetwork/src/Solver.cpp
Comment on lines 10599 to 10617
} else if (SELECT_CASE_var == "COIL:COOLING:WATERTOAIRHEATPUMP:EQUATIONFIT") {
ValidateComponent(m_state,
"Coil:Cooling:WaterToAirHeatPump:EquationFit",
DisSysCompCoilData(i).name,
IsNotOK,
format(RoutineName) + CurrentModuleObject);
if (IsNotOK) {
ErrorsFound = true;
}

} else if (SELECT_CASE_var == "COIL:HEATING:WATERTOAIRHEATPUMP:EQUATIONFIT") {
ValidateComponent(m_state,
"Coil:Heating:WaterToAirHeatPump:EquationFit",
DisSysCompCoilData(i).name,
IsNotOK,
format(RoutineName) + CurrentModuleObject);
if (IsNotOK) {
ErrorsFound = true;
}
Copy link
Collaborator Author

@lymereJ lymereJ Mar 5, 2025

Choose a reason for hiding this comment

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

Add Coil:*:WaterToAirHeatPump:EquationFit objects as a possible valid coil choice.

@lymereJ lymereJ requested a review from jasondegraw March 5, 2025 23:01
@shorowit
Copy link
Contributor

shorowit commented Mar 5, 2025 •
edited
Loading

@lymereJ Any chance you could support the Coil:*:WaterToAirHeatPump:VariableSpeedEquationFit objects too?

@lymereJ
Copy link
Collaborator Author

lymereJ commented Mar 5, 2025

I think so. I'll look to add it shortly.

@mjwitte mjwitte added the IDDChange Code changes impact the IDD file (cannot be merged after IO freeze) label Mar 6, 2025
jasondegraw
jasondegraw reviewed Mar 12, 2025
View reviewed changes
Copy link
Member

@jasondegraw jasondegraw left a comment

Choose a reason for hiding this comment

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

This looks good to go in as far as I can see.

@lymereJ we need to start compiling example models that use all of these coils. They probably won't fit in the example file set here, but that's OK. I'll follow up via email soonish.

@Myoldmopar
Copy link
Member

Myoldmopar commented Mar 17, 2025

This is also happy with develop pulled in. I checked with @jmarrec and since this object isn't wrapped in OpenStudio, we're comfortable making an exception here. But it's a lifetime single exception @lymereJ, enjoy it! 😆 Merging.

@Myoldmopar Myoldmopar merged commit c409806 into develop Mar 17, 2025
10 checks passed
@Myoldmopar Myoldmopar deleted the afn_dx_wahp_coils branch March 17, 2025 22:36
@jmarrec jmarrec mentioned this pull request Oct 22, 2025
Merged
19 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jasondegraw jasondegraw jasondegraw approved these changes

Assignees

@Myoldmopar Myoldmopar

Labels

AirflowNetwork Related primarily on airflow-network portions of the codebase IDDChange Code changes impact the IDD file (cannot be merged after IO freeze) 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.

8 participants

@lymereJ @shorowit @Myoldmopar @jasondegraw @nrel-bot-2 @nrel-bot-2c @mjwitte