-
Notifications
You must be signed in to change notification settings - Fork 396
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
Correction of Generic Contaminant Handling in Some Situations #10516
Conversation
There were a few places where GenContam was not implemented properly in the code. After a thorough search of the code for both CO2 and GenContam, it was discovered that there were a couple of places where the code for these two entities were not similar (GenContam code that needed to be present was missing). Also, an incorrect assignment of the simulation flag was also discovered. This commit fixes those issues.
Addition of a new unit test and enhancement of an existing one for this fix.
To const or to constexpr, that is the question...
src/EnergyPlus/DataZoneEquipment.cc
Outdated
sumCO2 += spaceOutletNode.CO2 * mixerSpace.fraction; | ||
sumGenContam += spaceOutletNode.GenContam * mixerSpace.fraction; |
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.
Most (all?) other places that work with CO2 and GenContam have ifs like if (state.dataContaminantBalance->Contaminant.GenericContamSimulation) {
. Should there be ifs here?
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.
@mjwitte hmmm…that’s a good point. I will look at implementing such if checks when I get back to this next week on Tuesday.
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.
@mjwitte Ok, I've added the IF checks as per your suggestion and just now made a commit (as well as merged in develop to get everything back up to speed). Anticipating that this should come back all green as before.
if (state.dataContaminantBalance->Contaminant.GenericContamSimulation) { | ||
state.dataLoopNodes->Node(AirOutletNode).GenContam = state.dataLoopNodes->Node(AirInletNode).GenContam; | ||
} |
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.
@Myoldmopar There are ifs like this scattered throughout the code. For simple assignments like this are these ifs saving any time or are they just clutter?
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 agree these checks aren't needed (just pass the 0). Also thinking of a safeAirNodeCopy function passing MassFlowRate, Quality, Press, MassFlowRateMin, MassFlowRateMax, MassFlowRateMinAvail and MassFlowRateMaxAvail, and CO2 and GenContam. A lot of these child update functions would reduce to:
safeAirNodeCopy(state, outletnode, inletnode);
state.dataLoopNodes->Node(AirOutletNode).Temp = varSpeedCoil.OutletAirDBTemp;
state.dataLoopNodes->Node(AirOutletNode).HumRat = varSpeedCoil.OutletAirHumRat;
state.dataLoopNodes->Node(AirOutletNode).Enthalpy = varSpeedCoil.OutletAirEnthalpy;
To conform to the pattern established in other parts of the code, IF statements were needed in the setOutletConditions routine of the ZoneEquipmentMixer. This also then required the slight modification of a unit test. This was in response to a comment from @mjwitte.
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.
Code changes look good. Unit tests pass. CI is clean and green. Merging. Thanks @RKStrand !
Pull request overview
NOTE: ENHANCEMENTS MUST FOLLOW A SUBMISSION PROCESS INCLUDING A FEATURE PROPOSAL AND DESIGN DOCUMENT PRIOR TO SUBMITTING CODE
Pull Request Author
Add to this list or remove from it as applicable. This is a simple templated set of guidelines.
Reviewer
This will not be exhaustively relevant to every PR.