-
Notifications
You must be signed in to change notification settings - Fork 398
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
Heat Pump Plant Loop EIR refactoring #10404
Conversation
…p_heat_recovery_refactor
This is a refactoring effort; hence, diffs are not expected. However, a diff was found in one example file ( |
An existing unit test failed due to the new |
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.
Code refactoring walkthrough for EIRPlantLoopHeatPump
} | ||
state.dataLoopNodes->Node(this->sourceSideNodes.outlet).Temp = this->sourceSideOutletTemp; | ||
// update report variables and nodes | ||
this->report(state); |
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.
Moved the node update to newly created function report()
.
@@ -198,6 +194,7 @@ void EIRPlantLoopHeatPump::resetReportingVariables() | |||
this->fractionalDefrostTime = 0.0; | |||
this->partLoadRatio = 0.0; | |||
this->cyclingRatio = 0.0; | |||
this->defrostPowerMultiplier = 1.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.
Since defrost calculation is refactored to a new function I added a new member variable defrostPowerMultiplier
for use else where.
Real64 constexpr RH60 = 60.0; | ||
Real64 constexpr rangeRH = 30.0; | ||
Real64 const reportingInterval = state.dataHVACGlobal->TimeStepSysSec; | ||
|
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.
Moved to a new function heatingCapacityModifierASHP
.
this->doPhysicsWSHP(state, currentLoad); | ||
} else if (this->airSource) { | ||
this->doPhysicsASHP(state, currentLoad); | ||
} |
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.
Now we have two separate functions doPhysicsWSHP()
and doPhysicsASHP()
that are wrapped in existing doPhysics()
. The doPhysics()
is a virtual function used for EIRFuelFiredHeatPump
module as well.
|
||
// evaluate the source side heat transfer rate | ||
this->calcSourceSideHeatTransferWSHP(state); | ||
} |
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.
The WSHP do physics new function doPhysicsWSHP()
, which is a wrapper for all water source calculations. No defrost calculation call here.
// energy balance on heat pump | ||
// this->sourceSideHeatTransfer = this->calcQsource(this->loadSideHeatTransfer, this->powerUsage); | ||
this->sourceSideHeatTransfer = this->calcQsource(this->loadSideHeatTransfer, this->fuelRate + this->powerUsage - this->standbyElecPower); | ||
this->sourceSideEnergy = this->sourceSideHeatTransfer * reportingInterval; | ||
|
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.
Report variables update has moved to the new member report()
function.
state.dataLoopNodes->Node(this->loadSideNodes.outlet).Temp = this->loadSideOutletTemp; | ||
state.dataLoopNodes->Node(this->sourceSideNodes.outlet).Temp = this->sourceSideOutletTemp; | ||
} | ||
|
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.
Add similar new member report() function for EIRFuelFiredHeatPump
module as well.
void calcSourceSideHeatTransferASHP(EnergyPlusData &state); | ||
|
||
virtual void report(EnergyPlusData &state); | ||
|
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.
The various refactored member functions.
void capModFTCurveCheck(EnergyPlusData &state, const Real64 loadSideOutletSPTemp, Real64 &capModFTemp); | ||
|
||
void eirModCurveCheck(EnergyPlusData &state, Real64 &eirModFTemp, Real64 &eirModFPLR); | ||
|
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.
Same as above.
@@ -3499,6 +3499,7 @@ TEST_F(EnergyPlusFixture, Test_Curve_Negative_Energy) | |||
thisCoolingPLHP->loadSideInletTemp = 20; | |||
thisCoolingPLHP->sourceSideInletTemp = 20; | |||
thisCoolingPLHP->doPhysics(*state, curLoad); | |||
thisCoolingPLHP->report(*state); |
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.
called the new report()
function here to update the report variables for testing.
This branch is ready for review, and it includes the fix for issue #10403. Because of this fix, diff is expected in |
@Nigusse this is now conflicted, looks like maybe you made a similar-but-not-quite-the-same change in your heat pump fix branch? I'm going to hold off on this for now, but feel free to merge up develop and resolve that small conflict. |
I am going to work on it now. |
@Myoldmopar Merged in develop and resolved conflict. Run regression tests on five example files (with EIR Heat Pump) and found no diffs. Diffs were not expected for this branch. |
Thanks @Nigusse, I'll let CI run, but this will merge if CI is green. |
All green, thanks @Nigusse, merging. |
Pull request overview
HeatPump:PlantLoop:EIR:*
module as part of a new feature that adds Heat Recovery capability to Air-to-water heat pumpNOTE: ENHANCEMENTS MUST FOLLOW A SUBMISSION PROCESS INCLUDING A FEATURE PROPOSAL AND DESIGN DOCUMENT PRIOR TO SUBMITTING CODE
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.