-
Notifications
You must be signed in to change notification settings - Fork 193
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.
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
v24.1.0-IOFreeze: No Load Supply Air Flow Rate Control Set To Low Speed #5096
Conversation
} | ||
} | ||
|
||
newObject.setString(10, "Yes"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ummm... should this be "No"? IDF Version Updater sets this to "No" (but the default is "Yes") ...
See resources/energyplus/Office_With_Many_HVAC_Types/in.idf
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me, but I'd like to see additions to model tests + FT tests for that field (a dozen of lines total)
Also, I'd like to ensure that the "Yes" is the former default. Is it @joseph-robertson ? (either it's documented in E+ that it is, or you've run an Openstudio-resources test that uses these objects and they show no EUI change).
Suggestions: changing to getBooleanFieldValue + shortening VT test to avoid annoying maintenance issues.
On the "No" vs "Yes" default issue... I tried running fan_on_off.rb on OpenStudio-resources, using both v3.7.0 and v3.8.0-alpha. The energy results change for both "No" and "Yes" (and are the same). So, something else must be accounting for the deviation. And not sure what to make of "No" and "Yes" having identical results. |
Assuming CI builds fine after I resolved conflicts, we can do ahead and merge this one, it's good enough to go into the main IOFreeze branch, we can figure out the Yes / No EUI deviation later on. |
CI Results for 5c31398:
|
Pull request overview
New "No Load Supply Air Flow Rate Control Set To Low Speed" field for the following objects:
ZoneHVAC:PackagedTerminalAirConditioner
ZoneHVAC:PackagedTerminalHeatPump
ZoneHVAC:WaterToAirHeatPump
AirLoopHVAC:UnitarySystem
Pull Request Author
src/model/test
)src/energyplus/Test
)src/osversion/VersionTranslator.cpp
)Labels:
IDDChange
APIChange
Pull Request - Ready for CI
so that CI builds your PRReview Checklist
This will not be exhaustively relevant to every PR.