-
Notifications
You must be signed in to change notification settings - Fork 195
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
Update to v9.6.0-IOFreeze #4406
Conversation
s = availabilityManagerListIdf.getString(openstudio::AvailabilityManagerAssignmentListFields::Name); | ||
if (s) { | ||
idfObject.setString(openstudio::AirLoopHVAC_OutdoorAirSystemFields::AvailabilityManagerListName, *s); | ||
} | ||
|
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.
Should we remove it from model (OpenStudio.idd) as well? Otherwise, is there a new mapping happening that I've missed?
src/model/SizingSystem.cpp
Outdated
setSizingOption("NonCoincident"); | ||
setAllOutdoorAirinCooling("No"); | ||
setAllOutdoorAirinHeating("No"); | ||
setCentralCoolingDesignSupplyAirHumidityRatio(0.0085); | ||
setCentralHeatingDesignSupplyAirHumidityRatio(0.0080); | ||
setCoolingDesignAirFlowMethod("DesignDay"); | ||
setCoolingDesignAirFlowRate(0.0); | ||
setHeatingDesignAirFlowMethod("DesignDay"); | ||
setHeatingDesignAirFlowRate(0.0); | ||
setSystemOutdoorAirMethod("ZoneSum"); |
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.
@joseph-robertson A bunch of these are no longer present in the Ctor, while they should. At the very least the
setAllOutdoorAirinHeating("No");
setAllOutdoorAirinCooling("No");
Please put them all back.
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.
This is the major source of EUI deviations in the Regression tests so far
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.
Ok so just to recap, this is what happened:
- I commented out set methods in the ctor that were just setting to default values (i.e., unnecessary calls to set methods)
- By commenting out
setAll
methods, we have the unintended consequence of using default values - We don't want to use idd defaults because it causes EUI deviations, so we'll stick with non-default values
…t 100% OA to preserve historical behavior See Notes in SizingSystem.cpp ctor. See this compiler explorer link to see the bug in action: https://godbolt.org/z/ojK1Y4G9j
43bbe6f
to
e22f7b0
Compare
Reg results look good. Once the latest commit is done building I think this looks ready to drop in! |
Addresses #3910, realign OS:SizingPeriod:DesignDay with E+ IDD
CI Results for 7b9ddd4:
|
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.
All looks good!
Pull request overview
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.