-
Notifications
You must be signed in to change notification settings - Fork 401
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 cooling supply fan power calculation when cycling #10341
Conversation
after multiplying cycling ratio to the outdoor unit, some super large COP appeared in some timesteps. This is probably due to the cycling ratio calculation, moving it to inside the compressor speed calculation fixed this problem.
This reverts commit 7d79eac.
@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. |
@mjwitte I have addressed the recent three comments above. I will look into the high changes in heating electricity later today. |
src/EnergyPlus/DXCoils.cc
Outdated
@@ -16880,7 +16880,7 @@ void CalcVRFCoolingCoil_FluidTCtrl(EnergyPlusData &state, | |||
} | |||
|
|||
// If cycling fan, send coil part-load fraction to on/off fan via HVACDataGlobals | |||
if (fanOp == HVAC::FanOp::Cycling) state.dataHVACGlobal->OnOffFanPartLoadFraction = PLF; | |||
if (fanOp == HVAC::FanOp::Cycling) state.dataHVACGlobal->OnOffFanPartLoadFraction = thisDXCoil.CoolingCoilRuntimeFraction; |
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 don't recall why this passes RTF now when it should pass PLF?
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.
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, you're passing the mass flow rate and RTF for speed 1 and then 0's for massflowrate2 and RTF2 to trick the fan model. But then the fan model also uses state.dataHVACGlobal->OnOffFanPartLoadFraction to calculate a local RTF. So don't change line 16883, leave it as PLF. Then in the call to the fan, pass the correct RTF. Actually, I don't think this is correct because thisDXCoil.CoolingCoilRuntimeFraction
will not be the same as _locRuntimeFrac
(that may not be a bad thing). I think you should pass PLF to the fan (using state.dataHVACGlobal->OnOffFanPartLoadFraction) and then let the fan calculate the _locRuntimeFrac to use for energy calculations, but I'm not actually sure since the coil PLF applies to the load and the fan PLF applies to the air flow. When the coil load and air flow are disconnected like they are in this VRF model, all bets are off as to what is happening.
Real64 _locRuntimeFrac = (state.dataHVACGlobal->OnOffFanPartLoadFraction >= 1.0)
? _localFlowFrac
: max(0.0, min(1.0, _localFlowFrac / state.dataHVACGlobal->OnOffFanPartLoadFraction));
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.
In the case where the fan is Continuous (the one corresponding to VAV fan in the VRF FluidTCtrl model), this is is how the _locRuntimeFrac is currently computed
if (_useFlowRatiosAndRunTimeFracs) {
_locFlowRatio = _localFlowRatio[mode];
_locRuntimeFrac = _localRuntimeFrac[mode];
} else {
_locFlowRatio = _localFlowFrac;
_locRuntimeFrac = 1.0;
}
and the _localRuntimeFrac
is calculated like this
if (present(_flowRatio1) && present(_flowRatio2) && present(_runTimeFrac1) && present(_runTimeFrac2)) {
_useFlowRatiosAndRunTimeFracs = true;
_localRuntimeFrac[0] = _runTimeFrac1;
_localRuntimeFrac[1] = _runTimeFrac2;
_localFlowRatio[0] = _flowRatio1;
_localAirMassFlow[0] = _localFlowRatio[0] * maxAirMassFlowRate * _localRuntimeFrac[0];
_localFlowRatio[1] = _flowRatio2;
_localAirMassFlow[1] = _localFlowRatio[1] * maxAirMassFlowRate * _localRuntimeFrac[1];
} else {
_localRuntimeFrac[0] = 1.0; // if runTimeFracs are not present, assume single-mode operation
_localRuntimeFrac[1] = 0.0; // if runTimeFracs are not present, assume single-mode operation
}
Should I change these to using the calculation like in the Discrete fan case like this?
Real64 _locRuntimeFrac = (state.dataHVACGlobal->OnOffFanPartLoadFraction >= 1.0)
? _localFlowFrac
: max(0.0, min(1.0, _localFlowFrac / state.dataHVACGlobal->OnOffFanPartLoadFraction));
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 would not change the fan model. I am just pointing out what I see that looks odd. PLF and RTF are an energy adjustment and I am trying to understand how coil PLR/PLF = RTF affect the fan energy. In other cycling coil/system models the fan air flow and coil load are dependent/proportional. In this model they are independent. I think just pass PLF over to the fan and leave it there for now.
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 I think this VRF coil doesn't have a "Part Load Fraction Correlation Curve Name" field.
PLF is computed like this
if (thisDXCoil.PLFFPLR(Mode) > 0 && CompCycRatio < 1.0) {
PLF = CurveValue(state, thisDXCoil.PLFFPLR(Mode), CompCycRatio); // Calculate part-load factor
} else {
PLF = 1.0;
}
if (PLF < 0.7) {
if (thisDXCoil.ErrIndex2 == 0) {
ShowWarningMessage(
state,
format(
"The PLF curve value for the DX cooling coil {} ={:.3R} for part-load ratio ={:.3R}", thisDXCoil.Name, PLF, PartLoadRatio));
ShowContinueErrorTimeStamp(state, "PLF curve values must be >= 0.7. PLF has been reset to 0.7 and simulation is continuing.");
ShowContinueError(state, "Check the IO reference manual for PLF curve guidance [Coil:Cooling:DX:SingleSpeed].");
}
ShowRecurringWarningErrorAtEnd(
state, thisDXCoil.Name + ", DX cooling coil PLF curve < 0.7 warning continues...", thisDXCoil.ErrIndex2, PLF, PLF);
PLF = 0.7;
}
So PLF will constantly be 1. But the coil do cycle. So I feel fan should not have PLF = 1
be passed into the system fan calculation all the time. Is this right? @rraustad
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.
there might be larger issue with the OU fan like negative VRF Heat Pump Outdoor Unit Fan Power value. This will be fixed in another PR
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.
With the outdoor fan adjustment removed, I'm good with this PR, but I have not reviewed the actual VRF code changes. Transition is good, and comparative results look good.
@dareumnam, I'm still confirming with Rich about the role of PLF on this fan. I will update the plot in a bit after I finalized it. |
Thanks for the discussions today. To summarize, the original issue was that the TU supply fan (Fan:VariableVolume) was set to run on cycling fan cycling coil mode, and we observed that when coil's runtime fraction is < 1, the fan power stays the same. I transitioned it to system model hoping it can do cycling as well. I achieved it by passing in the coil RTF as follows
According to @rraustad and @EnergyArchmage , the variable volume fan transitioned to the continuous mode Fan:System model should still run all the time when coil cycles. There should be a warning telling user that they should not use cycling fan mode when they have variable volume fan (Fan:System model with continuous mode). If this is the intention of how the system fan model works, then I guess the major contribution to the PR would be to throw a warning telling user that they shouldn't expect the fan to cycle because they used a wrong type of fan. If the intention is to allow a continuous system fan (transitioned from the variable volume fan) to also cycle on off when it's at lowest speed, then we can do it like the code chunk above? The reason I thought the continuous system fan is capable of cycling is that inside the
and the
|
@rraustad quick check here. Do you want to discuss this on a call before this proceeds? I'm inclined to one of two options:
|
Well, @yujiex has a good point. Passing in the RTF of the coil allows the VAV fan to cycle. E+ never allowed the VAV fan to cycle because it was intended to run the entire time step. If passing in RTF simulates a cycling VAV fan (i.e., a Fan:SystemModel set to continuous) then I think it's OK to allow that. We just did not allow this before because we didn't think outside the box. This will only affect VRF FluidTCtrl for now but allows a method to cycle a VAV fan (via the system fan model) for other parent equipment if needed. I think this is OK. |
I can't really form an opinion on this on a useful timescale. I pulled the branch but am bogged down trying to digest these changes to fan calling. |
Currently, I have reverted the part where the coil RTF is passed in. So right now, the continuous-mode system fan does not allow cycling at minimum fan power. Do we put the part where the fan cycling is allowed (by passing in the coil RTF) in a follow-up non-IO PR? Or if we decided we still don't allow that in the end, we'll put a warning or error message in that follow-up non-IO PR? |
@yujiex I am relying on the testing being done with VRF FluidTCtrl to determine what improvements are needed for that model. Constant and cycling fan can certainly be modeled now and if you think it's necessary to allow cycling of a Fan:SystemModel using continuous mode then that is acceptable as long as the results show proper results. There is no need to add that functionality back in to this branch. |
@rraustad I see. If we decided to allow continuous Fan:SystemModel to cycle, I'll do it in another PR. This current PR will just be the transition part. |
I just pulled develop. If there's not major unexpected regression diffs, would this PR be good to go as is? |
OK, this doesn't appear to affect anything outside of the expected few files. I'm inclined to merge. If anyone wants to hold this up, speak up soon. Otherwise this will go in with follow-ups as needed. |
Alright, tested fine, let the merging continue. Thanks @yujiex for this set of changes. I know there may still be follow-up work to do. @dareumnam, thanks for helping keep me updated on this PR. And @mjwitte and @rraustad thanks for looking this over! Merging! |
Thanks @rraustad @mjwitte @Myoldmopar @dareumnam for reviewing. I'll work on the followup PR and hope I can resolve that part soon. |
Pull request overview
The fan issue in the cooling part is fixed by multiplying the runtime fraction of the cooling coil (which also equals the VRF condenser cycling ratio).
This PR transitions the variable volume fan model used in the Supply Air Fan Object Type of ZoneHVAC:TerminalUnit:VariableRefrigerantFlow of a VRF FluidTCtrl system into a equivalent Fan:SystemModel with "Speed Control Method" being "continuous".
Allowing cycling of the Fan:SystemModel with continuous speed control method will be done in another follow-up PR.
Regression diff
Regression diffs appear in the following four files
eplusout.csv diffs
In VariableRefrigerantFlow_FluidTCtrl_5Zone, the idf file has the following difference due to the fan type transitioning.
the relative difference in eplusout.csv that are larger than 1e-12 is as follows.
VRF Heat Pump Cooling Electricity Rate and Energy diffs are due to changes in outdoor fan power. Similarly, VRF Heat Pump Heating Electricity Rate and Energy diffs are also caused by outdoor fan power diffs. These are expected as the outdoor fan power is changed in this feature to account for system cycling. Cooling:Electricity is affected by VRF Heat Pump Cooling/Heating Electricity, so the diffs are also expected
meter difference
The following shows the percent meter difference summary in VariableRefrigerantFlow_FluidTCtrl_5Zone. The differences are mainly in electricity outputs where fan electricity is part of (cooling, HVAC, and facility electricity rate)
table diffs
Similar to the meter differences, the diffs happen in fan electricity and related outputs where fan electricity is part of.
The following is the table diffs in VariableRefrigerantFlow_FluidTCtrl_5Zone. The default fields and autosized fields diffs are caused by converting from Fan:VariableVolume to Fan:System model. 5 more curve objects are added as a result of the conversion. The number of autosized fields increased as the design electric power is also autosized in addition to max air flow rate.
audit diff
in VariableRefrigerantFlow_FluidTCtrl_5Zone, the audit diff is as follows. The numCompSizeTableEntry diffs are because the Fan:SystemModel has two autosized fields: Max air flow rate and design electric power, while the Fan:VariableVolume only have one autosized entry: max air flow rate.
bnd diff
in VariableRefrigerantFlow_FluidTCtrl_5Zone, the bnd diff is as follows. The diffs are due to the Fan:VariableVolume to Fan:SystemModel.
eio diff
in VariableRefrigerantFlow_FluidTCtrl_5Zone, the eio diff is as follows. The eio diffs are mainly due to the difference in the autosized fields. There are also some very small warmup convergence differences.
...
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.