Skip to content

Conversation

@matthew-larson
Copy link
Contributor

Pull request overview

Before:
image

After:
image

NOTE: Need to update documentation for Companion Heat Pump Name field description

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

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

@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 May 26, 2021
@matthew-larson matthew-larson added this to the EnergyPlus 9.6 BugFix Freeze milestone May 26, 2021
@matthew-larson matthew-larson self-assigned this May 26, 2021
@matthew-larson
Copy link
Contributor Author

@Myoldmopar @nealkruis An alternative would be to move this function call to within the iteration steps (or possibly before setting the operating flow rates in the simulate function) and if both heat pumps are running, have a way to determine which load (heating or cooling) gets precedence and shut off it's companion heat pump. This would ensure the heat pumps are not heating/cooling concurrently and then provide a warning to the user that this action was taken.

@Myoldmopar
Copy link
Member

Have you given any additional thought to how you would determine which mode has precedence? I personally think that would be a lot nicer than just fatalling out, but I'm open to discussion. I suppose we could always ask the user to specify one mode over the other if we wanted, that would put the burden onto them to make the best decision for their particular application rather than us making that choice for them. @rraustad @mjwitte what do you think?

@rraustad
Copy link
Collaborator

The problem is the dual plant loops. Also sounds like a new SPM is required for objects that have 2 coils to model a single coil but that's out of scope. I would think the magnitude of the load would dictate which coil to operate. Usually this would be load > 0 for 1 coil and 0 load for the other but when that rule is broken, the largest load is met and the other coil is locked out. A field HP would also do this since it is a single coil but then it's also a single plant loop.

@Myoldmopar
Copy link
Member

@matthew-larson what's your thoughts on wrapping this up? The last comment from @rraustad does make sense, that you try to meet the larger demand, but it will be a problem if the previous plant loop isn't resimulated. But fatalling out seems excessive when it theoretically could just meet one of the demands. Let me know if we should discuss this again or if you have a path forward. Or if I'm missing something on the status of this.

@matthew-larson
Copy link
Contributor Author

@Myoldmopar I agree trying to meet the largest load makes the most sense. Due to the added complexity and priority with other issues, we are holding off on this for now. We may try to get it in this release if possible or wait until the next round. If it is urgent, we are OK with someone else taking over this pull request as well.

@Myoldmopar Myoldmopar modified the milestones: EnergyPlus 9.6 BugFix Freeze, EnergyPlus 9.6 Release Aug 6, 2021
@nrel-bot
Copy link

nrel-bot commented Sep 4, 2021

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

ShowContinueError(
state,
format("PLHP companion coil source side heat transfer rate= {:.2R} [W]", thisPLHP.companionHeatPumpCoil->sourceSideHeatTransfer));
ShowFatalError(state, "CheckConcurrentOperation: Simulation terminated because of problems in plant loop heat pump operation");
Copy link
Collaborator

Choose a reason for hiding this comment

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

I am not sure this is the correct method. I think this would occur when there is a cooling and heating HP on the same plant loop. I mean, this is a HP so there is only 1 HX. Ok, that makes sense to have a method to heat and cool the loop to be within some temperature range. If this is true, why would the inlet temperature of one HP ever be different from the other? Unless of course you have 2 HXs and one of those HPs is upstream of the other. And if that is the case then if the first one runs then the second one should be off. As an example, the plant loop temperature set points would be say 10C for cooling and something lower, like 7C for heating so that the plant loop temp stayed between 7C and 10C.. Did I get those control temps correct? So if the inlet temp = 11C, the cooling HP would be on and the heating HP off, even if the cooling HP operated the heating HP inlet temp should not be below 7C. Same if inlet temp = 6C, only heating HP would operate. So what scenario would cause both HPs to operate? Switched cooling/heating set point temperature? One HP overshooting the set point? What is happening in the defect file to cause both HPs to operate?

@nrel-bot
Copy link

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

4 similar comments
@nrel-bot-3
Copy link

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

@nrel-bot-2b
Copy link

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

@nrel-bot-3
Copy link

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

@nrel-bot-3
Copy link

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

@nrel-bot-2
Copy link

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

@nrel-bot-2
Copy link

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

@nrel-bot-2
Copy link

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

@nrel-bot-2
Copy link

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

1 similar comment
@nrel-bot-2
Copy link

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

@nrel-bot-2
Copy link

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

@nrel-bot-2c
Copy link

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

@nrel-bot-2c
Copy link

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

@nrel-bot-2c
Copy link

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

1 similar comment
@nrel-bot-2
Copy link

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

@nrel-bot-2c
Copy link

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

@nrel-bot-2c
Copy link

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

@nrel-bot-2c
Copy link

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

@nrel-bot-2
Copy link

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

2 similar comments
@nrel-bot-2c
Copy link

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

@nrel-bot-2
Copy link

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

@mitchute mitchute removed this from the EnergyPlus 24.2 milestone Oct 22, 2025
@nrel-bot-2c
Copy link

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

11 similar comments
@nrel-bot-2c
Copy link

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

@nrel-bot-2
Copy link

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

@nrel-bot-2
Copy link

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

@nrel-bot-2c
Copy link

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

@nrel-bot-2
Copy link

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

@nrel-bot-2
Copy link

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

@nrel-bot-2
Copy link

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

@nrel-bot-2
Copy link

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

@nrel-bot-2
Copy link

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

@nrel-bot-2
Copy link

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

@nrel-bot-2
Copy link

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

@nrel-bot-2c
Copy link

@jcyuan2020 @matthew-larson it has been 8 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 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.

HeatPump:PlantLoop:EIR:* allows simultaneous heating/cooling with just a warning