Skip to content

VRF power in heat recovery using thermostat offset priority is 0 when coils are active#6498

Merged
Myoldmopar merged 11 commits intodevelopfrom
#6485-VRF-power-in-heat-recovery-using-ThermostatOffsetPriority-is-0-when-coils-are-active
Mar 18, 2018
Merged

VRF power in heat recovery using thermostat offset priority is 0 when coils are active#6498
Myoldmopar merged 11 commits intodevelopfrom
#6485-VRF-power-in-heat-recovery-using-ThermostatOffsetPriority-is-0-when-coils-are-active

Conversation

@rraustad
Copy link
Copy Markdown
Collaborator

@rraustad rraustad commented Feb 23, 2018 •
edited
Loading

Pull request overview

The VRF power reported sometimes equals 0 when the Master Thermostat Priority Control Type = ThermostatOffsetPriority. This change corrects Issue #6485. Also corrected is the energy consumption during heat recovery and the start up multiplier used to derate performance at the beginning of heat recovery mode. A warning that notifies users when outdoor conditions exceed heat recovery mode operating temperatures was also improved to report only if heat recovery mode is requested.

Work Checklist

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)
  • At least one of the following appropriate labels must be added to this PR to be consumed into the changelog:
    • Defect: This pull request repairs a github defect issue. The github issue should be referenced in the PR description
    • Refactoring: This pull request includes code changes that don't change the functionality of the program, just perform refactoring
    • NewFeature: This pull request includes code to add a new feature to EnergyPlus
    • Performance: This pull request includes code changes that are directed at improving the runtime performance of EnergyPlus
    • DoNoPublish: This pull request includes changes that shouldn't be included in the changelog

Review Checklist

This will not be exhaustively relevant to every PR.

  • Code style (parentheses padding, variable names)
  • Functional code review (it has to work!)
  • If defect, results of running current develop vs this branch should exhibit the fix
  • CI status: all green or justified
  • Performance: CI Linux results include performance check -- verify this
  • Unit Test(s)
  • C++ checks:
    • Argument types
    • If any virtual classes, ensure virtual destructor included, other things
  • IDD changes:
    • Verify naming conventions and styles, memos and notes and defaults
    • Open windows IDF Editor with modified IDD to check for errors
    • If transition, add rules to spreadsheet
    • If transition, add transition source
    • If transition, update idfs
  • If new idf included, locally check the err file and other outputs
  • Documentation changes in place
  • Changed docs build successfully
  • ExpandObjects changes?
  • If output changes, including tabular output structure, add to output rules file for interfaces

rraustad
rraustad commented Feb 26, 2018
View reviewed changes
src/EnergyPlus/HVACVariableRefrigerantFlow.cc
Node( CondenserOutletNode ).MassFlowRate = CondenserWaterMassFlowRate;

Node( CondenserOutletNode ).MassFlowRateMaxAvail = Node( CondenserOutletNode ).MassFlowRateMaxAvail;
Node( CondenserOutletNode ).MassFlowRateMinAvail = Node( CondenserOutletNode ).MassFlowRateMinAvail;
Copy link
Copy Markdown
Collaborator Author

@rraustad rraustad Feb 26, 2018

Choose a reason for hiding this comment

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

Well these 2 lines look suspect. Should be outlet = inlet.

Copy link
Copy Markdown
Collaborator Author

@rraustad rraustad Feb 28, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Well, that uneasy feeling of not knowing exactly how to use these Min/Max/MinAvail/MaxAvail variables is upon me.

  1. UpdateVRFCondenser is only called for water-cooled equipment
  2. Water based equipment should not set Min/MaxAvail since SetComponentFlowRate handles that. These lines don't hurt anything here since it writes outlet = outlet (but should be removed eventually).
  3. Setting Node( CondenserOutletNode ).MassFlowRate = CondenserWaterMassFlowRate; here is OK but redundant since SetComponentMassFlowRate already did that.
  4. I noticed the VRF TU's are not passing MinAvail and MaxAvail through to the coil outlet node as do other air-side equipment. Probably works because TU's are zone equipment.
  5. Do no harm, no changes here at this point.

@rraustad rraustad added Defect Includes code to repair a defect in EnergyPlus NotIDDChange Code does not impact IDD (can be merged after IO freeze) labels Feb 26, 2018
@rraustad
Copy link
Copy Markdown
Collaborator Author

rraustad commented Feb 28, 2018 •
edited
Loading

CI shows 1 file with big diff's, HVACTemplte_5ZoneVRF. Not sure where these diff's are coming from.

The eplusout.csv.absdiff file says there are diff's in the Cooling Tower Heat Transfer Rate (and other tiny diff's but this is the largest). When I plot V8.8, V8.9 develop and V8.9 this branch, this data are identical. Even on this scale you can see there is not a difference of 600 W when the unit is off at x-axis time 30 and 31 (6:00 AM and 7:00 AM)

So this looks good to me and the diff's are explained as not real.

hvactemplate_5zonevrfdiffs

@rraustad
Copy link
Copy Markdown
Collaborator Author

rraustad commented Feb 28, 2018 •
edited
Loading

Here's the diff's side-by-side. There is a 12 W diff right at startup (see This branch column at 9 AM) that I can't explain, but from there the results are identical. This data was used to create the graph above.

vrfdiffs

@mjwitte mjwitte added this to the EnergyPlus 8.9.0 milestone Mar 2, 2018
@Myoldmopar
Copy link
Copy Markdown
Member

Myoldmopar commented Mar 16, 2018

@rraustad I don't expect you to also address this one tonight, but if you look at the CI results here you'll see one example file (HVACTemplate-5ZoneVRF) is actually failing, not just showing diffs. If you want to point me to a possible issue, I'm happy to try to find a few minutes to help track it down over the next day or so, but no guarantee. Thanks for all the effort.

@rraustad
Copy link
Copy Markdown
Collaborator Author

rraustad commented Mar 16, 2018

This is what worried me. First off that's HVAC template that failed, not the example file. So is it what I hacked up (or didn't) in expand objects or something else?

mjwitte added 2 commits March 16, 2018 14:09
@mjwitte
Merge remote-tracking branch 'remotes/origin/develop' into #6485-VRF-…
9280d0e 
…power-in-heat-recovery-using-ThermostatOffsetPriority-is-0-when-coils-are-active
@mjwitte
VRF protect against div by zero
d1be498
@mjwitte
Copy link
Copy Markdown
Contributor

mjwitte commented Mar 16, 2018

@Myoldmopar The test failure was a divide by zero. Let's see if CI is happy now.

@Myoldmopar
Merge remote-tracking branch 'origin/develop' into #6485-VRF-power-in…
335ac82 
…-heat-recovery-using-ThermostatOffsetPriority-is-0-when-coils-are-active
@Myoldmopar
Copy link
Copy Markdown
Member

Myoldmopar commented Mar 17, 2018

Pulled latest develop in here; letting CI run overnight, anticipate merging in the morning.

@Myoldmopar
Merge remote-tracking branch 'origin/develop' into #6485-VRF-power-in…
da891b8 
…-heat-recovery-using-ThermostatOffsetPriority-is-0-when-coils-are-active
@Myoldmopar
Copy link
Copy Markdown
Member

Myoldmopar commented Mar 17, 2018

@nrel-bot-2 was taking a while doing the Linux builds. I went ahead and merged in some other PRs, and they would start causing diffs here, so I pulled develop into this branch and I'll get a full set of CI results. It is looking good though from the initial results so I anticipate it dropping in easily.

@Myoldmopar Myoldmopar mentioned this pull request Mar 17, 2018
Merged
29 tasks
@Myoldmopar Myoldmopar merged commit fe4adef into develop Mar 18, 2018
@Myoldmopar Myoldmopar deleted the #6485-VRF-power-in-heat-recovery-using-ThermostatOffsetPriority-is-0-when-coils-are-active branch March 18, 2018 00:26
@Myoldmopar Myoldmopar mentioned this pull request Mar 18, 2018
Closed
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

Defect Includes code to repair a defect in EnergyPlus NotIDDChange Code does not impact IDD (can be merged after IO freeze)

Projects

None yet

Milestone

EnergyPlus 8.9.0

Development

Successfully merging this pull request may close these issues.

6 participants

@rraustad @Myoldmopar @mjwitte @nrel-bot @nrel-bot-2 @nrel-bot-3