-
Notifications
You must be signed in to change notification settings - Fork 389
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
Code Restructuring in ChillerHeater Model (intermediate step) #10601
Conversation
Created a new subroutine that avoids duplicate code and simplifies slightly the ChillerHeater subroutine.
Forgot the clang formatting for these changes.
Next step: more simplification of the code, this time dealing with the handling of the condenser water temperature. Decision now based on enum and handled by a Real64 function.
More restructuring for the ChillerHeater module, this time the calculation of CapFT and error checks are made into a stand-alone Real64 function.
Created a new routine that is used by both the chiller and chillerheater in this model. This is again to improve clarity in the routines and prepare for the changes that will be needed to solve the actual defect which is that the evaporator and compressor energy never get updated when heating is the predominant mode (modes 2 and 5).
Moved repetitive code from the chiller and heater models of the chiller-heater component. This code focuses on the calculation of part load ratio and the cycling fraction (when PLR is below the minimum).
As expected, CI is completely clean, this is great @RKStrand . I'll look over the refactors. |
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.
Looks like a nice preliminary cleanup pass. Breaking the functionality into small worker methods is a great way to identify responsibility, and eventually add clear, targeted, testing.
setChillerHeaterCondTemp = condLeavingTemp; | ||
} | ||
return setChillerHeaterCondTemp; | ||
} |
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.
Could simplify this further...
if (this->ChillerHeater(numChillerHeater).CondMode == CondenserModeTemperature::EnteringCondenser) {
return condEnteringTemp;
} else { // by default, if not EnteringCondenser, then this can only be LeavingCondenser
return condLeavingTemp;
}
or further...
if (this->ChillerHeater(numChillerHeater).CondMode == CondenserModeTemperature::EnteringCondenser) {
return condEnteringTemp;
}
return condLeavingTemp;
And because I think @amirroth enjoys ternaries, I suppose you could also
return this->ChillerHeater(numChillerHeater).CondMode == CondenserModeTemperature::EnteringCondenser ? condEnteringTemp : condLeavingTemp;
CI is all clean, code cleanup looks good, no reason to hold this up. I'll merge this and you can immediately carry on to the functional changes which may cause diffs, etc. Good luck! |
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.