Correction to AirflowNetwork:Multizone:Surface Handling of People Index #11167
Conversation
The code for AFN was not setting the zone number properly for surfaces within the AFN. Code existed for zone input to do this and link to a people statement for ASHRAE55 and CEN15251 adaptive modeling. The correction included getting all zone and surfaces to use the same (new) subroutine, simplifying the code within the main get routine and relegating the details of linking surfaces, zones, and people statements to the new subroutine.
Slight update to the new subroutine plus the addition of a unit test and a small clarification for the IO Reference section on AFN.
mjwitte
left a comment
There was a problem hiding this comment.
This works as-is, but the new function could be much simpler, I think.
| " requires a people object with respective model calculations."); | ||
| } | ||
| int mzZoneDataPtr; | ||
| Solver::get_people_index(MultizoneZoneData(i).CEN15251PeopleInd, mzZoneDataPtr, VentControlType::CEN15251, false, i, ErrorsFound); |
There was a problem hiding this comment.
This works, but I'm puzzled why there are some many function parameters. I would expect this to be an int function:
MultizoneZoneData(i).CEN15251PeopleInd = Solver::get_people_index( MultizoneZoneData(i).ZoneNum, VentControlType::CEN15251, ErrorsFound);
| case VentControlType::ASH55: | ||
| case VentControlType::CEN15251: | ||
| case VentControlType::CEN15251: { | ||
| int peopleIndex; |
There was a problem hiding this comment.
int mzPtr = MultizoneSurfaceData(i).ZonePtr
MultizoneZoneData(mzPtr).CEN15251PeopleInd = Solver::get_people_index(mzPtr , VentControlType::CEN15251, ErrorsFound);
With a similar block for case VentControlType::ASH55
There was a problem hiding this comment.
I see your point. There is a lot more complication than seems warranted. The zone pointer should be available already so sending that should avoid having to have the first block of code. I have often avoided setting one of the passed variables to a function like this one and made those general subroutines, but I guess there is nothing wrong with making this a function and still setting the error flag that is passed. I'll implement those changes and then mark this for re-review when I'm done. Thanks!
There was a problem hiding this comment.
@mjwitte Ok--I have reworked this so that it's a function and the function is much simpler/leaner. Just made the commit of code that still does what it needs to do. Also modified the unit test accordingly. Hopefully this addresses your concern.
Simplified solution by making the subroutine a function and narrowing down what the function does.
One more simplification/correction.
Pull request overview
Description of the purpose of this PR
For situations where a user specified AirflowNetwork:Multizone:Surface objects with CEN15251 (or ASH55) venting controls but the AirflowNetwork:Multizone:Zone object used some other venting control, no link was made to the correct people statement. As a result, no venting was ever done for this situation which was incorrect. Now, the code links up the proper zone and then links up to the correct People statement and venting occurs as specified by the user via a new subroutine which handles both ASH55 and CEN15251 for both the :Surface and :Zone objects. Two IDFs were included with the defect. One did not work because it met the condition shown above. The second had the :Zone object set to CEN15251 which then did allow venting. There were clear differences in the outputs. After the fix, both files produce exactly the same results as expected and these match the results of the second input file with :Zone set to CEN15251. No differences were noted in the ci results as anticipated. A minor documentation edit and a unit test are included.
Pull Request Author
Reviewer