-
Notifications
You must be signed in to change notification settings - Fork 394
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
Enhancing Multi-speed Coil Speed Level Control Actuators #9185
Conversation
@xuanluo113 This is straightforward enough. If I'm following correctly, since the actuator is applied to the unitary system, there is no change in inputs for this, just doc and code changes to extend this to work with Coil:Cooling:DX. When updating the documentation, some reformatting would be helpful (the Unitary Equipment actuator section is a bit hard to navigate). |
The branch is ready for review with a new unit test and three test files. Results are illustrated in the overview. |
@mjwitte @Myoldmopar Mike and Edwin, this branch is ready for review, and I wonder if you could take a look. As for the documentation, I'm not sure what is a better place to add the EMS description other than the |
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.
Regarding the documentation, it's fine to include it with the unitarysystem actuators, but that section could use some subheadings or something to make it more readable.
this->m_SpeedNum = this->m_NumOfSpeedCooling; | ||
CycRatio = this->m_CoolingCycRatio = 1.0; | ||
PartLoadFrac = SpeedRatio = this->m_CoolingSpeedRatio = 1.0; | ||
this->m_CoilSpeedErrIdx++; |
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.
This index should not be incremented here. This is an index to this particular recurring error message. If it's zero ShowRecurringWarningErrorAtEnd
will set it to the next available index. There should also be a regular warning message issued the first time this occurs (i.e. when m_CoilSpeedErrIdx=0
) . See UnitarySystem.cc lines 9347ff for an example of how to use ShowRecurringWarningErrorAtEnd
.
src/EnergyPlus/UnitarySystem.cc
Outdated
} else if (this->m_CoolingSpeedNum == 1) { | ||
SpeedRatio = this->m_CoolingSpeedRatio = 0.0; | ||
CycRatio = this->m_CoolingCycRatio = this->m_EMSOverrideCoilSpeedNumValue - floor(this->m_EMSOverrideCoilSpeedNumValue); | ||
if (useMaxedSpeed || this->m_CoolingCycRatio == 0) { |
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.
Not sure this should be an else if
. Looks like useMaxedSpeed
will always be false here.
src/EnergyPlus/UnitarySystem.cc
Outdated
} else { | ||
CycRatio = this->m_CoolingCycRatio = 1.0; | ||
SpeedRatio = this->m_CoolingSpeedRatio = this->m_EMSOverrideCoilSpeedNumValue - floor(this->m_EMSOverrideCoilSpeedNumValue); |
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 if m_EMSOverrideCoilSpeedNumValue
is <= 0?
@mjwitte Hi Mike. I've fixed the recurring warning issues, and added some subsubsection headings in the doc. |
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.
This is better, but there is a failed unit test now.
Thanks for adding the subheadings in the unitary equipment EMS actuators section. Please add one more "Unitary System Sizing" before the paragraph starting with "Actuators are available for overriding the autosize values".
ShowContinueError(state, | ||
" Exceeding maximum coil speed level. Speed level is set to the maximum coil speed level allowed."); | ||
} | ||
ShowRecurringWarningErrorAtEnd( |
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.
This is better. I was going to comment that the two error messages here will conflict once the m_CoilSpeedErrIdx
is set. But now I see that ShowRecurringWarningErrorAtEnd
ignores the incoming index and searches for a matching error string. A test file which sets the coil speed to values higher than max peed, and to negative values, does produce both recurring error messages. So, this is fine as-is.
src/EnergyPlus/UnitarySystem.cc
Outdated
@@ -12281,6 +12292,35 @@ namespace UnitarySystems { | |||
} else if (this->m_DehumidificationMode == 1) { | |||
OperationMode = DataHVACGlobals::coilEnhancedMode; | |||
} | |||
|
|||
// if (this->m_EMSOverrideCoilSpeedNumOn) { |
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.
There's a block of commented code here that needs to be removed.
@@ -249,10 +249,12 @@ add_simulation_test(IDF_FILE EMSCustomOutputVariable.idf EPW_FILE USA_IL_Chicago | |||
add_simulation_test(IDF_FILE EMSCustomSchedule.idf EPW_FILE USA_IL_Chicago-OHare.Intl.AP.725300_TMY3.epw) | |||
add_simulation_test(IDF_FILE EMSDemandManager_LargeOffice.idf EPW_FILE USA_IL_Chicago-OHare.Intl.AP.725300_TMY3.epw) | |||
add_simulation_test(IDF_FILE EMSDiscreteAirSystemSizes.idf EPW_FILE USA_IL_Chicago-OHare.Intl.AP.725300_TMY3.epw) | |||
add_simulation_test(IDF_FILE EMSLoadBasedMultiSpeedDXCoilOverrideControl.idf EPW_FILE USA_IL_Chicago-OHare.Intl.AP.725300_TMY3.epw) |
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.
This new test file has some warnings:
************* ** Warning ** SimHVAC: Exceeding Maximum iterations for all HVAC loops, during JAN14 continues
************* ** ~~~ ** This error occurred 44 total times;
************* ** ~~~ ** during Warmup 0 times;
************* ** ~~~ ** during Sizing 0 times.
*************
************* ** Warning ** Wrong coil speed EMS override value, for unit="SYS 1 FURNACE DX COOL HEATING COIL". Exceeding maximum coil speed level. Speed level is set to the maximum coil speed level allowed.
************* ** ~~~ ** This error occurred 63 total times;
************* ** ~~~ ** during Warmup 54 times;
************* ** ~~~ ** during Sizing 0 times.
************* ** ~~~ ** Max=1. Min=1.
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.
@xuanluo113
Docs are good.
Unit tests pass.
Output variable warnings are gone (I just pushed up that change for the other idf).
Still troubled by these warnings in EMSLoadBasedMultiSpeedDXCoilOverrideControl:
************* ** Warning ** SimHVAC: Exceeding Maximum iterations for all HVAC loops, during JAN14 continues
************* ** ~~~ ** This error occurred 44 total times;
************* ** ~~~ ** during Warmup 0 times;
************* ** ~~~ ** during Sizing 0 times.
*************
************* ** Warning ** Wrong coil speed EMS override value, for unit="SYS 1 FURNACE DX COOL HEATING COIL". Exceeding maximum coil speed level. Speed level is set to the maximum coil speed level allowed.
************* ** ~~~ ** This error occurred 63 total times;
************* ** ~~~ ** during Warmup 54 times;
************* ** ~~~ ** during Sizing 0 times.
************* ** ~~~ ** Max=1. Min=1.
*************
************* ** Warning ** SimHVAC: Exceeding Maximum iterations for all HVAC loops, during JULY7 continues
************* ** ~~~ ** This error occurred 4 total times;
************* ** ~~~ ** during Warmup 0 times;
************* ** ~~~ ** during Sizing 0 times.
If there's not an obvious fix for these, please post a followup issue to investigate further.
Merging this. Will post a followup issue about the warnings in EMSLoadBasedMultiSpeedDXCoilOverrideControl. |
Enhancing Multi-speed Coil Speed Level Control Actuators
Pull request overview
Previously in the 9.6.0 release, we added the EMS actuators to control the speed level of the cooling and heating multispeed coils, namely
Coil:Cooling:DX:MultiSpeed
andCoil:Heating:DX:Multispeed
. As an enhancement, we propose to extend the EMS functionality to a more general coil type in EnergyPlus,Coil:Cooling:DX
, which will replace the single speed, two speed, variable speed, multi-speed, and multi-stage coils going forward.Result of the
testfiles/EMSLoadBasedMultiSpeedDXCoilOverrideControl.idf
Result of the
testfiles/EMSSetpointBasedMultiSpeedDXCoilOverrideControl.idf
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.