-
Notifications
You must be signed in to change notification settings - Fork 438
Fix chiller chilling when it is off #10793
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
Conversation
@rraustad @EnergyArchmage This condition occurs when the chiller has a load, but the chiller shuts down here due to zero condenser water flow and leaves EnergyPlus/src/EnergyPlus/ChillerElectricEIR.cc Lines 1957 to 1972 in ec7398a
This has been set before this exit: EnergyPlus/src/EnergyPlus/ChillerElectricEIR.cc Line 1939 in ec7398a
(which seems wrong, wouldn't you want to intialize outlet=inlet before that actual calcs happen?) After this, EnergyPlus/src/EnergyPlus/ChillerElectricEIR.cc Lines 2585 to 2594 in ec7398a
|
Oh, never mind the condenser flow rate comment, those lines are for aircooled condenser. |
|
Line 1939 does seem wrong as something to do before a potential return at line 1970. Also, as an observation, if the return at 1970 happens I would think the calc routine would want to inform other later functions that the chiller RunFlag = false, |
Given the arguments here
Perhaps |
Probably, because that return value for &CurLoad will be used by the calling routine (I think) to update the remaining load. |
If the calling routine does use &CurLoad then I would think that MyLoad should represent exactly what load the chiller met. I don't think I have ever considered that before. |
Yes, that is the idea. So it can update remaining load for dispatch to other machines if any. |
|
|
@@ -1962,6 +1962,8 @@ void ElectricEIRChillerSpecs::calculate(EnergyPlusData &state, Real64 &MyLoad, b | |||
state, this->CWPlantLoc, this->CondMassFlowIndex, this->CDPlantLoc, DataPlant::CriteriaType::MassFlowRate, this->CondMassFlowRate); | |||
|
|||
if (this->CondMassFlowRate < DataBranchAirLoopPlant::MassFlowTolerance) { | |||
// Shut chiller off if there is no condenser water flow | |||
MyLoad = 0.0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This does look like it boils down to this. I looked at PullCompInterconnectTrigger to see what that was trying to accomplish. This code looks like it can be deleted. I'm not asking for that to happen here but it's extra clock cycles. Removed in #10794,
// Check to make sure we didn't reuse the index in multiple components
if (CurCriteria.CallingCompLoopNum != plantLoc.loopNum || CurCriteria.CallingCompLoopSideNum != plantLoc.loopSideNum ||
CurCriteria.CallingCompBranchNum != plantLoc.branchNum || CurCriteria.CallingCompCompNum != plantLoc.compNum) {
// Diagnostic fatal: component does not properly utilize unique indexing
}
With the same fix applied to all chiller types, annual regression results:
|
|
@rraustad I'm doubting this approach now due to these results:
Maybe this needs to check for (gasp) |
Well, it looked good when I reviewed this. MyLoad is disconnected from water flow rate so, for now, I think no need to look at flowlock and this is just a way to tell the plant that this component did not meet a load or what was actually met (and probably should not exceed original MyLoad?). Since this does look like the correct approach I think verifying a components operation in 1 of these files. Was this component meeting a load that it shouldn't have, or is now passing that load to another component, or is off now and the plant load increases on the next iteration? Too hard to think this through without some hard data. |
For WaterSideEconomizer_NonIntegrated-Chiller, here are some hours where the chiller is showing evap outlet temp < evap inlet temp when the chiller is off. Develop is on the left, this branch on the right. This file has 652 hours like that with develop, zero hours with this branch, so maybe this is fixing a real problem. |
I do think this is fixing something. Note the integrated economizer inlet now tracks the correct water temp. Why this temp changed from 10 C to 16 C is a bit of a mystery but without that free cooling the plant temps have risen. What are the plant outlet node temps wrt set point? maybe a plant outlet temp vs plant mass flow rate plot. And if the plant return water temp is > 12 C, why isn't the chiller on? is there a bypass pipe that has flow? There was no mass flow rate at the chiller before so it seems it should be off, but why? Availability Manager? |
So, the waterside economizer HX hardly ever runs, and I haven't figured out why the chiller is off when there's a high loop demand. There's no plant availability manager, and a simple plant operating scheme with the chiller always available. There is flow through the HX on the chilled water side, but no flow on the other side of the HX. Not sure why the HX has flow if it's not contributing. Hmmm, this is a water cooled Chiller:EngineDriven. Looking at 02/23 10:00-15:00, in both branches, the CENTRAL CHILLER:Chiller Evaporator Mass Flow Rate kg/s is zero, but there is flow on the Central Chiller Inlet/Outlet Nodes(!). The chiller is LeavingSetpointModulated, maybe that's not working right? |
@EnergyArchmage has found a few places in code where the WWHX is not passing information correctly (i.e., SafeCopyPlantNode is needed in a few places). I can't recall exactly what the details are so maybe he could comment on what you are seeing. |
@rraustad Oh, silly me. This is why the chiller is not operating:
The condenser pump is scheduled off, but everything else is always available. So the chiller tries to run, but then when it sees zero condenser flow it's "off" (but not really) - exactly the case that this is fixing. Changing the file so that the condenser pump is always available eliminates the diffs between develop and this branch and eliminates the unmet hours. So, this file needs some review, because the HX is only running for 8 hrs of the year in Chicago. But that can be a separate issue. Some unit tests would be nice here. |
This just makes me wonder why a pump would ever be schedule off. |
|
|
||
// test with no condenser flow available - chiller should be off | ||
state->dataLoopNodes->Node(thisChiller.CondInletNodeNum).MassFlowRate = 0.0; | ||
state->dataLoopNodes->Node(thisChiller.CondInletNodeNum).MassFlowRateMaxAvail = 0.0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
MaxAvail does need to be set to 0 here, at the inlet node, but I wonder why. It's a limit. If the chiller requests 0 then it should set to 0 if not in series with another component? FlowCtrl = Invalid in this unit test so it's not series active. I tested this unit test both ways, with this line commented out and with the MaxAvail = 1. It failed both times. I guess the plant manager will set MaxAvail as necessary, at the condenser inlet, and this line is needed for that reason. The chiller can be off and still have flow at the condenser and/or evaporator if the plant requests it (e.g., no bypass loop).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just wanted to be sure the chiller saw that there was no condenser flow. Anyways, the extended unit test fails with develop. I'm going to add the evap mass flow = 0 change now to the other chiller types.
|
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like this adds the right zeroing/initialization to missing spots. I'm happy with this.
@@ -1728,6 +1728,9 @@ void BLASTAbsorberSpecs::calculate(EnergyPlusData &state, Real64 &MyLoad, bool R | |||
this->CondOutletTemp = state.dataLoopNodes->Node(this->CondInletNodeNum).Temp; | |||
this->CondMassFlowRate = 0.0; | |||
this->QCondenser = 0.0; | |||
MyLoad = 0.0; | |||
this->EvapMassFlowRate = 0.0; | |||
PlantUtilities::SetComponentFlowRate(state, this->EvapMassFlowRate, this->EvapInletNodeNum, this->EvapOutletNodeNum, this->CWPlantLoc); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But of course.
this->EvapMassFlowRate = 0.0; | ||
PlantUtilities::SetComponentFlowRate(state, this->EvapMassFlowRate, this->EvapInletNodeNum, this->EvapOutletNodeNum, this->CWPlantLoc); | ||
return; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks right.
And it's happy with develop pulled in as well. Thanks @mjwitte |
Pull request overview
MyLoad
to zero if lack of condenser water flow causes the chiller to be off. No more of the above warnings are produced (with a full annual regression).MyLoad = 0.0
this->EvapMassFlowRate = 0.0
SetComponentFlowRate
for evaporator flow rate.Pull Request Author
Add to this list or remove from it as applicable. This is a simple templated set of guidelines.
Reviewer
This will not be exhaustively relevant to every PR.