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.
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
There was a problem hiding this comment.
Oops.
What do you think of GetUserDefinedAirComponent, or GetUserDefinedZoneAirComponent?
"There are only two hard things in Computer Science: cache invalidation and naming things."
-- Phil Karlton
| 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.
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.
We really do hurt ourselves by the choice of our example files. This is a good fix. So would be init_state.
|
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. |
|
This is looking ready. We'll let CI finish then merge this. |
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