Skip to content

Conversation

@jmarrec
Copy link
Collaborator

@jmarrec jmarrec commented Oct 24, 2025

Pull request overview

@joseph-robertson

Please watch this:

In case someone actually says it isn't supported, but I'm leaning towards it being required.

Pull Request Author

  • Model API Changes / Additions
  • Any new or modified fields have been implemented in the EnergyPlus ForwardTranslator (and ReverseTranslator as appropriate)
  • Model API methods are tested (in src/model/test)
  • EnergyPlus ForwardTranslator Tests (in src/energyplus/Test)
  • If a new object or method, added a test in NREL/OpenStudio-resources: Add Link
  • If needed, added VersionTranslation rules for the objects (src/osversion/VersionTranslator.cpp)
  • Verified that C# bindings built fine on Windows, partial classes used as needed, etc.
  • All new and existing tests passes
  • If methods have been deprecated, update rest of code to use the new methods

Labels:

  • If change to an IDD file, add the label IDDChange
  • If breaking existing API, add the label APIChange
  • If deemed ready, add label Pull Request - Ready for CI so that CI builds your PR

Review Checklist

This will not be exhaustively relevant to every PR.

  • Perform a Code Review on GitHub
  • Code Style, strip trailing whitespace, etc.
  • All related changes have been implemented: model changes, model tests, FT changes, FT tests, VersionTranslation, OS App
  • Labeling is ok
  • 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

@jmarrec jmarrec self-assigned this Oct 24, 2025
@jmarrec jmarrec added component - HVAC Pull Request - Ready for CI This pull request if finalized and is ready for continuous integration verification prior to merge. labels Oct 24, 2025
Copy link
Collaborator

@joseph-robertson joseph-robertson left a comment

Choose a reason for hiding this comment

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

Have we decided that the ctor for Nominal Cooling Capacity should be autosized? If so, don't we need to update VT to create a WaterHeater:Sizing object in the event the field was blank (i.e., gets autosized)?

Edit: should the ctor behavior be the same as the VT behavior? Meaning, would it make sense to have blank -> 0.0 in VT but autosize in the ctor?

@joseph-robertson
Copy link
Collaborator

Have we decided that the ctor for Nominal Cooling Capacity should be autosized? If so, don't we need to update VT to create a WaterHeater:Sizing object in the event the field was blank (i.e., gets autosized)?

Edit: should the ctor behavior be the same as the VT behavior? Meaning, would it make sense to have blank -> 0.0 in VT but autosize in the ctor?

Seems like VT should maintain the old behavior, so I'm setting blank Nominal Cooling Capacity to 0.0. 8f1c594

@joseph-robertson joseph-robertson self-requested a review November 12, 2025 20:28
Copy link
Collaborator

@joseph-robertson joseph-robertson left a comment

Choose a reason for hiding this comment

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

If you are good with the recent VT change I made, I think this PR is ready to drop in?

@jmarrec
Copy link
Collaborator Author

jmarrec commented Nov 13, 2025

If you are good with the recent VT change I made, I think this PR is ready to drop in?

I think I want the VT to actually add a WaterHeaterSizing, I overlooked that, but your change made me think about it, thanks!

Will do just now, let CI take a pass on it, and then it can merge.

@jmarrec jmarrec force-pushed the 5495_follow_up_add_WHSizing branch from 8f1c594 to be50ce1 Compare November 13, 2025 09:43
@jmarrec jmarrec merged commit d973b8f into develop Nov 13, 2025
3 of 6 checks passed
@jmarrec jmarrec deleted the 5495_follow_up_add_WHSizing branch November 13, 2025 14:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

component - HVAC Pull Request - Ready for CI This pull request if finalized and is ready for continuous integration verification prior to merge.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants