Skip to content
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 documentation of VRF Heat Pump Total Heating Rate, should equal the sum of coil heating rate, not air terminal heating rate #10627

Open
wants to merge 17 commits into
base: develop
Choose a base branch
from

Conversation

yujiex
Copy link
Collaborator

@yujiex yujiex commented Jul 26, 2024

Pull request overview

Details on the fix of heat pump heating rate not matching total heating coil heating rate

The issue of heat pump heating rate not matching total heating coil heating rate, is caused by three factors

  • In function ControlVRFIUCoil, the capacity calculation method in FanSpdResidualHeat uses temperature difference (TotCap = FanSpdRto * Garate * 1005.0 * (Tout - Tin)) rather enthalpy difference (TotCap = FanSpdRto * RatedAirMassFlowRate * (PsyHFnTdbW(Tout, Wout) - PsyHFnTdbW(Tin, Win))). This is different from the coil heating rate calculation, causing some mismatches. So a function FanSpdResidualHeatUsingH is used to compute the capacity instead.
  • The piping correction value used in the coil calculation is different from the one used in heat pump heating rate calculation, causing some mismatches. So the piping loss adjustment to the coil heating rate is moved to the end of the VRF simulation to make sure the piping correction factor is up-to-date.
  • The heat pump capacity gets value here: TotalCondHeatingCapacity = this->HeatingCapacity;, its value is then passed to TotalTUHeatingCapacity and used in function LimitTUCapacity to compute an upper bound on coil heating output, when the flag FirstHVACIteration is true. So a check on this flag is added, to ensure that the update of this->HeatingCapacity syncs with the coil side.

before fixing

image
eplusout_develop.csv

after fixing
image
eplusout_feature.csv

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.

  • Title of PR should be user-synopsis style (clearly understandable in a standalone changelog context)
  • Label the PR with at least one of: Defect, Refactoring, NewFeature, Performance, and/or DoNoPublish
  • Pull requests that impact EnergyPlus code must also include unit tests to cover enhancement or defect repair
  • Author should provide a "walkthrough" of relevant code changes using a GitHub code review comment process
  • If any diffs are expected, author must demonstrate they are justified using plots and descriptions
  • If changes fix a defect, the fix should be demonstrated in plots and descriptions
  • If any defect files are updated to a more recent version, upload new versions here or on DevSupport
  • If IDD requires transition, transition source, rules, ExpandObjects, and IDFs must be updated, and add IDDChange label
  • If structural output changes, add to output rules file and add OutputChange label
  • If adding/removing any LaTeX docs or figures, update that document's CMakeLists file dependencies

Reviewer

This will not be exhaustively relevant to every PR.

  • Perform a Code Review on GitHub
  • If branch is behind develop, merge develop and build locally to check for side effects of the merge
  • If defect, verify by running develop branch and reproducing defect, then running PR and reproducing fix
  • If feature, test running new feature, try creative ways to break it
  • CI status: all green or justified
  • Check that performance is not impacted (CI Linux results include performance check)
  • Run Unit Test(s) locally
  • Check any new function arguments for performance impacts
  • Verify IDF naming conventions and styles, memos and notes and defaults
  • If new idf included, locally check the err file and other outputs

previously it says HP total heating rate is the sum of air terminal heating
rate (which includes supplemental heating coil heating rate). This is wrong for
both the curve-based model and FluidTCtrl model. FluidTCtrl model doesn't have
an entry to this output field, so one is added.
@yujiex yujiex added Defect Includes code to repair a defect in EnergyPlus Documentation Related primarily on the LaTeX-based EnergyPlus documentation labels Jul 26, 2024
@yujiex yujiex added this to the EnergyPlus 24.2 milestone Jul 26, 2024
@yujiex yujiex self-assigned this Jul 26, 2024
coil heating rate calculation is like the following
AirMassFlow * (OutletAirEnthalpy - InletAirEnthalpy) * PartLoadRatio

In the computation of fan speed ratio, the coil heating capacity is
FanSpdRto * Garate * 1005.0 * (Tout - Tin)

The difference between
1005.0 * (Tout - Tin) and (OutletAirEnthalpy - InletAirEnthalpy)
can cause mismatches between coil heating rate and heating demand

Here we change the fan speed ratio caculation to also use the enthalpy
difference way to compute its heating capacity to match the demand
@@ -619,7 +619,7 @@ \subsubsection{Outputs}\label{outputs-039}

\paragraph{VRF Heat Pump Total Heating Rate {[}W{]}}\label{vrf-heat-pump-total-heating-rate-w}

This output field is the operating total heating capacity of the variable refrigerant flow heat pump in Watts. The capacity includes any degradation due to defrost mode. This value is calculated for each HVAC system time step being simulated, and the results are averaged for the time step being reported. This value should match the sum of the individual zone terminal unit output variables for Zone VRF Air Terminal Total Heating Rate.
This output field is the operating total heating capacity of the variable refrigerant flow heat pump in Watts. The capacity includes any degradation due to defrost mode. This value is calculated for each HVAC system time step being simulated, and the results are averaged for the time step being reported. This value should match the sum of the individual zone terminal unit heating coil output variables for Heating Coil Heating Rate.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It thought this should say "sum of the individual zone terminal unit heating coil output variables for Heating Coil Heating Rate less any piping loss."

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might just be the total of heating coil heating rate rather than total coil heating rate - piping loss.

in CalcVRFCondenser, vrf.TotalHeatingCapacity (VRF Heat Pump Total Heating Rate output variable) is computed like this

vrf.TotalHeatingCapacity = TotalCondHeatingCapacity * HeatingPLR * CyclingRatio;

And in HeatingPLR is like this

if (TotalCondHeatingCapacity > 0.0) {
    HeatingPLR = min(1.0, (TUHeatingLoad / vrf.PipingCorrectionHeating) / TotalCondHeatingCapacity);
} else {
    HeatingPLR = 0.0;
}

For normal cases where TotalCondHeatingCapacity > 0.0,

vrf.TotalHeatingCapacity would just be

TotalCondHeatingCapacity * ((TUHeatingLoad / vrf.PipingCorrectionHeating) / TotalCondHeatingCapacity) * CyclingRatio 
= TUHeatingLoad / vrf.PipingCorrectionHeating * CyclingRatio

TUHeatingLoad / vrf.PipingCorrectionHeating would be coil heating rate plus the piping loss.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, I meant coil heating rates plus piping loss is the output from the outdoor unit.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see. Sorry I misunderstood earlier. I will modify the description.

@@ -1153,6 +1153,10 @@ \subsubsection{Outputs}

Note: refer to the rdd file after a simulation for exact output variable names

\paragraph{VRF Heat Pump Total Heating Rate {[}W{]}}\label{vrf-heat-pump-total-heating-rate-w}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs a unique label (there are duplicate label warnings when file changes are viewed in github).

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch. I'll add a suffix to it

@@ -17346,7 +17346,7 @@ void ControlVRFIUCoil(EnergyPlusData &state,
MaxSC = 20;
Garate = state.dataDXCoils->DXCoil(CoilIndex).RatedAirMassFlowRate(1);
// why always limit the minimum fan speed ratio to 0.65?
FanSpdRatioMin = min(max(OAMassFlow / Garate, 0.65), 1.0); // ensure that coil flow rate is higher than OA flow rate
FanSpdRatioMin = min(max(OAMassFlow / Garate, 0.0), 1.0); // ensure that coil flow rate is higher than OA flow rate
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems like a big change, unrelated to the heating rate outputs. Does this branch include some changes from another branch?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I might have left it in by accident. Just reverted it.

@yujiex
Copy link
Collaborator Author

yujiex commented Aug 26, 2024

@mjwitte @rraustad would you mind taking another look at this PR? Thanks. I was wondering whether the type of adjustment in HVACVariableRefrigerantFlow.cc look Okay.

if (std::abs(ZnSenLoad) < 100.0) ZnSenLoad = sign(100.0, ZnSenLoad);
Real64 Wout = Win;
Real64 TotCap = FanSpdRto * RatedAirMassFlowRate * (PsyHFnTdbW(Tout, Wout) - PsyHFnTdbW(Tin, Win));
return (TotCap - ZnSenLoad) / ZnSenLoad;
Copy link
Contributor

@rraustad rraustad Aug 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you really need to iterate on this? There's only 1 unknown.., FanSpdRto.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess not. I will change it to directly calculate FanSpdRto

Tout = Tin + (Ts_1 - Tin) * (1 - BF);
Real64 RatedAirMassFlowRate = state.dataDXCoils->DXCoil(CoilIndex).RatedAirMassFlowRate[0];
auto f = [QCoilSenHeatingLoad, RatedAirMassFlowRate, Tout, Tin, Win](Real64 FanSpdRto) {
return FanSpdResidualHeatUsingH(FanSpdRto, QCoilSenHeatingLoad, RatedAirMassFlowRate, Tout, Tin, Win);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not saying to change this but it just seems odd to me when meeting a load to modulate the fan based on suction temperature (which meets the load using air flow) instead of modulating the compressor at some known fan speed. I guess this is an artifact of using VS fan. I would hope in the case of a VS fan that the refrigerant suction T is relatively constant.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems that modulating the compressor is at the next step after the calculation of the TU's are finished. In this function, I don't think refrigerant suction temperature changes

}
if (this->TUHeatingLoad / this->PipingCorrectionHeating > TotalCondHeatingCapacity) {
this->TotalHeatingCapacity = this->HeatingCapacityPrev * HeatingPLR * this->PipingCorrectionHeatingPrev;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TotalHeatingCapacity should match the sum of the TU capacity + piping losses. So isn't it just that? TotalHeatingCapacity = Q_h_TU_PL = TU_HeatingLoad + Pipe_Q_h?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, it is matching this.

autosize, !- Cooling Outdoor Air Flow Rate {m3/s}
autosize, !- Heating Outdoor Air Flow Rate {m3/s}
0, !- Cooling Outdoor Air Flow Rate {m3/s}
0, !- Heating Outdoor Air Flow Rate {m3/s}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does including OA flow cause a difference between the sum of TU heating capacity + piping losses and condenser total heating capacity?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it does cause some more difference

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rraustad Here is the output file when the heating cooling air flow rate is autosize
eplusout_when heating cooling air flow rate autosize.xlsx

The following is a snapshot sorted with column O in descending order

image

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If L x M = 9083.83288 * 0.98793126 = 8974.2, which closely matches column K at 8973.19993, then why is the TU heating coil "allowed" to provide more than that "max" heating rate? Is the TU heating coil capacity getting limited by the MaxHeatingCapacity variable? I recall discussing that function LimitTUCapacity should include piping losses.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Coil capacity should have been limited. Maybe there's some lingering issues there. I will check on that.

Copy link
Collaborator Author

@yujiex yujiex Aug 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rraustad I've changed to just using TU_HeatingLoad for this->TotalHeatingCapacity. Now the total "Heating Coil Heating Rate" and the "VRF Heat Pump Total Heating Rate" equals (regardless of whether the "Heating/Cooling Outdoor Air Flow Rate" is autosize or 0). The following are the outputs for these two cases.

eplusout_0 heating cooling air flow.csv
eplusout_autosize heating cooling air flow.csv

The coil capacity is indeed limited. I set the OU evaporative capacity to 5000W (coil capacity is 10023W). Coil heating rate is less than OU capacity at max speed.

image

@dareumnam dareumnam self-requested a review August 28, 2024 17:39
Copy link

github-actions bot commented Sep 3, 2024

⚠️ Regressions detected on macos-14 for commit b9a481a

Regression Summary
  • EIO: 3
  • ESO Big Diffs: 3
  • Table Small Diffs: 1
  • MTR Small Diffs: 1
  • Table Big Diffs: 2
  • Table String Diffs: 1
  • ESO Small Diffs: 1

@dareumnam
Copy link
Collaborator

@rraustad @mjwitte Do you have any further comments on this? If this PR needs more time for discussion, we can move it to the 25.1 bug freeze milestone, as we're planning to tag the first RC for 24.2 tomorrow.

@@ -619,7 +619,7 @@ \subsubsection{Outputs}\label{outputs-039}

\paragraph{VRF Heat Pump Total Heating Rate {[}W{]}}\label{vrf-heat-pump-total-heating-rate-w}

This output field is the operating total heating capacity of the variable refrigerant flow heat pump in Watts. The capacity includes any degradation due to defrost mode. This value is calculated for each HVAC system time step being simulated, and the results are averaged for the time step being reported. This value should match the sum of the individual zone terminal unit output variables for Zone VRF Air Terminal Total Heating Rate.
This output field is the operating total heating capacity of the variable refrigerant flow heat pump in Watts. The capacity includes any degradation due to defrost mode. This value is calculated for each HVAC system time step being simulated, and the results are averaged for the time step being reported. This value should match the sum of the individual zone terminal unit heating coil output variables for Heating Coil Heating Rate plus any piping loss.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This does seem like the correct definition for the VRF heating capacity.

@@ -12219,7 +12221,8 @@ void VRFCondenserEquipment::CalcVRFCondenser_FluidTCtrl(EnergyPlusData &state)
}

this->TotalCoolingCapacity = TotalCondCoolingCapacity * CoolingPLR;
this->TotalHeatingCapacity = TotalCondHeatingCapacity * HeatingPLR;
// adjustment for matching HP heating rate and coil heating rate
this->TotalHeatingCapacity = TU_HeatingLoad;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not actually correct. The TU_HeatingLoad is the target (coil load + piping loss) that the model will iterate on to find a solution. The TotalHeatingCapacity is the result. With a tolerance on the iterations these 2 variables should not match. They should be very close (like within 0.001%), but not match. How far off TotalHeatingCapacity is from TU_HeatingLoad tells you how well the model did to calculate the final result. To equate these at the end will hide any issues with the model. TU_HeatingLoad should match exactly with the coil loads + piping loss. TotalHeatingCapacity should represent the refrigerant side heating capacity. Since this model has refrigerant properties could TotalHeatingCapacity = Mdot deltaH on the refrigerant side? Then you would know how well the model did to meet the load.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see.

I think the piping correction is not consistent in the IU and OU calculation, which might have partly caused the discrepancy. See comment here: #10625 (comment)

There might be other places contributing to it as well. I've been playing around with adjusting stuff but so far I've not been able to figure out how to effectively resolve the discrepancy.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe just revert this one change and revisit how different these 2 results are. TotalHeatingCapacity versus TU_HeatingLoad. Maybe also add another report for refrigerant side capacity?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will do that

@Myoldmopar
Copy link
Member

Thank you both for the comments this afternoon. I'm going to mark, for now, that this is not going into 24.2. If things get resolved and we agree, we can certainly go ahead and drop it in. But I'm going to change it to focus on other things.

@Myoldmopar Myoldmopar removed this from the EnergyPlus 24.2 milestone Sep 11, 2024
@Myoldmopar Myoldmopar added this to the EnergyPlus 25.1 milestone Sep 11, 2024
@yujiex
Copy link
Collaborator Author

yujiex commented Sep 11, 2024

Thanks, @Myoldmopar. I will investigate this more and hope to resolve it in the next round

@nrel-bot-2c
Copy link

@yujiex it has been 8 days since this pull request was last updated.

@nrel-bot-2c
Copy link

@yujiex it has been 7 days since this pull request was last updated.

1 similar comment
@nrel-bot-2c
Copy link

@yujiex it has been 7 days since this pull request was last updated.

Copy link

⚠️ Regressions detected on macos-14 for commit 4735f12

Regression Summary
  • EIO: 4
  • ERR: 2
  • ESO Big Diffs: 4
  • Table Big Diffs: 4
  • Table String Diffs: 4
  • MTR Big Diffs: 2

Copy link

⚠️ Regressions detected on macos-14 for commit 6c6c059

Regression Summary
  • EIO: 4
  • ERR: 1
  • ESO Big Diffs: 4
  • Table Big Diffs: 4
  • Table String Diffs: 4
  • MTR Big Diffs: 2

Copy link

⚠️ Regressions detected on macos-14 for commit 8ace59e

Regression Summary
  • EIO: 4
  • ESO Big Diffs: 4
  • Table Big Diffs: 3
  • MTR Small Diffs: 1
  • Table String Diffs: 2
  • Table Small Diffs: 1

@yujiex
Copy link
Collaborator Author

yujiex commented Oct 10, 2024

@mjwitte and @rraustad I have modified the code fixing the mismatch between the coil total heating rate and heat pump total heating rate. Some details are added in the PR description to explain the contributing factors to the issue and how they're addressed. Please see if they make sense.

I tried using Chicago weather, there's still noticeable difference between the heating coil heating rate and the heat pump heating rate. So there might be other things to change to make them agree. I'll look into it.

@nrel-bot-2c
Copy link

@yujiex @Myoldmopar it has been 29 days since this pull request was last updated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Defect Includes code to repair a defect in EnergyPlus Documentation Related primarily on the LaTeX-based EnergyPlus documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

VRF FluidTCtrl heat pump total heating rate doesn't match zone VRF air terminal total heating rate
10 participants