-
Notifications
You must be signed in to change notification settings - Fork 389
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
Fix VRF FluidTCtrl negative outdoor unit fan power #10649
Open
yujiex
wants to merge
20
commits into
develop
Choose a base branch
from
fixVRFnegativeFanPower
base: develop
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 2 commits
Commits
Show all changes
20 commits
Select commit
Hold shift + click to select a range
7f2acaf
use abs(QCoil) in air flow calculation
ae3fec6
Merge remote-tracking branch 'origin/develop' into fixVRFnegativeFanP…
cabd9e6
revert abs(Q_coil), reduce h_IU_cond_in label230, fix Q_evap_req neg
765218b
Merge remote-tracking branch 'origin/develop' into fixVRFnegativeFanP…
35d4b69
add converge check that h_comp_out_new changed
0f1d4fd
Merge remote-tracking branch 'origin/develop' into fixVRFnegativeFanP…
935df9e
Revert "add converge check that h_comp_out_new changed"
38ad9f1
Patch for Q_cond_req < min speed power, CompSpdActual int to real
d54f356
clang-format
39567c8
fix unit test
aae3346
simplify the code by condition on sign of Q_evap_req
bd53fd7
Merge remote-tracking branch 'origin/develop' into fixVRFnegativeFanP…
4e979ea
use PLR in compressor speed calculation
1b58521
modify PLR application on the speed level > 1 case
cce66e0
Merge remote-tracking branch 'origin/develop' into fixVRFnegativeFanP…
a135044
fix unit test after changing compSpdActual calculation
835253a
remove unused Q_evap_req, remove C_cap_operation adjustment
436f277
fix unit test after removing C_cap_operation adjustment
a777bf5
fix uninitialized RatedHeatCapacity in VRF HR model
380d580
use newly derived r to compute compressor speed and Q_evap_req
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 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.
If I'm understanding
Q_coil
correctly, it should never be <0. While this fixes the issue (which occurs only a few timesteps in the year) it masks a problem higher up. The Q that is passed in to this function shouldn't be <0, ever.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.
So Q_coil is negative when it's in cooling mode. It is positive when it's heating
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.
I'm not sure that's the case.
Q_coil
here is the heat rejection when the unit is in cooling mode. So it's heating the outdoor air, so (Tcoilout-Tcoilin) should be positive, so you would expectQ_coil
to be positive.In the next block where it's in heating mode (so it's cooling the outdoor air) delta is
(h_coil_in - h_coil_out
so, again deltaH is positive and soQ_coil
should be positive.I ran in the debugger and saw both positive and negative values of
Q_coil
in theOperationMode == HXOpMode::CondMode
block. Tried to watch the values in theOperationMode == HXOpMode::EvapMode
block, but it never goes there. Even with Atlanta weather. That doesn't seem correct. Is it possible that the value passed in forOperationMode
is not correct 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.
Now I'm not certain whether
Q_coil
should be always positive or not.Although, from the definition of this function, it seems they do need to be absolute value of
Q_coil
.Maybe I should just add a
Q_coil = abs(Q_coil)
at the beginning of the call to this function? @mjwitteThere 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.
No, I think that will mask other problems. To me, the "absolute value" comment implies that the value passed in will always be positive. We need to understand if
HXOpMode
is being passed in correctly and better understand the variables that are being passed in forQ_coil
.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.
I see. I will look into it. Currently there are definitely both positive and negatives for the same mode
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.
I've done some investigations and had a few findings. Although I'm not sure if this is enough to figure out a fix. @mjwitte
about the operation mode
According to the documentation, there are 6 heat recovery modes
Here is how the heat recovery mode is determined
One issue I saw is that
rps2_cond
is negative sometimes. Here's some debugging print. I used theVariableRefrigerantFlow_FluidTCtrl_HR_5Zone.idf
andUSA_FL_Miami.Intl.AP.722020_TMY3.epw
combination. For the 01/02 07, 4th time step,rps2_cond
is negative.This is caused by the following in
VRFCondenserEquipment::VRFOU_CompSpd
, whereQ_req
is smaller than theCompEvaporatingPWRSpd(CounterCompSpdTemp)
whenCounterCompSpdTemp = 1
.I was considering, maybe
Q_evap_req = Q_cond_req - CompEvaporatingPWRSpd(CounterCompSpdTemp);
should beQ_evap_req = max(0.0, Q_cond_req - CompEvaporatingPWRSpd(CounterCompSpdTemp));
Even with this, there still remains a question of how this "much larger" or "much smaller" in mode 2 and 5 is reflected in the code. I feel the current code seems to be just testing for "larger" or "smaller".
about the
Q_coil
signthe input argument
Q_h_OU
inm_air_cond = this->VRFOU_FlowRate( state, HXOpMode::CondMode, Tdischarge, this->SC, Q_h_OU, state.dataEnvrn->OutDryBulbTemp, state.dataEnvrn->OutHumRat);
corresponds to theQ_coil
referred to in the last comment.Here is how it's computed. As
Q_c_tot + Ncomp
is smaller thanQ_h_TU_PL
, theQ_h_OU
is negative.Following is the debug print
For this specific instance, the
Q_h_TU_PL
is mostly on the piping loss term.Q_h_TU_PL = TU_HeatingLoad + Pipe_Q_h;
Maybe being in HR mode 2 should have ensured
Q_c_tot + Ncomp
much larger thanQ_h_TU_PL
? (asQ_c_tot
andNcomp
is the compressor performance at the compressor speedrps1_evap
that meets the cooling loadQ_c_TU_PL
). But currently it's not.about the high piping loss
There's also some strange behavior about the very large piping loss.
Piping loss is calculated here, in the iterations (label230 region), other input arguments stay the same, the
h_IU_cond_in
updates, leading to updates in piping loss (Pipe_Q_h
).The convergence for the label230 checks whether
h_comp_out
andh_comp_out_new
is close and whetherh_IU_cond_in
is maxed out. So before it's convergedh_IU_cond_in
keeps increasing till it reaches the upper boundh_IU_cond_in_up
.I've added some debug print after this line.
The following is the output for the iteration.
h_comp_out_new
stays the same whileh_comp_out
gets updated in functionVRFOU_PipeLossH()
. However, theh_comp_out
is not going in the right direction. It's much larger thanh_comp_out_new
at the beginning and keeps increasing each round tillh_IU_cond_in
is maxed out. Largeh_IU_cond_in
will lead to high piping loss. This potentially contributes to the fact thatQ_h_tot - Q_h_TU_PL
is negative (Q_h_TU_PL
gets too large as the piping loss term is large)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.
So during these iterations h_comp_out_new did not change but h_comp_out did change? That makes no sense. It feels like this iteration loop isn't updating something that SHOULD change in the h_comp_out_new equation otherwise the loop should terminate.
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.
Yeah, for the specific time step when the fan power became negative, this is what happens. Maybe that could be another criteria (checking whether h_comp_out_new changes) to add to the convergence check