Skip to content

Fix #10831 - HeatPump:PlantLoop:EIR:Heating with Setpoint control not operating when no setpoint on component outlet node #11247

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

Merged
mitchute merged 6 commits into from
Dec 18, 2025
+302 −26
Merged

Fix #10831 - HeatPump:PlantLoop:EIR:Heating with Setpoint control not operating when no setpoint on component outlet node #11247

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
333aed2
Add a unit test for #10831
jmarrec Oct 1, 2025
17925dc
Fix #10831 - Assume overall Loop setpoint for EIRPLHP when ControlTyp…
jmarrec Oct 1, 2025
4563a60
Modify unit test to check the warning about overall loop setpoint ass…
jmarrec Oct 1, 2025
014e097
Get Setpoint properly
jmarrec Oct 1, 2025
8c69966
Try to address review comments from @rraustad
jmarrec Nov 10, 2025
5c352d5
Modify tests accordingly after ea2693c0ac2e7692b943596d1726fc02baaf60e8
jmarrec Nov 21, 2025
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
53 changes: 49 additions & 4 deletions src/EnergyPlus/PlantLoopHeatPumpEIR.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,8 @@ void EIRPlantLoopHeatPump::simulate(

if (this->running) {
if (this->sysControlType == ControlType::Setpoint) {
Real64 leavingSetpoint = state.dataLoopNodes->Node(this->loadSideNodes.outlet).TempSetPoint;
// Call the helper so we handle SingleSetpoint versus DualSetpointDeadband instead of just grabbing TempSetPoint;
Real64 leavingSetpoint = this->getLoadSideOutletSetPointTemp(state);
Comment on lines +142 to +143
Copy link
Contributor Author

@jmarrec jmarrec Oct 2, 2025

Choose a reason for hiding this comment

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

This was prexisting as

Real64 leavingSetpoint = state.dataLoopNodes->Node(this->loadSideNodes.outlet).TempSetPoint;

But in case the Setpoint is actually DualSetpointWithDeadband, I don't think this is working as intended. So we call the helper instead.

I hope I got that right?

Copy link
Collaborator

@rraustad rraustad Nov 20, 2025

Choose a reason for hiding this comment

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

This appears to be the right thing to do but looking at this function it checks the plant control type and I'm not sure the EIR HP Setpoint control type is meant to be this flexible (i.e., to allow multiple set point types at the equipment outlet node). It's as if the getLoadSideOutletSetPointTemp function should look to the equipment Node(this->loadSideNodes.outlet).TempSetpoint variable when using Setpoint control type and this function doesn't do that. In the case of the plant DualSetPointDeadBand control scheme the equipment outlet node would need to find the TempSetpointHi/Lo set points but who's to say that the user intended to control to set point based on the plant control scheme. I got the feeling that @mjwitte would have fatal'd out if the correct set point was not found and I don't disagree with that. i.e., if the user chooses Setpoint then there better be a set point of the correct type at the equipment outlet node. There needs to be more thought on when EIR HP Setpoint control is SingleSetPoint (cooling or heating plant) or DualSetPointDeadBand (mixed plant), I can see both being useful but now only SingleSetPoint is used. Maybe the defect file should not have been using Setpoint control in this case as there was no set point manager used for the EIR HP outlet node?

Real64 CurSpecHeat = this->loadSidePlantLoc.loop->glycol->getSpecificHeat(state, loadSideInletTemp, "EIRPlantLoopHeatPump::simulate");
Real64 controlLoad = this->loadSideMassFlowRate * CurSpecHeat * (leavingSetpoint - loadSideInletTemp);

Expand All @@ -160,17 +161,17 @@ Real64 EIRPlantLoopHeatPump::getLoadSideOutletSetPointTemp(EnergyPlusData &state
if (this->loadSidePlantLoc.loop->LoopDemandCalcScheme == DataPlant::LoopDemandCalcScheme::SingleSetPoint) {
if (this->loadSidePlantLoc.comp->CurOpSchemeType == DataPlant::OpScheme::CompSetPtBased) {
// there will be a valid set-point on outlet
return state.dataLoopNodes->Node(this->loadSideNodes.outlet).TempSetPoint;
return state.dataLoopNodes->Node(this->setPointNodeNum).TempSetPoint;
} // use plant loop overall set-point
return state.dataLoopNodes->Node(this->loadSidePlantLoc.loop->TempSetPointNodeNum).TempSetPoint;
}
if (this->loadSidePlantLoc.loop->LoopDemandCalcScheme == DataPlant::LoopDemandCalcScheme::DualSetPointDeadBand) {
if (this->loadSidePlantLoc.comp->CurOpSchemeType == DataPlant::OpScheme::CompSetPtBased) {
// there will be a valid set-point on outlet
if (this->EIRHPType == DataPlant::PlantEquipmentType::HeatPumpEIRCooling) {
return state.dataLoopNodes->Node(this->loadSideNodes.outlet).TempSetPointHi;
return state.dataLoopNodes->Node(this->setPointNodeNum).TempSetPointHi;
}
return state.dataLoopNodes->Node(this->loadSideNodes.outlet).TempSetPointLo;
return state.dataLoopNodes->Node(this->setPointNodeNum).TempSetPointLo;

} // use plant loop overall set-point
if (this->EIRHPType == DataPlant::PlantEquipmentType::HeatPumpEIRCooling) {
Expand Down Expand Up @@ -2650,6 +2651,50 @@ void EIRPlantLoopHeatPump::oneTimeInit(EnergyPlusData &state)
}
}

if (this->sysControlType == ControlType::Setpoint) {

// check if setpoint on outlet node
if ((state.dataLoopNodes->Node(this->loadSideNodes.outlet).TempSetPoint == DataLoopNode::SensedNodeFlagValue) &&
(state.dataLoopNodes->Node(this->loadSideNodes.outlet).TempSetPointHi == DataLoopNode::SensedNodeFlagValue)) {
Copy link
Collaborator

@rraustad rraustad Nov 20, 2025

Choose a reason for hiding this comment

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

I'm not sure TempSetPointHi (or Lo) should be included here for the "equipment" setpoint control type. However, I haven't investigated this recently and am not sure which Node().TempSetPoint variables are used in the Setpoint control case.

HeatPump:PlantLoop:EIR:Cooling,
   A13, \field Control Type
        \note Heat pump can be controlled on leaving water temperature set point or plant load
        \type choice
        \key Setpoint
        \key Load
        \default Load

if (!state.dataGlobal->AnyEnergyManagementSystemInModel) {
if (!this->SetpointSetToLoopErrDone) {
ShowWarningError(state,
format("{}: Missing temperature setpoint for Setpoint Controlled {} name = \"{}\"",
routineName,
DataPlant::PlantEquipTypeNames[static_cast<int>(this->EIRHPType)],
this->name));
ShowContinueError(state, " A temperature setpoint is needed at the load side outlet node, use a SetpointManager");
ShowContinueError(state, " The overall loop setpoint will be assumed for the Heat Pump. The simulation continues ... ");
this->SetpointSetToLoopErrDone = true;
}
} else {
// need call to EMS to check node
bool fatalError = false; // but not really fatal yet, but should be.
EMSManager::CheckIfNodeSetPointManagedByEMS(state, this->loadSideNodes.outlet, HVAC::CtrlVarType::Temp, fatalError);
state.dataLoopNodes->NodeSetpointCheck(this->loadSideNodes.outlet).needsSetpointChecking = false;
if (fatalError) {
if (!this->SetpointSetToLoopErrDone) {
ShowWarningError(state,
format("{}: Missing temperature setpoint for Setpoint Controlled {} name = \"{}\"",
routineName,
DataPlant::PlantEquipTypeNames[static_cast<int>(this->EIRHPType)],
this->name));
ShowContinueError(state, " A temperature setpoint is needed at the load side outlet node when ControlType = Setpoint");
ShowContinueError(state, " use a Setpoint Manager to establish a setpoint at the outlet node ");
ShowContinueError(state, " or use an EMS actuator to establish a setpoint at the outlet node ");
ShowContinueError(state, " The overall loop setpoint will be assumed for the Heat Pump. The simulation continues ... ");
this->SetpointSetToLoopErrDone = true;
}
}
}
this->setPointNodeNum = this->loadSidePlantLoc.loop->TempSetPointNodeNum;
} else {
this->setPointNodeNum = this->loadSideNodes.outlet;
}
} else {
this->setPointNodeNum = this->loadSidePlantLoc.loop->TempSetPointNodeNum;
Comment on lines +2690 to +2695
Copy link
Collaborator

@mitchute mitchute Dec 18, 2025

Choose a reason for hiding this comment

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

This looks perfectly reasonable to me, and follows @rraustad's suggestions.

}

if (errFlag) {
ShowFatalError(state, format("{}: Program terminated due to previous condition(s).", routineName));
}
Expand Down
4 changes: 4 additions & 0 deletions src/EnergyPlus/PlantLoopHeatPumpEIR.hh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,8 @@ namespace EIRPlantLoopHeatPumps {
ControlType sysControlType = ControlType::Invalid;
DataPlant::FlowMode flowMode = DataPlant::FlowMode::Invalid;

bool SetpointSetToLoopErrDone = false; // True if the warning about setpoint is missing at the outlet node has been issued

// sizing data
Real64 heatSizingRatio = 1.0;
HeatSizingType heatSizingMethod = HeatSizingType::Invalid;
Expand Down Expand Up @@ -197,6 +199,8 @@ namespace EIRPlantLoopHeatPumps {
InOutNodePair heatRecoveryNodes;
bool heatRecoveryHeatPump = false; // HP that transfers heat between plants and should not increase plant size

int setPointNodeNum = 0;

// counters and indexes
int condMassFlowRateTriggerIndex = 0;
int recurringConcurrentOperationWarningIndex = 0;
Expand Down
Loading
Loading