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

Resolve warnings/errors for unreasonable DHW temperatures in Example Files #9078

Merged
merged 9 commits into from
Dec 9, 2021

Conversation

matthew-larson
Copy link
Contributor

@matthew-larson matthew-larson commented Sep 20, 2021

Pull request overview

  • Fixes Resolve warnings/errors for unreasonable DHW temperatures in Example Files #9075
  • This pull request is a followup to PR Add warnings/errors for unreasonable DHW temperatures for WaterUse:Equipment #8808 to resolve new warnings/errors created in the Example Files. 35 Example Files were generating warnings/errors unreasonable DHW temperatures for WaterUse:Equipment. It was found that many of the new warnings were being generated during the warmup period, in which the DHW temperature was decreasing over time (separate issue?), below the target temperature. A conditional statement was added to check state.dataGlobal->WarmupFlag and the warnings are now only generated if the unreasonable DHW temperatures occur outside of the warmup period.
  • Warnings were also being generated when the target temperature matched the hot water supply temperature, and due to losses in the system, the hot water temperature would fall just short of the target temperature. To resolve this, the Target Temperature Schedule Name was set to blank and the schedule removed from the model, so the target temperature just becomes the incoming hot water temperature. This may also indicate a larger issue where the water heater is not supplying the correct hot water temperature and is underestimating the water heater energy use.

60C Water Heater Setpoint Temperature
image

  • WaterHeaterHeatPumpWrappedCondenser.idf is the lone exception where the target temperature is below the hot water supply temperature but the incoming hot water temperature doesn't maintain the hot water supply temperature and still falls below the target temperature, so warnings will still be generated for this model.

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

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
  • 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

@matthew-larson matthew-larson added Defect Includes code to repair a defect in EnergyPlus NotIDDChange Code does not impact IDD (can be merged after IO freeze) labels Sep 20, 2021
@matthew-larson matthew-larson added this to the EnergyPlus 2022.1 milestone Sep 20, 2021
@matthew-larson matthew-larson self-assigned this Sep 20, 2021
@matthew-larson matthew-larson marked this pull request as ready for review September 22, 2021 15:31
@@ -1058,6 +1058,7 @@ namespace WaterUse {
// Calculate desired hot and cold water flow rates

Real64 const EPSILON(1.e-3);
Real64 TempDiff;
Copy link
Member

Choose a reason for hiding this comment

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

You can declare and define all three diffs separately here, that way you don't have to calculate them once for the if statements and again within them.

this->CWHWTempErrIndex,
TempDiff,
TempDiff);
}
}
} else if ((this->TargetTemp - this->HotTemp) > EPSILON) {
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if these should be separate if statements. It strikes me that they aren't mutually exclusive. i.e., you could have a target temperature that is colder than the hot water temperature that is colder than the cold water temperature. The question is if we want to show all three warnings in this scenario.

@@ -1942,13 +1942,6 @@
For: AllDays, !- Field 2
Until: 24:00,0.2; !- Field 3

Schedule:Compact,
Copy link
Member

Choose a reason for hiding this comment

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

I believe these are models provided by PNNL. They might want to make sure that they are making the same changes for their other prototype models too. @lymereJ do you know who to contact?

Copy link
Collaborator

@lymereJ lymereJ Sep 22, 2021

Choose a reason for hiding this comment

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

Thanks! I'll let @jiangithub and @yanchenpnnl know.

@nrel-bot-3
Copy link

@matthew-larson @lgentile it has been 28 days since this pull request was last updated.

@Myoldmopar
Copy link
Member

@nealkruis you previously requested changes on this, could you take a look at whether you still request changes after the last commit? Thanks!

@nealkruis
Copy link
Member

I think this is ready to review. The only outstanding task is creating an issue for water heaters unable to meet its setpoint despite having plenty of heating capacity (@matthew-larson can you handle writing that up?).

Note: Our assumptions for these example files is that the intention when the target temperature was the same as the setpoint temperature was for 100% hot water (not necessarily at the set point). This interpretation does not change results, but some of these files (and some users) may want to show when the models are not able to deliver the target temperature, especially if these are supposed to represent prototypical energy use.

@shorowit
Copy link
Contributor

shorowit commented Nov 1, 2021

@matthew-larson @nealkruis Have you considered the situation where a solar water heating system is present? If we remove the Target Temperature Schedule Name from the WaterUse:Equipment objects, we can end up delivering mixed water temperatures above the desired setpoint at times, which is not a good result. But if we include the Target Temperature Schedule Name to limit the mixed water temperatures to our desired setpoint, we can still get these target temperature warnings at certain times. (Although in my quick test file below, it only occurred warmup? But I assume that it could happen during the simulation period too, especially if say the beginning of the simulation period is cloudy.)

Some IDFs to help:

  • Solar water heating with Target Temperature Schedule: with.idf.txt
  • Solar water heating without Target Temperature Schedule: without.idf.txt

@nealkruis
Copy link
Member

@shorowit we can look into this, but it looks like you are not using a tempering valve with your water heating system. I would think this would resolve the issue.

@shorowit
Copy link
Contributor

shorowit commented Nov 2, 2021

Interesting, thanks for the info @nealkruis. We've never used that object before as we've historically used the Target Temperature Schedule to essentially provide tempering -- but have moved away from it to avoid these new E+ warnings (except for WaterUse:Equipment objects that represent hot water fixtures that typically use water at temperatures (e.g., 105F) below the water heater setpoint (e.g., 125F)).

Maybe the TemperingValve is indeed the solution. Unfortunately I don't have time right now to understand the nuances of how the TemperingValve object differs from using the Target Temperature Schedule.

@nealkruis
Copy link
Member

I believe most solar hot water systems need a tempering valve to prevent scalding. This is on the supply side whereas the target temperatures are applied to the demand side.

I do think that there is likely a new feature that would help provide greater flexibility in EnergyPlus which would be allowing each water use to specify a fraction of hot water instead of a target temperature. There are a lot of appliances that don't really use a target temperature but define settings based on different amounts of hot vs. cold water.

@nrel-bot
Copy link

nrel-bot commented Dec 1, 2021

@matthew-larson @lgentile it has been 28 days since this pull request was last updated.

Copy link
Member

@Myoldmopar Myoldmopar left a comment

Choose a reason for hiding this comment

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

This all seems fine. Looks like there is still maybe an outstanding task but beyond the scope of this PR. The IDF changes here look like nice cleanups, and deferring the warnings until not warmup is reasonable. I'll pull develop in here and let it run.

@Myoldmopar
Copy link
Member

This is looking good. ERR diffs show warnings getting removed, as expected. Other diffs are related to the IDF cleanups. @nealkruis you are still showing as "requesting changes" on this PR. Could you confirm that or clear that if you are satisfied? I think this is otherwise ready to go.

@Myoldmopar
Copy link
Member

Tested locally and I think this is all good to go. Merging.

@Myoldmopar Myoldmopar merged commit 603ffa2 into NREL:develop Dec 9, 2021
@Myoldmopar Myoldmopar deleted the fix-dhw-examplefiles branch December 9, 2021 21:32
yujiex pushed a commit that referenced this pull request Jan 27, 2022
Resolve warnings/errors for unreasonable DHW temperatures in Example Files
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 NotIDDChange Code does not impact IDD (can be merged after IO freeze)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Resolve warnings/errors for unreasonable DHW temperatures in Example Files
10 participants