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

jmarrec · 2025-10-02T10:20:25Z

Pull request overview

Fix HeatPump:PlantLoop:EIR:Heating with Setpoint control not operating #10831

Description of the purpose of this PR

Pull Request Author

Reviewer

…e=Setpoint and missing one at outlet

…umed

jmarrec · 2025-10-02T10:29:32Z

Running the defect files on dev support.

primary_school_setpoint_ctrl_v2420.idf: this is Setpoint Controlled, but there is no setpoint on the component outlet
primary_school_setpoint_ctrl-addSetPoint_v2420.idf: this places the same setpoint as the overall loop setpoint on the component outlet
- It is provided as a reference point

Diff of the two files:

Now, the result. As expected, the addSetPoint case is unchanged with this branch. The other one correctly assumes the overall loop setpoint in the after case.

Note that the eplusout.err does not include the new warning because that file has EMS.

jmarrec · 2025-10-02T10:37:05Z

I have a MCVE on DevSupport too, where I do see the new warning.

   ** Warning ** EIRPlantLoopHeatPump : oneTimeInit: Missing temperature setpoint for Setpoint Controlled HeatPump:PlantLoop:EIR:Heating name = "HEATING COIL"
   **   ~~~   **   A temperature setpoint is needed at the load side outlet node, use a SetpointManager
   **   ~~~   **   The overall loop setpoint will be assumed for the Heat Pump. The simulation continues ... 
   ** Warning ** EIRPlantLoopHeatPump : oneTimeInit: Missing temperature setpoint for Setpoint Controlled HeatPump:PlantLoop:EIR:Cooling name = "COOLING COIL"
   **   ~~~   **   A temperature setpoint is needed at the load side outlet node, use a SetpointManager
   **   ~~~   **   The overall loop setpoint will be assumed for the Heat Pump. The simulation continues ...

jmarrec · 2025-10-02T10:39:18Z

src/EnergyPlus/PlantLoopHeatPumpEIR.cc

+        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)) {
+                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->SetpointSetToLoop = true;
+                state.dataLoopNodes->Node(this->loadSideNodes.outlet).TempSetPoint =
+                    state.dataLoopNodes->Node(this->loadSidePlantLoc.loop->TempSetPointNodeNum).TempSetPoint;
+                state.dataLoopNodes->Node(this->loadSideNodes.outlet).TempSetPointLo =
+                    state.dataLoopNodes->Node(this->loadSidePlantLoc.loop->TempSetPointNodeNum).TempSetPointLo;
+                state.dataLoopNodes->Node(this->loadSideNodes.outlet).TempSetPointHi =
+                    state.dataLoopNodes->Node(this->loadSidePlantLoc.loop->TempSetPointNodeNum).TempSetPointHi;
+            }
+        }


In oneTimeInit, when SetpointCOntrolled, we check if there's no setpoint, and assume the overall loop setpoint.

Error is dumped once only, and the "SetpointSetToLoop" is set to true.

I am surprised this model does not check for a valid set point since it's been around for quite some time. It points to a set point node but where is the check for DataLoopNode::SensedNodeFlagValue? Maybe at a higher level in the plant manager. Anyway, I think getLoadSideOutletSetPointTemp already handles where the set point is located. I think you just need to correct old line 142 to new line 155 and always call this function. Once you made that 1-line change you fixed it. The new warnings are nice but I have to believe someone has already thought of plant node set point temperatures and warnings for all the plant equipment that can be controlled to a set point, at the component outlet or loop outlet based on the control type scheme.

Real64 EIRPlantLoopHeatPump::getLoadSideOutletSetPointTemp(EnergyPlusData &state) if (thisLoadPlantLoop.LoopDemandCalcScheme == DataPlant::LoopDemandCalcScheme::SingleSetPoint) { if (thisLoadComp.CurOpSchemeType == DataPlant::OpScheme::CompSetPtBased) { // there will be a valid set-point on outlet return state.dataLoopNodes->Node(this->loadSideNodes.outlet).TempSetPoint; } else { // use plant loop overall set-point return state.dataLoopNodes->Node(thisLoadPlantLoop.TempSetPointNodeNum).TempSetPoint; } } else if (thisLoadPlantLoop.LoopDemandCalcScheme == DataPlant::LoopDemandCalcScheme::DualSetPointDeadBand) {

PlantManager line 2483:

if (this->SupplyEnvrnFlag && state.dataGlobal->BeginEnvrnFlag) { for (int LoopNum = 1; LoopNum <= this->plant->TotNumLoops; ++LoopNum) { // check if setpoints being placed on node properly if (this->plant->PlantLoop(LoopNum).LoopDemandCalcScheme == DataPlant::LoopDemandCalcScheme::DualSetPointDeadBand) { if (state.dataLoopNodes->Node(this->plant->PlantLoop(LoopNum).TempSetPointNodeNum).TempSetPointHi == SensedNodeFlagValue) { if (!state.dataGlobal->AnyEnergyManagementSystemInModel) {

Note that the boiler and chillers use this as the equipment set point control node. I searched for TempSetPointNodeNum and it shows up in most/all plant equipment. Maybe that needs to be used instead of what getLoadSideOutletSetPointTemp does? This is weird, the EIR HP can be set point controlled while the plant can use other control types at the same time. If the EIR HP is SP controlled and has SPs at it's outlet node, and the plant is SP controlled and has SPs at the loop outlet node then which node has the priority for use as the SP? Maybe what you have done is more accurate for equipment that have a ControlType input field where the SP should be at the equipment outlet. I am, however, thinking it would be better to figure this out early and use a new this->setPointNodeNum variable that just points to the correct node and jetison copying node data each iteration. Maybe that would be a good follow up to this PR and fix all this hokeyness in the equipment models after thinking this through.

this->plant->PlantLoop(LoopNum).TempSetPointNodeNum

Summary of what is needed to complete this work. If the EIR HP is set point controlled then a warning that the SP is missing at the outlet is fine. The plant does have a SP at the plant outlet node. But getLoadSideOutletSetPointTemp will only use that SP if the plant op scheme is NOT CompSetPtBased. I just don't like the idea of copying node data each iteration so I would rather find a different solution. What I would do is either fatal out if there is not SP at the EIR HP outlet node, or right here check if state.dataLoopNodes->Node(this->loadSideNodes.outlet).TempSetPoint == DataLoopNode::SensedNodeFlagValue and use the plant loop outlet node SP instead. @mjwitte ?

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; } else { // use plant loop overall set-point return state.dataLoopNodes->Node(this->loadSidePlantLoc.loop->TempSetPointNodeNum).TempSetPoint; }

I do like the idea of using this in the future, but for this issue what this change is doing is fine. setPointNodeNum would be set up early to point to the correct node, either the EIR HP outlet node or the plant loop outlet node.

return state.dataLoopNodes->Node(this->setPointNodeNum).TempSetPoint;

in getInput or Init choose either:

this->setPointNodeNum = this->loadSideNodes.outlet, or this->setPointNodeNum = this->loadSidePlantLoc.loop->TempSetPointNodeNum

This would simplify the code in getLoadSideOutletSetPointTemp.

jmarrec · 2025-10-02T10:39:31Z

src/EnergyPlus/PlantLoopHeatPumpEIR.hh

+        bool SetpointSetToLoop = false;        // True if the setpoint is missing at the outlet node
+        bool SetpointSetToLoopErrDone = false; // True if setpoint warning issued


New state vars

jmarrec · 2025-10-02T10:40:07Z

src/EnergyPlus/PlantLoopHeatPumpEIR.cc

    if (this->running) {
        if (this->sysControlType == ControlType::Setpoint) {
-            Real64 leavingSetpoint = state.dataLoopNodes->Node(this->loadSideNodes.outlet).TempSetPoint;
+            if (this->SetpointSetToLoop) {
+                // Copy the overall loop setpoints to the Heat Pump outlet node
+                state.dataLoopNodes->Node(this->loadSideNodes.outlet).TempSetPoint =
+                    state.dataLoopNodes->Node(this->loadSidePlantLoc.loop->TempSetPointNodeNum).TempSetPoint;
+                // Hi is used when (this->EIRHPType == DataPlant::PlantEquipmentType::HeatPumpEIRCooling) {
+                state.dataLoopNodes->Node(this->loadSideNodes.outlet).TempSetPointHi =
+                    state.dataLoopNodes->Node(this->loadSidePlantLoc.loop->TempSetPointNodeNum).TempSetPointHi;
+                // Lo when HeatPumpEIRHeating
+                state.dataLoopNodes->Node(this->loadSideNodes.outlet).TempSetPointLo =
+                    state.dataLoopNodes->Node(this->loadSidePlantLoc.loop->TempSetPointNodeNum).TempSetPointLo;
+            }


If SetpointSetToLoop is true, in simulate we just copy the plant overall setpoints onto the outlet node.

jmarrec · 2025-10-02T10:41:00Z

src/EnergyPlus/PlantLoopHeatPumpEIR.cc

+            // Call the helper so we handle SingleSetpoint versus DualSetpointDeadband instead of just grabbing TempSetPoint;
+            Real64 leavingSetpoint = this->getLoadSideOutletSetPointTemp(state);


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?

jmarrec · 2025-10-02T10:42:22Z

tst/EnergyPlus/unit/PlantLoopHeatPumpEIR.unit.cc

+    EXPECT_NEAR(30.0, thisHeatingPLHP->getLoadSideOutletSetPointTemp(*state), 0.001);
+    EXPECT_NEAR(30.0, state->dataLoopNodes->Node(thisHeatingPLHP->loadSideNodes.outlet).TempSetPoint, 0.001);


before fix, -999...

jmarrec · 2025-10-02T10:42:53Z

tst/EnergyPlus/unit/PlantLoopHeatPumpEIR.unit.cc

+    EXPECT_NEAR(10.0, thisHeatingPLHP->getLoadSideOutletSetPointTemp(*state), 0.001);
+    EXPECT_NEAR(DataLoopNode::SensedNodeFlagValue, state->dataLoopNodes->Node(thisHeatingPLHP->loadSideNodes.outlet).TempSetPoint, 0.001);
+    EXPECT_NEAR(30.0, state->dataLoopNodes->Node(thisHeatingPLHP->loadSideNodes.outlet).TempSetPointHi, 0.001);
+    EXPECT_NEAR(10.0, state->dataLoopNodes->Node(thisHeatingPLHP->loadSideNodes.outlet).TempSetPointLo, 0.001);


Same here, not 10.0 (or 30.0) but -999 before fix

github-actions · 2025-10-02T10:53:36Z

⚠️ Regressions detected on macos-14 for commit `5339267`

Regression Summary

EIO: 3
ERR: 3
ESO Big Diffs: 3
Table Big Diffs: 1
Table Small Diffs: 1

github-actions · 2025-10-02T11:03:30Z

⚠️ Regressions detected on ubuntu-24.04 for commit `5339267`

Regression Summary

EIO: 3
ERR: 3
ESO Big Diffs: 3
Table Big Diffs: 1
Table Small Diffs: 1

jmarrec added 4 commits October 2, 2025 11:22

Add a unit test for #10831

8b046da

Fix #10831 - Assume overall Loop setpoint for EIRPLHP when ControlTyp…

ab48ffb

…e=Setpoint and missing one at outlet

Modify unit test to check the warning about overall loop setpoint ass…

e6b4c9c

…umed

Get Setpoint properly

cca3edc

jmarrec self-assigned this Oct 2, 2025

jmarrec added Defect Includes code to repair a defect in EnergyPlus NotIDDChange Code does not impact IDD (can be merged after IO freeze) labels Oct 2, 2025

jmarrec commented Oct 2, 2025

View reviewed changes

mitchute requested review from mitchute and rraustad October 8, 2025 19:58

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

		// Call the helper so we handle SingleSetpoint versus DualSetpointDeadband instead of just grabbing TempSetPoint;
		Real64 leavingSetpoint = this->getLoadSideOutletSetPointTemp(state);

		EXPECT_NEAR(30.0, thisHeatingPLHP->getLoadSideOutletSetPointTemp(*state), 0.001);
		EXPECT_NEAR(30.0, state->dataLoopNodes->Node(thisHeatingPLHP->loadSideNodes.outlet).TempSetPoint, 0.001);

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

Are you sure you want to change the base?

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

Conversation

jmarrec commented Oct 2, 2025

Pull request overview

Description of the purpose of this PR

Pull Request Author

Reviewer

Uh oh!

jmarrec commented Oct 2, 2025

Uh oh!

jmarrec commented Oct 2, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rraustad Oct 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rraustad Oct 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Oct 2, 2025

⚠️ Regressions detected on macos-14 for commit 5339267

Uh oh!

github-actions bot commented Oct 2, 2025

⚠️ Regressions detected on ubuntu-24.04 for commit 5339267

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

rraustad Oct 8, 2025 •

edited

Loading

rraustad Oct 11, 2025 •

edited

Loading

⚠️ Regressions detected on macos-14 for commit `5339267`

⚠️ Regressions detected on ubuntu-24.04 for commit `5339267`