-
Notifications
You must be signed in to change notification settings - Fork 378
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
Addressed CoolingTower:VariableSpeed control problems #7702
Merged
Merged
Changes from all commits
Commits
Show all changes
14 commits
Select commit
Hold shift + click to select a range
c9c67ea
addresses VS tower control issue
6644efa
Merge branch 'develop' into 170845444_Issue7464
554312b
Merge branch 'develop' into 170845444_Issue7464
4a86338
added unit test
4a3254a
Merge branch 'develop' into 170845444_Issue7464
65ad1c2
Modified unit test and did cleanup.
4bd3499
modified the fix
c6f93ee
Merge branch 'develop' into 170845444_Issue7464
c6d5cc4
Updated I/O Ref
0edb64d
Merge branch 'develop' into 170845444_Issue7464
c9d665d
Resolved Develop Merge In Conflict
595f4f2
Minor documentation change
a7b1850
Merge branch 'develop' into 170845444_Issue7464
3d848e7
relaxed VS Cooling Tower range max value
File filter
Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
There are no files selected for viewing
This file contains 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
This file contains 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
Oops, something went wrong.
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.
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.
@Nigusse Is this overriding the range for all tower types?
Rather than overriding this here, why not set
towers(this->VSTower).MaxRangeTemp
up inGetTowerInput
, here.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.
Yes, the 22.2222 is used for all variable speed cooling towers. I can set it but the default value is used elsewhere to check if the actual tower range is outside the default range. I am concerned to change any curve values that are related the data used to generate the coefficients. And one reason that I chose 22.2222 is that it is the same as the VS York CT so that it does not affect the York Model; otherwise, I experimented with a value higher than that. If you still insist, I will make the changes per your suggestion.
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.
@mjwitte Below is alternative suggestion for your consideration. First, this change impacts the toolcool VS cooling tower model only; second, this model will be able to check and throw warning if the requested tower range is outside the VS cooltool existing (default) tower range maximum limit.
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.
Nah, let's leave it like it is. Please push up the doc changes and we'll call this complete.
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.
Done. Thanks!