Followup VRF warning about solveRoot of Te at small load condition #11000
Conversation
|
| } else { | ||
| SmallLoadTe = MinOutdoorUnitTe; | ||
| } | ||
| T_suction + 6); // SmallLoadTe is the updated Te' |
There was a problem hiding this comment.
- Why is this SolveRoot call, lines 13960 and 13961, still here? It doesn't do anything.
- If you are going to execute line 13968 anyway, then you may as well make this an
if () { } else { }to eliminate the extra equivalence. - I would still like to see a warning for SolFla == -2 and let the code coverage tell us it's never used than to possibly miss a coding mistake. At the very least put an assert there so a developer might stumble across an issue.
- What happened to the recurring warning?
There was a problem hiding this comment.
@rraustad Thanks for the comments.
for 1. I think might have forgot to change it back. Now the upper bound is changed back to T_suction.
2. I changed it to if else logic
3. I added back the warnings for the SolFla = -2 case
4. Also added back recurring warnings in the SolFla = -2
| int LowLoadTeError2PosTsuc = 0; | ||
| int LowLoadTeError2PosTsucIndex = 0; // warning message index | ||
| int LowLoadTeError2PosOUTe = 0; | ||
| int LowLoadTeError2PosOUTeIndex = 0; // warning message index |
There was a problem hiding this comment.
Are you planning to use these?
There was a problem hiding this comment.
yes, added back the warnings
|
|
|
@Myoldmopar @yujiex @Myoldmopar it has been 29 days since this pull request was last updated. |
|
@Myoldmopar @yujiex @Myoldmopar it has been 35 days since this pull request was last updated. |
|
This reverts commit 3399351.
|
|
|
|
@Myoldmopar @yujiex @Myoldmopar it has been 28 days since this pull request was last updated. |
|
|
||
| General::SolveRoot(state, 1.0e-3, MaxIter, SolFla, SmallLoadTe, f, MinOutdoorUnitTe, | ||
| T_suction); // SmallLoadTe is the updated Te' | ||
| if (SolFla == -1) { |
There was a problem hiding this comment.
Line 14219 was moved to new line 14232, right? If so, the call to SolveRoot at 14219 can be deleted.
| SmallLoadTe); | ||
| } else { | ||
| // demand > capacity at both endpoints of the Te range, take the end point x where f(x) is closer to zero | ||
| if (f_xmin > f_xmax) { // f(T_suction > 0, not equal as SolFla will not be -2 |
There was a problem hiding this comment.
Is this comment SolFla will not be -2 correct? because this code is in the } else if (SolFla == -2) { block.
|
@yujiex this is about ready. Just take a look at these last comments. |
|
@yujiex @Myoldmopar it has been 28 days since this pull request was last updated. |
1 similar comment
|
@yujiex @Myoldmopar it has been 28 days since this pull request was last updated. |
Pull request overview
Description of the purpose of this PR
Pull Request Author
Reviewer