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.

Sign up for GitHub

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

Jump to bottom

Remove FlowLock from plant components #10790

Draft
wants to merge 15 commits into
base: develop
Choose a base branch
from
Draft

Remove FlowLock from plant components #10790

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
d56f211
ChillerElectricEIR remove flowlock for no load flow
mjwitte Oct 14, 2024
8560a93
New okToIssueWarning function checks FlowLock
mjwitte Oct 14, 2024
37e676b
ChillerElectricEIR remove flowlock for warnings
mjwitte Oct 14, 2024
08a5f61
Merge remote-tracking branch 'remotes/origin/develop' into flowlock1
mjwitte Oct 15, 2024
0b330e4
ChillerReformulatedEIR remove flowlock
mjwitte Oct 15, 2024
04e9107
Merge remote-tracking branch 'remotes/origin/develop' into flowlock1
mjwitte Oct 16, 2024
22e12fe
ChillerElectricEIR set EvapMassFlowRate in initialize
mjwitte Oct 16, 2024
0e0005b
Warn if chiller chills when off
mjwitte Oct 17, 2024
e827c62
Warn if chiller chills when off
mjwitte Oct 17, 2024
394a2c7
Fix chiller off when condenser flow is zero
mjwitte Oct 17, 2024
172283a
Revert Warn if chiller chills when off
mjwitte Oct 18, 2024
03fce6d
Revert "ChillerReformulatedEIR remove flowlock"
mjwitte Oct 18, 2024
89d94b4
Merge remote-tracking branch 'remotes/origin/chillerEIRrepairs' into …
mjwitte Oct 18, 2024
290bf6e
Address const comment
mjwitte Oct 18, 2024
7cecb90
Merge remote-tracking branch 'remotes/origin/develop' into flowlock1
mjwitte Nov 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
144 changes: 73 additions & 71 deletions src/EnergyPlus/ChillerElectricEIR.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1346,14 +1346,14 @@ void ElectricEIRChillerSpecs::initialize(EnergyPlusData &state, bool const RunFl
state.dataLoopNodes->Node(state.dataPlnt->PlantLoop(this->CWPlantLoc.loopNum).TempSetPointNodeNum).TempSetPointHi;
}

Real64 mdot = 0.0;
this->EvapMassFlowRate = 0.0;
Copy link
Contributor

Choose a reason for hiding this comment

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

Well that's even better. I didn't think of that. It takes a village.

Real64 mdotCond = 0.0;
if ((std::abs(MyLoad) > 0.0) && RunFlag) {
mdot = this->EvapMassFlowRateMax;
if ((MyLoad < 0.0) && RunFlag) {
this->EvapMassFlowRate = this->EvapMassFlowRateMax;
mdotCond = this->CondMassFlowRateMax;
}

PlantUtilities::SetComponentFlowRate(state, mdot, this->EvapInletNodeNum, this->EvapOutletNodeNum, this->CWPlantLoc);
PlantUtilities::SetComponentFlowRate(state, this->EvapMassFlowRate, this->EvapInletNodeNum, this->EvapOutletNodeNum, this->CWPlantLoc);
Copy link
Contributor Author

@mjwitte mjwitte Oct 16, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Tried to set EvapMassFlowRate up in initialize but that starts introducing diffs. The chiller inlet node temp is slightly different from before in the first timestep when the chiller comes on. Note that just getting rid of abs here did not introduce any diffs. I'm running just the three icestorage files and getting diffs just in IceStorage-Parallel. CI might show other files with diffs.

I'm running annuals locally, so maybe CI won't show any diffs at all.

Copy link
Contributor

Choose a reason for hiding this comment

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

You didn't change anything else (and the abs isn't the cause). So why would using a report variable to make this initialization be any different than what was here originally? I wonder if it's this in update that is getting hit now?

    if (this->CondMassFlowRate < DataBranchAirLoopPlant::MassFlowTolerance &&
        this->EvapMassFlowRate < DataBranchAirLoopPlant::MassFlowTolerance) {


if (this->CondenserType == DataPlant::CondenserType::WaterCooled) {
PlantUtilities::SetComponentFlowRate(state, mdotCond, this->CondInletNodeNum, this->CondOutletNodeNum, this->CDPlantLoc);
Expand All @@ -1362,6 +1362,7 @@ void ElectricEIRChillerSpecs::initialize(EnergyPlusData &state, bool const RunFl
PlantUtilities::MinFlowIfBranchHasVSPump(state, this->CDPlantLoc, this->VSBranchPumpFoundCond, this->VSLoopPumpFoundCond, false);
}
// Initialize heat recovery flow rates at node
Real64 mdot = 0.0;
if (this->HeatRecActive) {
mdot = RunFlag ? this->DesignHeatRecMassFlowRate : 0.0; // if RunFlag is true, mdot = this->DesignHeatRecMassFlowRate, else mdot = 0.0
PlantUtilities::SetComponentFlowRate(state, mdot, this->HeatRecInletNodeNum, this->HeatRecOutletNodeNum, this->HRPlantLoc);
Expand Down Expand Up @@ -1866,18 +1867,24 @@ void ElectricEIRChillerSpecs::calculate(EnergyPlusData &state, Real64 &MyLoad, b
// if the component control is SERIESACTIVE we set the component flow to inlet flow so that
// flow resolver will not shut down the branch
if (MyLoad >= 0 || !RunFlag) {
if (this->EquipFlowCtrl == DataBranchAirLoopPlant::ControlType::SeriesActive ||
state.dataPlnt->PlantLoop(this->CWPlantLoc.loopNum).LoopSide(this->CWPlantLoc.loopSideNum).FlowLock == DataPlant::FlowLock::Locked) {
this->EvapMassFlowRate = state.dataLoopNodes->Node(this->EvapInletNodeNum).MassFlowRate;
}
if (this->CondenserType == DataPlant::CondenserType::WaterCooled) {
if (DataPlant::CompData::getPlantComponent(state, this->CDPlantLoc).FlowCtrl == DataBranchAirLoopPlant::ControlType::SeriesActive) {
this->CondMassFlowRate = state.dataLoopNodes->Node(this->CondInletNodeNum).MassFlowRate;
}
}
if (this->CondenserType == DataPlant::CondenserType::EvapCooled) {
this->EvapMassFlowRate = 0.0;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Had to add these zero lines to eliminate diffs for annual runs on the icestorage files (those were the first that came to mind where the chiller would be on a seriesactive branch).

Copy link
Contributor

Choose a reason for hiding this comment

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

This does look like a good change, but diffs from what?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Running this in the debugger, there are many times when it gets here to the no load block but EvapMassFlowRate is not zero, so calling SetComponentFlowRate with the non-zero flow changed the operation. Now, why isn't the flow rate zero before it gets here if there's no load, not sure. So maybe setting it to zero is a band-aid for something missed earlier?

Copy link
Contributor

Choose a reason for hiding this comment

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

EvapMassFlowRate appears to be the flow from the previous iteration. The node flow rate is set in initialize, but that's on the node data not this struct variable. So the node info should already be correct here? except that initialize uses std::abs(MyLoad). I guess it depends on how RunFlag gets set.

Real64 mdot = 0.0;
Real64 mdotCond = 0.0;
if ((std::abs(MyLoad) > 0.0) && RunFlag) {
    mdot = this->EvapMassFlowRateMax;
    mdotCond = this->CondMassFlowRateMax;
}
PlantUtilities::SetComponentFlowRate(state, mdot, this->EvapInletNodeNum, this->EvapOutletNodeNum, this->CWPlantLoc);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@rraustad Thanks, that explains the diffs. The old code would set the flow rates based on the node data. But when using SetComponentFlowRate the value of the flow when called is seen as a request, so if the node flow rate is zero but the max avail is not zero, then passing in a non-zero flow could result in the node flow rate being changed.

Copy link
Contributor

Choose a reason for hiding this comment

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

If you moved line 1869 to initialize, right after the code I show above, and changed it to this->EvapMassFlowRate = mdot then I think you can remove lines 1869 and 1870 since line 1869 was already called in initialize (I am a little worried about the difference in conditionals). That would just leave the clean up of the condenser flows as you've done below at lines 1872 - 1889. If no other changes are needed then just leave it since this is a good change as is.

Copy link
Contributor

Choose a reason for hiding this comment

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

I thought of a better way. Just set EvapMassFlowRate to the node flow here and delete the evap side SetComponentFlowRate call.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My understanding of the goal here is to let SetComponentFlowRate make the connection with the node, so setting it to the node flow rate would be back to the old code.

But your point is well taken that this shouldn't need a call to SetComponentFlowRate when load is zero, because that's already happened in initialize.

So, why is initialize using std::abs(MyLoad) > 0.0? To be consistent with the tests in calculate and update it should be MyLoad < 0.0.

And with the old code, I don't see anywhere that this->EvapMassFlowRate gets set to zero when the chiller is off.

And there are places that are setting node mass flow rates directly or reading from them, and messing with max/min avails etc. Hmm, this effort was just supposed to focus on eliminating FlowLock . . .

Copy link
Contributor

@rraustad rraustad Oct 16, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I think the original code (std::abs(MyLoad)) in init was just trying to force a positive value for load. But I was worried that MyLoad could be passed as a positive value in some cases and not be the same logic used in calc (i.e., might be diffs if you fixed it), I didn't track that down. I suspect there is a std::min(MyLoad, 0) in the plant manager for cooling components. So MyLoad < 0 should be the same as using abs. No reset of EvapMassFlowRate is probably an oversite. Scatter plotting that report variable and the node mass flow rate should give a perfect 45 degree slope. I'd venture to say that's not happening right now in some cases. Don't go too far with this, you've reached your goal. I was just trying to tighten this up a little to remove the unnecessary call to SetComponentFlowRate when no load/off condition exists, and how to get that done as efficient as possible. i.e., set EvapMassFlowRate right here in the if (MyLoad >= 0 || !RunFlag) { block instead of init so it's not always executed, it will get updated correctly later in calc if there is a load. Nothing really needs to change here because it's being done correctly now, it's just using that redundant call to match up EvapMassFlowRate with what happened in init which "could" be eliminated by reading the node. I also just noticed that the condenser water flow rate is also called in init so that node data is also already set correctly, no need for that function call either, and the condenser flow report variable is already zero'd at the top of calc (you could move that here). Reading the node has got to be faster than the function call(s). The min/max avail manipulation (I've done that myself in the past) should be done in SetComponentFlowRate because that's very important for the plant flow resolver, but that can be looked at later. I think I'm done now.

PlantUtilities::SetComponentFlowRate(state, this->EvapMassFlowRate, this->EvapInletNodeNum, this->EvapOutletNodeNum, this->CWPlantLoc);

switch (this->CondenserType) {
case DataPlant::CondenserType::AirCooled: {
// nothing to do for air cooled
} break;
case DataPlant::CondenserType::WaterCooled: {
this->CondMassFlowRate = 0.0;
PlantUtilities::SetComponentFlowRate(state, this->CondMassFlowRate, this->CondInletNodeNum, this->CondOutletNodeNum, this->CDPlantLoc);
} break;
case DataPlant::CondenserType::EvapCooled: {
CalcBasinHeaterPower(
state, this->BasinHeaterPowerFTempDiff, this->BasinHeaterSchedulePtr, this->BasinHeaterSetPointTemp, this->BasinHeaterPower);
} break;
default: {
} break;
assert(false);
}
this->PrintMessage = false;
return;
Expand Down Expand Up @@ -1962,6 +1969,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;
if (this->EvapMassFlowRate < DataBranchAirLoopPlant::MassFlowTolerance) {
// Use PlantUtilities::SetComponentFlowRate to decide actual flow
PlantUtilities::SetComponentFlowRate(
Expand Down Expand Up @@ -2028,27 +2037,26 @@ void ElectricEIRChillerSpecs::calculate(EnergyPlusData &state, Real64 &MyLoad, b
this->ChillerCapFT = Curve::CurveValue(state, this->ChillerCapFTIndex, EvapOutletTempSetPoint, AvgCondSinkTemp);

if (this->ChillerCapFT < 0) {
if (this->ChillerCapFTError < 1 &&
state.dataPlnt->PlantLoop(this->CWPlantLoc.loopNum).LoopSide(this->CWPlantLoc.loopSideNum).FlowLock != DataPlant::FlowLock::Unlocked &&
!state.dataGlobal->WarmupFlag) {
if (PlantUtilities::okToIssueWarning(state, this->CWPlantLoc)) {
++this->ChillerCapFTError;
ShowWarningError(state, format("CHILLER:ELECTRIC:EIR \"{}\":", this->Name));
ShowContinueError(state, format(" Chiller Capacity as a Function of Temperature curve output is negative ({:.3R}).", this->ChillerCapFT));
ShowContinueError(state,
format(" Negative value occurs using an Evaporator Outlet Temp of {:.1R} and a Condenser Inlet Temp of {:.1R}.",
EvapOutletTempSetPoint,
condInletTemp));
ShowContinueErrorTimeStamp(state, " Resetting curve output to zero and continuing simulation.");
} else if (state.dataPlnt->PlantLoop(this->CWPlantLoc.loopNum).LoopSide(this->CWPlantLoc.loopSideNum).FlowLock !=
DataPlant::FlowLock::Unlocked &&
!state.dataGlobal->WarmupFlag) {
++this->ChillerCapFTError;
ShowRecurringWarningErrorAtEnd(state,
"CHILLER:ELECTRIC:EIR \"" + this->Name +
"\": Chiller Capacity as a Function of Temperature curve output is negative warning continues...",
this->ChillerCapFTErrorIndex,
this->ChillerCapFT,
this->ChillerCapFT);
if (this->ChillerCapFTError < 1) {
ShowWarningError(state, format("CHILLER:ELECTRIC:EIR \"{}\":", this->Name));
ShowContinueError(state,
format(" Chiller Capacity as a Function of Temperature curve output is negative ({:.3R}).", this->ChillerCapFT));
ShowContinueError(state,
format(" Negative value occurs using an Evaporator Outlet Temp of {:.1R} and a Condenser Inlet Temp of {:.1R}.",
EvapOutletTempSetPoint,
condInletTemp));
ShowContinueErrorTimeStamp(state, " Resetting curve output to zero and continuing simulation.");
} else {
++this->ChillerCapFTError;
ShowRecurringWarningErrorAtEnd(state,
"CHILLER:ELECTRIC:EIR \"" + this->Name +
"\": Chiller Capacity as a Function of Temperature curve output is negative warning continues...",
this->ChillerCapFTErrorIndex,
this->ChillerCapFT,
this->ChillerCapFT);
}
}
this->ChillerCapFT = 0.0;
}
Expand Down Expand Up @@ -2308,51 +2316,45 @@ void ElectricEIRChillerSpecs::calculate(EnergyPlusData &state, Real64 &MyLoad, b

this->ChillerEIRFT = Curve::CurveValue(state, this->ChillerEIRFTIndex, this->EvapOutletTemp, AvgCondSinkTemp);
if (this->ChillerEIRFT < 0.0) {
if (this->ChillerEIRFTError < 1 &&
state.dataPlnt->PlantLoop(this->CWPlantLoc.loopNum).LoopSide(this->CWPlantLoc.loopSideNum).FlowLock != DataPlant::FlowLock::Unlocked &&
!state.dataGlobal->WarmupFlag) {
++this->ChillerEIRFTError;
ShowWarningError(state, format("CHILLER:ELECTRIC:EIR \"{}\":", this->Name));
ShowContinueError(state, format(" Chiller EIR as a Function of Temperature curve output is negative ({:.3R}).", this->ChillerEIRFT));
ShowContinueError(state,
format(" Negative value occurs using an Evaporator Outlet Temp of {:.1R} and a Condenser Inlet Temp of {:.1R}.",
this->EvapOutletTemp,
condInletTemp));
ShowContinueErrorTimeStamp(state, " Resetting curve output to zero and continuing simulation.");
} else if (state.dataPlnt->PlantLoop(this->CWPlantLoc.loopNum).LoopSide(this->CWPlantLoc.loopSideNum).FlowLock !=
DataPlant::FlowLock::Unlocked &&
!state.dataGlobal->WarmupFlag) {
if (PlantUtilities::okToIssueWarning(state, this->CWPlantLoc)) {
++this->ChillerEIRFTError;
ShowRecurringWarningErrorAtEnd(state,
"CHILLER:ELECTRIC:EIR \"" + this->Name +
"\": Chiller EIR as a Function of Temperature curve output is negative warning continues...",
this->ChillerEIRFTErrorIndex,
this->ChillerEIRFT,
this->ChillerEIRFT);
if (this->ChillerEIRFTError < 1) {
ShowWarningError(state, format("CHILLER:ELECTRIC:EIR \"{}\":", this->Name));
ShowContinueError(state, format(" Chiller EIR as a Function of Temperature curve output is negative ({:.3R}).", this->ChillerEIRFT));
ShowContinueError(state,
format(" Negative value occurs using an Evaporator Outlet Temp of {:.1R} and a Condenser Inlet Temp of {:.1R}.",
this->EvapOutletTemp,
condInletTemp));
ShowContinueErrorTimeStamp(state, " Resetting curve output to zero and continuing simulation.");
} else {
ShowRecurringWarningErrorAtEnd(state,
"CHILLER:ELECTRIC:EIR \"" + this->Name +
"\": Chiller EIR as a Function of Temperature curve output is negative warning continues...",
this->ChillerEIRFTErrorIndex,
this->ChillerEIRFT,
this->ChillerEIRFT);
}
}
this->ChillerEIRFT = 0.0;
}

this->ChillerEIRFPLR = Curve::CurveValue(state, this->ChillerEIRFPLRIndex, PartLoadRat);
if (this->ChillerEIRFPLR < 0.0) {
if (this->ChillerEIRFPLRError < 1 &&
state.dataPlnt->PlantLoop(this->CWPlantLoc.loopNum).LoopSide(this->CWPlantLoc.loopSideNum).FlowLock != DataPlant::FlowLock::Unlocked &&
!state.dataGlobal->WarmupFlag) {
++this->ChillerEIRFPLRError;
ShowWarningError(state, format("CHILLER:ELECTRIC:EIR \"{}\":", this->Name));
ShowContinueError(state, format(" Chiller EIR as a function of PLR curve output is negative ({:.3R}).", this->ChillerEIRFPLR));
ShowContinueError(state, format(" Negative value occurs using a part-load ratio of {:.3R}.", PartLoadRat));
ShowContinueErrorTimeStamp(state, " Resetting curve output to zero and continuing simulation.");
} else if (state.dataPlnt->PlantLoop(this->CWPlantLoc.loopNum).LoopSide(this->CWPlantLoc.loopSideNum).FlowLock !=
DataPlant::FlowLock::Unlocked &&
!state.dataGlobal->WarmupFlag) {
if (PlantUtilities::okToIssueWarning(state, this->CWPlantLoc)) {
++this->ChillerEIRFPLRError;
ShowRecurringWarningErrorAtEnd(state,
"CHILLER:ELECTRIC:EIR \"" + this->Name +
"\": Chiller EIR as a function of PLR curve output is negative warning continues...",
this->ChillerEIRFPLRErrorIndex,
this->ChillerEIRFPLR,
this->ChillerEIRFPLR);
if (this->ChillerEIRFPLRError < 1) {
ShowWarningError(state, format("CHILLER:ELECTRIC:EIR \"{}\":", this->Name));
ShowContinueError(state, format(" Chiller EIR as a function of PLR curve output is negative ({:.3R}).", this->ChillerEIRFPLR));
ShowContinueError(state, format(" Negative value occurs using a part-load ratio of {:.3R}.", PartLoadRat));
ShowContinueErrorTimeStamp(state, " Resetting curve output to zero and continuing simulation.");
} else {
ShowRecurringWarningErrorAtEnd(state,
"CHILLER:ELECTRIC:EIR \"" + this->Name +
"\": Chiller EIR as a function of PLR curve output is negative warning continues...",
this->ChillerEIRFPLRErrorIndex,
this->ChillerEIRFPLR,
this->ChillerEIRFPLR);
}
}
this->ChillerEIRFPLR = 0.0;
}
Expand Down
5 changes: 5 additions & 0 deletions src/EnergyPlus/PlantUtilities.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2045,4 +2045,9 @@ MinFlowIfBranchHasVSPump(EnergyPlusData &state, PlantLocation const &plantLoc, b
return branchPumpMinFlowLimit;
}

bool okToIssueWarning(EnergyPlusData const &state, PlantLocation const &plantLoc)
{
return (state.dataPlnt->PlantLoop(plantLoc.loopNum).LoopSide(plantLoc.loopSideNum).FlowLock != DataPlant::FlowLock::Unlocked &&
!state.dataGlobal->WarmupFlag);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't see it in CppCheck results but it sure looks like the state argument can be declared as const? Maybe you tried that?

} // namespace EnergyPlus::PlantUtilities
2 changes: 2 additions & 0 deletions src/EnergyPlus/PlantUtilities.hh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,8 @@ namespace PlantUtilities {
Real64 MinFlowIfBranchHasVSPump(
EnergyPlusData &state, PlantLocation const &pLantLoc, bool &foundBranchPump, bool &foundLoopPump, bool const setFlowStatus);

bool okToIssueWarning(EnergyPlusData const &state, PlantLocation const &plantLoc);

struct CriteriaData
{
// Members
Expand Down
Loading