Skip to content

Fix 11042 CentralHeatPumpSystem in IDD should use type node for node names#11220

Merged
mitchute merged 3 commits intoNatLabRockies:developfrom
bigladder:Fix-11042-CentralHeatPumpSystem-in-IDD-should-use-type-node-for-node-names
Sep 30, 2025
Merged

Fix 11042 CentralHeatPumpSystem in IDD should use type node for node names#11220
mitchute merged 3 commits intoNatLabRockies:developfrom
bigladder:Fix-11042-CentralHeatPumpSystem-in-IDD-should-use-type-node-for-node-names

Conversation

@kevin-moos
Copy link
Contributor

@kevin-moos kevin-moos commented Sep 19, 2025

Pull request overview

Description of the purpose of this PR

Update Energy+.idd - CentralHeatPumpSystem should use \type node for node names

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
Copy link
Contributor

mjwitte commented Sep 25, 2025

@kevin-moos Is this ready for final review? If so, please change from "draft" to "Ready for review".

@mjwitte mjwitte added the Defect Includes code to repair a defect in EnergyPlus label Sep 25, 2025
@mjwitte mjwitte added this to the EnergyPlus 25.2 IO Freeze milestone Sep 25, 2025
@mjwitte mjwitte added the IDDChange Code changes impact the IDD file (cannot be merged after IO freeze) label Sep 25, 2025
@kevin-moos kevin-moos marked this pull request as ready for review September 26, 2025 01:42
rraustad
rraustad approved these changes Sep 30, 2025
View reviewed changes
Copy link
Collaborator

@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.

Looks correct

@mitchute
Copy link
Collaborator

mitchute commented Sep 30, 2025

Since we're here, there may be one more of these to fix.

AirflowNetwork:Distribution:Node,
      \min-fields 4
      \memo This object represents an air distribution node in the AirflowNetwork model.
 A1 , \field Name
      \required-field
      \type alpha
      \reference AirflowNetworkNodeAndZoneNames
      \note Enter a unique name for this object.
 A2 , \field Component Name or Node Name
      \type alpha
      \note Designates node names defined in another object. The node name may occur in air branches.
      \note Enter a node name to represent a node already defined in an air loop.
      \note Leave this field blank if the Node or Object Type field below is entered as
      \note AirLoopHVAC:ZoneMixer, AirLoopHVAC:ZoneSplitter, AirLoopHVAC:OutdoorAirSystem, or Other.

Can anyone confirm A2 here should also be \type node ? @jasondegraw ?

@JasonGlazer
Copy link
Contributor

JasonGlazer commented Sep 30, 2025

It is a little confusing, it is talking about an HVAC node but the field name "Component Name of Node Name" seems to imply that it can be more than an HVAC node but also a "Component Name".

In the code it seems to be used to lookup the name of an HVAC node, but it also has a comparison to an empty string or to "Other".

The structure it is in calls it:

        std::string EPlusName;     // EnergyPlus node name

Overall, I think yes, it should be \type node

But I will defer to @jasondegraw on this.

@mjwitte
Copy link
Contributor

mjwitte commented Sep 30, 2025

No, it should not be type node, because it also accepts a zone name (assuming these IDD comments are correct).

 A2 , \field Component Name or Node Name
      \type alpha
      \note Designates node names defined in another object. The node name may occur in air branches.
      \note Enter a node name to represent a node already defined in an air loop.
      \note Leave this field blank if the Node or Object Type field below is entered as
      \note AirLoopHVAC:ZoneMixer, AirLoopHVAC:ZoneSplitter, AirLoopHVAC:OutdoorAirSystem, or Other.
 A3 , \field Component Object Type or Node Type
      \type choice
<snip>
      \key Zone
      \key Other
      \default Other
      \note Designates Node type for the Node or Component Name defined in the field above.
<snip>
      \note Zone -- Enter a zone name for duct simple model to calculate duct loss without using AFN model.

@mitchute
Copy link
Collaborator

mitchute commented Sep 30, 2025

OK then. Thanks @mjwitte and @JasonGlazer. This is simple enough. Merging.

@mitchute mitchute merged commit 6f1978e into NatLabRockies:develop Sep 30, 2025
12 of 13 checks passed
@kevin-moos kevin-moos deleted the Fix-11042-CentralHeatPumpSystem-in-IDD-should-use-type-node-for-node-names branch September 30, 2025 19:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rraustad rraustad rraustad approved these changes

Assignees

@kevin-moos kevin-moos

Labels

Defect Includes code to repair a defect in EnergyPlus IDDChange Code changes impact the IDD file (cannot be merged after IO freeze)

Projects

None yet

Milestone

EnergyPlus 25.2 IO Freeze

Development

Successfully merging this pull request may close these issues.

CentralHeatPumpSystem in Energy+.idd should use \type node for node names

6 participants

@kevin-moos @mjwitte @mitchute @JasonGlazer @rraustad @nrel-bot-2