Skip to content

Conversation

@lymereJ
Copy link
Collaborator

@lymereJ lymereJ commented May 19, 2025

Pull request overview

Description of the purpose of this PR

Fix gas-fired absorption flow requests.

Pull Request Author

  • 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
  • If any defect files are updated to a more recent version, upload new versions here or on DevSupport

Reviewer

  • 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
  • Verify IDF naming conventions and styles, memos and notes and defaults
  • If new idf included, locally check the err file and other outputs

@lymereJ lymereJ added the Defect Includes code to repair a defect in EnergyPlus label May 19, 2025
@lymereJ lymereJ added this to the EnergyPlus 25.2 IO Freeze milestone May 19, 2025
@github-actions
Copy link

⚠️ Regressions detected on macos-14 for commit 41c650f

Regression Summary
  • ESO Big Diffs: 1

@lymereJ
Copy link
Collaborator Author

lymereJ commented May 20, 2025

Here are the results for the (GAHP) defect file after updating it to 25.2 and with this branch.

image

Here's the updated file:
large_office_6A_vav_hw_reheat_out_gahp.idf.zip

@NREL NREL deleted a comment from github-actions bot May 20, 2025
@lymereJ
Copy link
Collaborator Author

lymereJ commented May 20, 2025

ESO diff explanation:
Unlike the defect file, in develop, the example file shows flow rate through the unit, but because the load on the loop is small the GAHP does not use any gas (see code change walk-through). With this branch, the unit has a fuel rate and heat transfer greater than 0 during these times which I think is correct. See screenshot below (develop is on top, results from this branch are at the bottom).

image

Copy link
Collaborator Author

@lymereJ lymereJ left a comment

Choose a reason for hiding this comment

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

Code changes walkthrough:

: this->sourceSideDesignMassFlowRate;

if (!FirstHVACIteration && this->flowControl == DataPlant::FlowMode::VariableSpeedPump) {
if (!FirstHVACIteration && this->flowMode == DataPlant::FlowMode::VariableSpeedPump) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Use flowMode to be consistent with the enum class name.

}
}

void EIRFuelFiredHeatPump::setOperatingFlowRatesASHP(EnergyPlusData &state, bool FirstHVACIteration, Real64 const currentLoad)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Set the flow rates at the same time as EIRPlantLoopHeatPumps. Move the logic from EIRFuelFiredHeatPump::doPhysics here.

Comment on lines -2866 to +2778
if (partLoadRatio < this->minPLR) {
this->fuelRate = 0.0;
this->powerUsage = 0.0;
} else {
this->fuelRate = this->loadSideHeatTransfer / (this->referenceCOP * CRF) * eirModifierFuncPLR * eirModifierFuncTemp * eirDefrost;
this->fuelRate = this->loadSideHeatTransfer / (this->referenceCOP * CRF) * eirModifierFuncPLR * eirModifierFuncTemp * eirDefrost;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The previous behavior was to zero out the fuel rate when the partLoadRatio was less than the minPLR which triggered instances where the unit was not using any energy.

Comment on lines +4505 to +4515
bool firstHVAC = true;
Real64 curLoad = 100;
bool runFlag = true;
Real64 constexpr specifiedLoadSetpoint = 45;
state->dataLoopNodes->Node(thisHeatingPLHP->loadSideNodes.outlet).TempSetPoint = specifiedLoadSetpoint;
state->dataLoopNodes->Node(thisHeatingPLHP->loadSideNodes.inlet).Temp = 40;
state->dataLoopNodes->Node(thisHeatingPLHP->sourceSideNodes.inlet).Temp = 30;
thisHeatingPLHP->simulate(*state, myLoadLocation, firstHVAC, curLoad, runFlag);
EXPECT_TRUE(thisHeatingPLHP->fuelRate > 0);
EXPECT_NEAR(5.0, thisHeatingPLHP->loadSideMassFlowRate, 0.001);
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Test to check that fuelRate > 0 when partLoadRatio < minPLR.

Comment on lines +4517 to +4529
{
bool firstHVAC = false;
Real64 curLoad = 2000;
bool runFlag = true;
Real64 constexpr expectedLoadMassFlowRate = 0.0478;
Real64 constexpr specifiedLoadSetpoint = 45;
state->dataLoopNodes->Node(thisHeatingPLHP->loadSideNodes.outlet).TempSetPoint = specifiedLoadSetpoint;
state->dataLoopNodes->Node(thisHeatingPLHP->loadSideNodes.inlet).Temp = 35;
state->dataLoopNodes->Node(thisHeatingPLHP->sourceSideNodes.inlet).Temp = 30;
thisHeatingPLHP->flowMode = DataPlant::FlowMode::LeavingSetpointModulated;
thisHeatingPLHP->simulate(*state, myLoadLocation, firstHVAC, curLoad, runFlag);
EXPECT_NEAR(expectedLoadMassFlowRate, thisHeatingPLHP->loadSideMassFlowRate, 0.001);
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Test to check the variable flow logic.

@lymereJ lymereJ marked this pull request as ready for review May 20, 2025 20:29
# Conflicts:
#	src/EnergyPlus/PlantLoopHeatPumpEIR.cc
@github-actions
Copy link

⚠️ Regressions detected on macos-14 for commit 4e46c89

Regression Summary
  • Table Big Diffs: 3
  • ESO Big Diffs: 1

}
this->powerUsage = this->nominalAuxElecPower * eirAuxElecFuncTemp * eirAuxElecFuncPLR;
if (this->defrostType == DefrostType::Timed) {
this->powerUsage += this->defrostResistiveHeaterCap * this->defrostOpTimeFrac * reportingInterval;
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is reportingInterval included here? It was included before but looks odd. It seems that defrostOpTimeFrac * reportingInterval = seconds and J/s * s = joules. So powerUsage += joules ??

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, that does not seem correct. Should I make the change in this PR? Or, open an issue and fix it in a different one?

Copy link
Member

Choose a reason for hiding this comment

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

I think go ahead and make the change here rather than opening a new PR.

@github-actions
Copy link

⚠️ Regressions detected on macos-14 for commit d32fe3a

Regression Summary
  • Table Big Diffs: 3
  • ESO Big Diffs: 1

@Myoldmopar
Copy link
Member

Alright, and with that fix in place, I think this is good to go in. Thanks for spotting it @rraustad and thanks for the quick fix up @lymereJ. This checks out locally and is ready to go, merging.

@Myoldmopar Myoldmopar merged commit dfa3d53 into develop May 28, 2025
9 checks passed
@Myoldmopar Myoldmopar deleted the fix_gahp_flow branch May 28, 2025 13:41
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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Gas-Fired absorption heat pump flow control issue

6 participants