-
Notifications
You must be signed in to change notification settings - Fork 208
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
Hotfix #4265 following #3863 - Do not translate ControllerList if no external controllers for AirLoopHVACDedicatedOutdoorAirSystem #4266
Conversation
// If it's not empty: then we have work to do. | ||
// If it's empty, there's at least the controller Outdoor Air UNLESS it's on a AirLoopHVACDOAS in which case do nothing | ||
if (!controllers.empty() || !modelObject.airLoopHVACDedicatedOutdoorAirSystem()) { | ||
|
||
IdfObject _controllerList(IddObjectType::AirLoopHVAC_ControllerList); |
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 actual fix. The ControllerList is translated only if we have work to do. This is based on how I understand E+ expects it...
…IDD (it's confusing)
2217be4
to
bab7db5
Compare
@eringold Could you try this branch with your specific application and let me know if that solves it please? |
@jmarrec of course, thanks for jumping on this! |
OS:AirLoopHVAC:ControllerList, | ||
\memo List controllers in order of control sequence | ||
\extensible:2 | ||
\min-fields 4 | ||
A1, \field Handle | ||
\type handle | ||
\required-field | ||
A2, \field Name | ||
\type alpha | ||
\required-field | ||
\reference ControllerLists | ||
A3, \field Controller Object Type | ||
\type choice | ||
\required-field | ||
\begin-extensible | ||
\key Controller:WaterCoil | ||
\key Controller:OutdoorAir | ||
A4; \field Controller Name | ||
\type object-list | ||
\required-field | ||
\object-list AirLoopControllers | ||
|
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.
@kbenne I removed this, as it's actually unused.
@jmarrec So this was a case of the eplus idd permitted it but there is no example idf that demonstrates it? |
@joseph-robertson essentially yes. The only eplus example file with AirLoopHVAC:DOAS is 'SmallOffice_CentralDOAS.idf' which has water coils on the AirLoopHVAC:OAS, so that ControllerList was populated. |
@joseph-robertson Yeah it's a weird corner case. The only example in E+ is SmallOffice_CentralDOAS.idf. and it has this:
So it has a control list, but because it has coils... If you look at the E+ idd, there is a hint that you may need to omit it... AirLoopHVAC:OutdoorAirSystem,
\memo Outdoor air subsystem for an AirLoopHVAC. Includes an outdoor air mixing box and
\memo optional outdoor air conditioning equipment such as heat recovery, preheat, and precool
\memo coils. From the perspective of the primary air loop the outdoor air system is treated
\memo as a single component.
\min-fields 3
A1, \field Name
\required-field
\type alpha
\reference-class-name validBranchEquipmentTypes
\reference validBranchEquipmentNames
A2, \field Controller List Name
+ \note Enter the name of an AirLoopHVAC:ControllerList object or blank if this object is used in
+ \note AirLoopHVAC:DedicatedOutdoorAirSystem.
\type object-list
\object-list ControllerLists If you ask me, E+ should just not complain if you provide a ControllerList but it references nothing... Except the ControllerList has |
CI Results for bab7db5:
|
@jmarrec this works - a ControllerList is not created when no components that require it are on the OAS. Now, since an OutdoorAirMixer is not listed in the AirLoopHVAC:OAS referenced by the AirLoopHVAC:DOAS, if you don't add any components to the DOAS's OAS, you will end up with an empty AirLoopHVAC:OutdoorAirSystem:EquipmentList, which will cause the sim to fail. But since the point of the DOAS is to contain HVAC components, I'm not sure OpenStudio needs to protect against that case. Just thought I'd mention. |
Pull request overview
Pull Request Author
Add to this list or remove from it as applicable. This is a simple templated set of guidelines.
src/model/test
)src/energyplus/Test
)src/osversion/VersionTranslator.cpp
)src/openstudio_lib/library/OpenStudioPolicy.xml
)Labels:
IDDChange
APIChange
Pull Request - Ready for CI
so that CI builds your PRReview Checklist
This will not be exhaustively relevant to every PR.