-
Notifications
You must be signed in to change notification settings - Fork 377
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
Fix autosize bug in variable speed cooling coils. #10503
base: develop
Are you sure you want to change the base?
Conversation
src/EnergyPlus/VariableSpeedCoils.cc
Outdated
ShowContinueError(state, format("...{} cannot be < 0.0.", cNumericFields(10))); | ||
ShowContinueError(state, format("...entered value=[{:.2T}].", NumArray(10))); | ||
ErrorsFound = true; | ||
if (state.dataVariableSpeedCoils->VarSpeedCoil(DXCoilNum).EvapCondPumpElecNomPower != Constant::AutoCalculate) { |
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.
The Constant::AutoSize and Constant::AutoCalculate do have the same value but they mean different things. We should probably change one of the values. The auto calculate field is defined with \autocalculatable
. So this field, which is autosizable, should compare to the AutoSize variable.
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.
OK, this is correct and the idd shows autosize when it should be autocalculatable. You know an input is auto calculated when there is a rule of thumb used to size the input.
// Auto size high speed evap condenser pump power to Total Capacity * 0.004266 w/w (15 w/ton)
EvapCondPumpElecNomPowerDes = varSpeedCoil.RatedCapCoolTotal * 0.004266;
LatentLoad, | ||
OnOffAirFlowRatio); | ||
} | ||
|
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.
What is the value of Evaporative Condenser Pump Rated Power Consumption after this call?
@tanaya-mankad are there other changes you expect to make? These changes look fine for the issue. |
Yes, this is the (greatly simplified) set of changes. Thanks. |
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.
These changes look good to me.
@tanaya-mankad, this has a conflict in the unit test code. It's otherwise ready, so if you could resolve that, I'll give it a quick once over and get it merged in. My guess is that two different unit tests were added to the same spot in the unit test file. It's either that, or one of the namespace-ish refactors just changed a line. If you hit trouble, let me know and I can give it a shot. |
Hi @Myoldmopar , I'm not finding the failure in the Dashboard. Could you point me to a link? |
It's a merge conflict here on the pr, I'm not sure I can send a link to it 🙃 |
Pull request overview
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.