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

Update VRF min/max outdoor unit inlet temp field description #8691

Merged
merged 11 commits into from
Jun 22, 2021

Conversation

rraustad
Copy link
Contributor

@rraustad rraustad commented Apr 2, 2021

Pull request overview

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

@rraustad rraustad added the Documentation Related primarily on the LaTeX-based EnergyPlus documentation label Apr 2, 2021
@@ -74,13 +74,13 @@ \subsubsection{Inputs}\label{inputs-051}

This numeric field defines the cooling coefficient of performance at rated conditions. The cooling coefficient of performance includes compressor power and condenser fan power. This COP value does not account for impacts due to the supply air fan. The nominal heat pump cooling COP must be greater than 0. If this field is left blank, a default coefficient of performance of 3.3 is assumed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

A user was confused on the use and interpretation of the next few input fields. A water-cooled VRF will not use the outdoor air temp to determine operation. Generalize the field name and describe in text.

Copy link
Contributor

Choose a reason for hiding this comment

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

Both coefficient of performance and COP are used. There is no connection between them. I prefer to use "cooling coefficient of performance (COP) in the first sentence and use COP to replace rest of coefficient of performance.

EXPECT_NEAR(state->dataLoopNodes->Node(thisTU.VRFTUOutletNodeNum).Temp, thisTU.coilTempSetPoint, 0.01);
EXPECT_NEAR(state->dataLoopNodes->Node(thisTU.VRFTUOutletNodeNum).Temp, 20.0, 0.01); // TU outlet is at set point = 20
EXPECT_NEAR(state->dataLoopNodes->Node(thisTU.VRFTUOutletNodeNum).Temp, 20.0, 0.01);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Add to existing unit test a check on min/max operating limits

@rraustad
Copy link
Contributor Author

rraustad commented Apr 2, 2021

Do example file field names need to be updated? Or is there a script for that?

@rraustad
Copy link
Contributor Author

rraustad commented Apr 2, 2021

@Myoldmopar I think I nailed it on the "Labels" description yet I got tagged on the PR labeling.

@@ -74,13 +74,13 @@ \subsubsection{Inputs}\label{inputs-051}

This numeric field defines the cooling coefficient of performance at rated conditions. The cooling coefficient of performance includes compressor power and condenser fan power. This COP value does not account for impacts due to the supply air fan. The nominal heat pump cooling COP must be greater than 0. If this field is left blank, a default coefficient of performance of 3.3 is assumed.

Copy link
Contributor

Choose a reason for hiding this comment

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

Both coefficient of performance and COP are used. There is no connection between them. I prefer to use "cooling coefficient of performance (COP) in the first sentence and use COP to replace rest of coefficient of performance.


This numeric field defines the minimum source temperature allowed for cooling operation. For air-cooled equipment outdoor dry-bulb temperature is used. For water-cooled equipment inlet water temperature is used. Below this temperature, cooling is disabled. If this field is left blank, the default value is -6ºC.
This numeric field defines the minimum source inlet temperature allowed for cooling operation. For air-cooled equipment outdoor dry-bulb temperature is used. For water-cooled equipment inlet water temperature is used. Below this temperature, cooling is disabled. If this field is left blank, the default value is -6ºC.
Copy link
Contributor

Choose a reason for hiding this comment

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

My understanding is that this field provides a minimum temperature to disable cooling operation. It can be either air for air cooled equipment, or water for water-cooled equipment. I prefer to remove "Outdoor" in the field name, because it can be water temperature.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, this is a minimum temperature to disable cooling/heating operation, but a minimum temperature of what/where? So my choice was to use "Minimum/Maximum Outdoor Unit Inlet Temperature in Cooling/Heating Mode" or "Minimum/Maximum Condenser Inlet Temperature in Cooling/Heating Mode", however, in heating mode the outdoor unit is actually the evaporator so I tried to be very generic using Minimum/Maximum Outdoor Unit Inlet Temperature in Cooling/Heating Mode. I guess I could change this to "Minimum/Maximum Unit Inlet Temperature in Cooling/Heating Mode" but that doesn't seem as descriptive.

Let's decide what this name should be before I change this.

Choices:

  • Minimum/Maximum Outdoor Unit Inlet Temperature in Cooling/Heating Mode
  • Minimum/Maximum Condenser Inlet Temperature in Cooling/Heating Mode
  • Minimum/Maximum Source Inlet Temperature in Cooling/Heating Mode
  • Minimum/Maximum Unit Inlet Temperature in Cooling/Heating Mode
  • Minimum/Maximum Inlet Temperature in Cooling/Heating Mode
  • Minimum/Maximum Temperature in Cooling/Heating Mode

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a field in the AirConditioner:VariableRefrigerantFlow object so I guess it's implied to be a temperature in that location. I guess choice 6 seems reasonable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mjwitte do you have a preference for this field name? Or other suggestion?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am going with the last option, Minimum/Maximum Temperature in Cooling/Heating Mode.

Copy link
Contributor

Choose a reason for hiding this comment

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

Well, given that the node field name is A35, \field Condenser Inlet Node Name, it seems that Min/Max Condenser Inlet Node Temp ... would be the most consistent. If you want to rename this node, that's fine, but it seems these temperature fields should use the same naming.


This numeric field defines the maximum source temperature allowed for cooling operation. For air-cooled equipment outdoor dry-bulb temperature is used. For water-cooled equipment inlet water temperature is used. Above this temperature, cooling is disabled. If this field is left blank, the default value is 43ºC.
This numeric field defines the maximum source inlet temperature allowed for cooling operation. For air-cooled equipment outdoor dry-bulb temperature is used. For water-cooled equipment inlet water temperature is used. Above this temperature, cooling is disabled. If this field is left blank, the default value is 43ºC.
Copy link
Contributor

Choose a reason for hiding this comment

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

See above comment.

@rraustad rraustad added the IDDChange Code changes impact the IDD file (cannot be merged after IO freeze) label Apr 2, 2021
@rraustad
Copy link
Contributor Author

I am considering A35, \field Fluid Inlet Node Name, and then Min/Max Fluid Inlet Temp..., but am not quite satisfied with that choice even though it is as generic as it gets.

@mjwitte
Copy link
Contributor

mjwitte commented Apr 20, 2021

I don't really like Fluid, because even though technically, air is a "fluid", I tend to think of liquids when I see that word. What you've done already here is adequate, not worth messing with. @mitchute If you're ok with that, then go ahead with final review here.

Copy link
Collaborator

@mitchute mitchute left a comment

Choose a reason for hiding this comment

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

This looks OK to me. I'm with Mike--skip adding "fluid" if to A35 if you can help it.

@rraustad
Copy link
Contributor Author

Well, I don't mind changing to MinMax Condenser Inlet Node Temperature ... It will only take about an hour. Up to you guys.

@mitchute
Copy link
Collaborator

"MinMax Condenser Inlet Node Temperature" is about as clear as it gets as far as I'm concerned. Go for it.

@rraustad rraustad added the Defect Includes code to repair a defect in EnergyPlus label Apr 30, 2021
@rraustad
Copy link
Contributor Author

@lgu1234 this should be ready now.

@lgu1234
Copy link
Contributor

lgu1234 commented May 26, 2021

@Myoldmopar I am OK for the changes. Please take a final look.

@rraustad
Copy link
Contributor Author

rraustad commented Jun 9, 2021

Will change this to EXPECT_NEAR in unit test VRF_SysModel_inAirloop.
EXPECT_EQ(SysOutputProvided, 0.0); where SysOutputProvided = 2.47e-12

For the failed unit test with 2.47e-12 zero issue, the calculation of SysOutputProvided is:

LoadMet = AirMassFlow * PsyDeltaHSenFnTdb2W2Tdb1W1(TempOut, SpecHumOut, TempIn, SpecHumIn); // sensible {W}
AirMassFlow = 0.682...
TempIn = 24.000000000000000
TempOut = 24.000000000000004
SpecHumIn = 0.010000000000000
SpecHumOut = 0.010000000000000

The coils are off. The slight error is in the OA Mixer mixing calculation of mixed air temp.

state.dataMixedAir->OAMixer(OAMixerNum).MixEnthalpy =
    (RecircMassFlowRate * RecircEnthalpy +
     state.dataMixedAir->OAMixer(OAMixerNum).OAMassFlowRate * state.dataMixedAir->OAMixer(OAMixerNum).OAEnthalpy) /
    state.dataMixedAir->OAMixer(OAMixerNum).MixMassFlowRate;
state.dataMixedAir->OAMixer(OAMixerNum).MixHumRat = (RecircMassFlowRate * RecircHumRat + state.dataMixedAir->OAMixer(OAMixerNum).OAMassFlowRate *
                                                                                             state.dataMixedAir->OAMixer(OAMixerNum).OAHumRat) /
                                                    state.dataMixedAir->OAMixer(OAMixerNum).MixMassFlowRate;
state.dataMixedAir->OAMixer(OAMixerNum).MixPressure =
    (RecircMassFlowRate * RecircPressure +
     state.dataMixedAir->OAMixer(OAMixerNum).OAMassFlowRate * state.dataMixedAir->OAMixer(OAMixerNum).OAPressure) /
    state.dataMixedAir->OAMixer(OAMixerNum).MixMassFlowRate;
// Mixed air temperature is calculated from the mixed air enthalpy and humidity ratio.
state.dataMixedAir->OAMixer(OAMixerNum).MixTemp =
    PsyTdbFnHW(state.dataMixedAir->OAMixer(OAMixerNum).MixEnthalpy, state.dataMixedAir->OAMixer(OAMixerNum).MixHumRat);

image

@@ -1,1565 +1,1565 @@
// EnergyPlus, Copyright (c) 1996-2021, The Board of Trustees of the University of Illinois,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I changed 3 lines of code in this unit test file. Good luck finding them. Something is going on with Clang.

Copy link
Contributor Author

@rraustad rraustad Jun 9, 2021

Choose a reason for hiding this comment

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

Nope, I just changed the VRF unit test file. @Myoldmopar might want to check if line endings is still an issue in UnitaryHybridAirConditioner.unit.cc

@Myoldmopar
Copy link
Member

Got this tab open to get this reviewed in the morning. I will check on the line endings issue first.

@Myoldmopar
Copy link
Member

This looks great now. I pulled develop in and then converted the line endings in that unit test file back. It is testing now but after it passes I'll push the changes up. Assuming the unitary file now shows proper lines changed I'll merge it in directly. Thanks @rraustad for the changes and @mjwitte @lgu1234 @mitchute for the reviews.

@Myoldmopar
Copy link
Member

Files changed tab is back in good shape. Merging.

@Myoldmopar Myoldmopar merged commit e32b496 into develop Jun 22, 2021
@Myoldmopar Myoldmopar deleted the 8687-Water-cooled-VRF-heating-mode-operation branch June 22, 2021 15:32
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 IDDChange Code changes impact the IDD file (cannot be merged after IO freeze)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Water-cooled VRF heating mode operation
10 participants