-
Notifications
You must be signed in to change notification settings - Fork 457
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
Merged
mitchute
merged 14 commits into
develop
from
10570-UnitarySystem-VS-coil-SP-not-working
Oct 31, 2025
Merged
Changes from 9 commits
Commits
Show all changes
14 commits
Select commit
Hold shift + click to select a range
6f83135
Update UnitarySystem set point control
rraustad 5fe7768
Merge branch 'develop' of https://github.com/NREL/EnergyPlus into 105…
rraustad bdf39dd
Fix unit tests
rraustad 70c65fc
Merge branch 'develop' of https://github.com/NREL/EnergyPlus into 105…
rraustad 3e23136
Removing these doesn't change answer. LastMode should hold last opera…
rraustad fd52405
Revert as much code and still get defect file to run
rraustad 295e013
Merge branch 'develop' of https://github.com/NREL/EnergyPlus into 105…
rraustad 281ef2d
Back out everything to start over
rraustad fd4d74b
Bare minimum to get UnitarySystem to turn on
rraustad 4fc2af5
Add unit test, revert call to calcPassiveSystem in SP control function
rraustad 6898bd6
Merge branch 'develop' of https://github.com/NREL/EnergyPlus into 105…
rraustad d44fccb
clang format
dareumnam 1607d66
Merge branch 'develop' of https://github.com/NREL/EnergyPlus into 105…
dareumnam 92357b7
Merge branch 'develop' into 10570-UnitarySystem-VS-coil-SP-not-working
mitchute File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.
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.