-
Notifications
You must be signed in to change notification settings - Fork 458
Update UnitarySystem set point control #10571
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
Update UnitarySystem set point control #10571
Conversation
|
This branch shows all coils now turn on and set point is maintained except for when the AvailabilityManger is turning off the system at night for a few hours and on a weekend. I'm not exactly sure yet where the change in air flow from cooling to heating is coming from. |
|
@rraustad @Myoldmopar it has been 28 days since this pull request was last updated. |
4 similar comments
|
@rraustad @Myoldmopar it has been 28 days since this pull request was last updated. |
|
@rraustad @Myoldmopar it has been 28 days since this pull request was last updated. |
|
@rraustad @Myoldmopar it has been 28 days since this pull request was last updated. |
|
@rraustad @Myoldmopar it has been 28 days since this pull request was last updated. |
|
@rraustad it has been 16 days since this pull request was last updated. |
|
@rraustad it has been 7 days since this pull request was last updated. |
2 similar comments
|
@rraustad it has been 7 days since this pull request was last updated. |
|
@rraustad it has been 7 days since this pull request was last updated. |
|
@rraustad it has been 8 days since this pull request was last updated. |
|
@rraustad it has been 7 days since this pull request was last updated. |
9 similar comments
|
@rraustad it has been 7 days since this pull request was last updated. |
|
@rraustad it has been 7 days since this pull request was last updated. |
|
@rraustad it has been 7 days since this pull request was last updated. |
|
@rraustad it has been 7 days since this pull request was last updated. |
|
@rraustad it has been 7 days since this pull request was last updated. |
|
@rraustad it has been 7 days since this pull request was last updated. |
|
@rraustad it has been 7 days since this pull request was last updated. |
|
@rraustad it has been 7 days since this pull request was last updated. |
|
@rraustad it has been 7 days since this pull request was last updated. |
|
@rraustad it has been 8 days since this pull request was last updated. |
|
@rraustad it has been 7 days since this pull request was last updated. |
4 similar comments
|
@rraustad it has been 7 days since this pull request was last updated. |
|
@rraustad it has been 7 days since this pull request was last updated. |
|
@rraustad it has been 7 days since this pull request was last updated. |
|
@rraustad it has been 7 days since this pull request was last updated. |
|
|
|
|
|
@mitchute I backed out all changes and started over. These are the minimum changes that get the system to at least turn on. I think I would rather have the system at least operate, albeit with some control issues, than not turn on at all. I will post a new issue to address the remaining issue which is out of scope and a user input control problem to either use the previous on air flow or off air flow (i.e. the m_AirFlowControl flag). This defect file is set point controlled which should honor this control choice. |
src/EnergyPlus/UnitarySystem.cc
Outdated
| // set variables used in fan call inside function calcPassiveSystem | ||
| state.dataUnitarySystems->m_massFlow1 = state.dataLoopNodes->Node(this->AirInNode).MassFlowRate; | ||
| state.dataUnitarySystems->m_runTimeFraction1 = (state.dataUnitarySystems->m_massFlow1 > 0.0) ? 1.0 : 0.0; // constant fan mode is required | ||
|
|
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.
If either of these changes are removed, the system air flow goes to 0 and there is no control to set point temperature. I'm not quite sure the initialization of LastMode should be done in getInput but I am warming up to it since the mode needs to start somewhere. If the mode is not set from 0 to cooling or heating mode the fan does not turn on. So this change seems OK. And if the fan operating parameters are not set, for set point controlled systems, then the fan won't turn on. So this also seems OK.
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.
@rraustad do you think this is captured in our tests already? Given your description, it seems like we should have some sort of test to ensure this continues to function.
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 was hard to track down. The defect file has Fan:SystemModel in a modified example file (UnitarySystem_VSHeatPumpWaterToAirEquationFit). Unit tests have legacy fans. One unit test has Fan:System models but doesn't test controls. The Fan:SystemModel needs to set these 2 variables for the fan call in calcPassiveSystem. This may have started with the fan refactor or a change to UnitarySystem that did not notice this corner case. I'm trying to figure out a good place for a unit test.
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.
First 2 unit tests passed fine without any changes. Then figured out that it was the coil type and the fan type that caused this issue and a specific unit test was added. That unit test fails without these changes. This coil type was calling calcPassiveSystem when it did not need to. Those lines were deleted. Since that function is no longer called the new lines to set the fan parameters were also not needed.
|
…70-UnitarySystem-VS-coil-SP-not-working
|
|
…70-UnitarySystem-VS-coil-SP-not-working
|
|
|
|
mitchute
left a comment
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.
Looking good here. Merging.



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.