-
Notifications
You must be signed in to change notification settings - Fork 448
Fix AirTerminal:SingleDuct:UserDefined crash (accessing uninitialized but unneeded AirDistUnit array) #11223
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
base: develop
Are you sure you want to change the base?
Conversation
// one assumes if there isn't one assigned, it's an error? | ||
if (state.dataUserDefinedComponents->UserAirTerminal(CompLoop).ADUNum == 0) { | ||
ShowSevereError(state, | ||
format("GetUserDefinedComponents: No matching Air Distribution Unit for {} = {}", |
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.
I hate to do this but you renamed this function to GetUserDefinedAirTerminal. And yes, this is an error , the AirTerminal is specified in an ADU. That's the only place this object is listed as allowed. So this AirTerminal must not be listed in an ADU and therefore is an orphan object.
ZoneHVAC:AirDistributionUnit,
\memo Central air system air distribution unit, serves as a wrapper for a specific type of
\memo air terminal unit. This object is referenced in a ZoneHVAC:EquipmentList.
\min-fields 4
A1 , \field Name
\required-field
\reference ZoneEquipmentNames
A2 , \field Air Distribution Unit Outlet Node Name
\required-field
\type node
A3 , \field Air Terminal Object Type
\type choice
\key AirTerminal:DualDuct:ConstantVolume
\key AirTerminal:DualDuct:VAV
\key AirTerminal:SingleDuct:ConstantVolume:Reheat
\key AirTerminal:SingleDuct:ConstantVolume:NoReheat
\key AirTerminal:SingleDuct:ConstantVolume:FourPipeBeam
\key AirTerminal:SingleDuct:VAV:Reheat
\key AirTerminal:SingleDuct:VAV:NoReheat
\key AirTerminal:SingleDuct:SeriesPIU:Reheat
\key AirTerminal:SingleDuct:ParallelPIU:Reheat
\key AirTerminal:SingleDuct:ConstantVolume:FourPipeInduction
\key AirTerminal:SingleDuct:VAV:Reheat:VariableSpeedFan
\key AirTerminal:SingleDuct:VAV:HeatAndCool:Reheat
\key AirTerminal:SingleDuct:VAV:HeatAndCool:NoReheat
\key AirTerminal:SingleDuct:ConstantVolume:CooledBeam
\key AirTerminal:DualDuct:VAV:OutdoorAir
\key AirTerminal:SingleDuct:UserDefined
\key AirTerminal:SingleDuct:Mixer
\required-field
state.dataUserDefinedComponents->UserAirTerminal(CompLoop).ADUNum = ADUNum; | ||
if (state.dataUserDefinedComponents->NumUserAirTerminals > 0) { // Skip this code if the only User Defined type is ZoneHVAC | ||
int ADUNum = 0; | ||
for (ADUNum = 1; ADUNum <= (int)state.dataDefineEquipment->AirDistUnit.size(); ++ADUNum) { |
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.
I wasn't going to say anything since this was existing but you might as well fix this one while your in here, for( int ADUNum
, line 2113 isn't needed.
if (state.dataUserDefinedComponents->GetPlantCompInput) { | ||
GetUserDefinedPlantComponents(state); | ||
state.dataUserDefinedComponents->GetPlantCompInput = false; | ||
} |
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.
We really do hurt ourselves by the choice of our example files. This is a good fix. So would be init_state.
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.
@mitchute this looks good to go with a simple change.
Thanks @rraustad. I'll kick it back to @tanaya-mankad for now, but feel free to ping me when ready and we'll get it in. |
Pull request overview
Description of the purpose of this PR
Two user-defined types were being initialized in the same function, GetUserDefinedComponents. This conflation led to AirTerminal objects having ZoneHVAC objects' needs being applied to them, when their members hadn't yet been sized. A solution was to pull the "Get" methods into two separate methods, fixing a copy-paste typo along the way. Also, since so many of the objects in UserDefinedComponents are wholesale duplication of code, we could consolidate a bit using a base struct. (However, most of the duplication has been left alone.)
The issue seems to have been exposed due to a test file containing both types of user-defined air objects. Existing IDF test files that only contain one or the other seem to simulate normally.
Pull Request Author
Reviewer