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.

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

Modify ground surface conduction calculation for Kiva surfaces and ensure conduction heat balance #9269

Merged
merged 19 commits into from
Mar 18, 2022

Conversation

matthew-larson
Copy link
Contributor

@matthew-larson matthew-larson commented Feb 14, 2022

Pull request overview

  • Fixes Kiva zone heat balance around inner surface boundary using conduction not balancing #9268
  • This pull request modifies the CalcSurfaceWeightedMRT calculation for Kiva surfaces to exclude the weighted surface so the area-weighted average of the surface temperatures more closely align with the results from Kiva. The conduction calculation was also redefined as conduction = -(convection + radiation) to resolve the heat balance when using conduction.
  • A vector was also added called AllHTKivaSurfaceList to contain the surface numbers of the Foundation boundary condition surfaces and refactored some code using this new vector.

Below are the annual conduction energy values showing the original (OLD) CalcSurfaceWeightedMRT calculation for Kiva surfaces and the NEW CalcSurfaceWeightedMRT calculation for Kiva surfaces. The modified calculation leads to a balance in conduction energy and heating/cooling energy within the zone.
image

Pull Request Author

Add to this list or remove from it as applicable. This is a simple templated set of guidelines.

  • 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

Reviewer

This will not be exhaustively relevant to every PR.

  • 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
  • CI status: all green or justified
  • Check that performance is not impacted (CI Linux results include performance check)
  • Run Unit Test(s) locally

@matthew-larson matthew-larson added Defect Includes code to repair a defect in EnergyPlus NotIDDChange Code does not impact IDD (can be merged after IO freeze) labels Feb 14, 2022
@matthew-larson matthew-larson added this to the EnergyPlus 22.1 milestone Feb 14, 2022
@matthew-larson matthew-larson self-assigned this Feb 14, 2022
Comment on lines 2146 to 2151
if (AverageWithSurface) {
CalcSurfaceWeightedMRT =
0.5 * (state.dataHeatBalSurf->SurfInsideTempHist(1)(SurfNum) + (SumAET / state.dataThermalComforts->ZoneAESum(ZoneNum)));
} else {
CalcSurfaceWeightedMRT = SumAET / state.dataThermalComforts->ZoneAESum(ZoneNum);
}
Copy link
Member

Choose a reason for hiding this comment

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

How about this:

CalcSurfaceWeightedMRT = SumAET / state.dataThermalComforts->ZoneAESum(ZoneNum);
if (AverageWithSurface) {
    CalcSurfaceWeightedMRT = 0.5 * (state.dataHeatBalSurf->SurfInsideTempHist(1)(SurfNum) + CalcSurfaceWeightedMRT
}

Comment on lines 2158 to 2162
if (AverageWithSurface) {
CalcSurfaceWeightedMRT = 0.5 * (state.dataHeatBalSurf->SurfInsideTempHist(1)(SurfNum) + state.dataHeatBalFanSys->MAT(ZoneNum));
} else {
CalcSurfaceWeightedMRT = state.dataHeatBalFanSys->MAT(ZoneNum);
}
Copy link
Member

Choose a reason for hiding this comment

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

You can do something similar here.

Comment on lines 5751 to 5758
// Inside face conduction calculation for Kiva surfaces
if (surface.HeatTransferAlgorithm == DataSurfaces::HeatTransferModel::Kiva) {
state.dataHeatBalSurf->SurfOpaqInsFaceCondFlux(surfNum) =
-(state.dataHeatBalSurf->SurfQdotConvInPerArea(surfNum) + state.dataHeatBalSurf->SurfQdotRadNetLWInPerArea(surfNum) +
state.dataHeatBalSurf->SurfQdotRadHVACInPerArea(surfNum) + state.dataHeatBal->SurfQdotRadIntGainsInPerArea(surfNum) +
state.dataHeatBalSurf->SurfQdotRadSolarInRepPerArea(surfNum) + state.dataHeatBalSurf->SurfQdotRadLightsInPerArea(surfNum));
}

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 this should happen in UpdateIntermediateSurfaceHeatBalanceResults. I also think you can use SurfOpaqQRadSWInAbs instead of SurfQdotRadSolarInRepPerArea + SurfQdotRadLightsInPerArea.

You also should remove the calculation of SurfOpaqInsFaceCondFlux for Kiva surfaces in CalcHeatBalanceInsideSurf2.

@@ -2085,7 +2085,7 @@ namespace ThermalComfort {
return CalcAngleFactorMRT;
}

Real64 CalcSurfaceWeightedMRT(EnergyPlusData &state, int const ZoneNum, int const SurfNum)
Real64 CalcSurfaceWeightedMRT(EnergyPlusData &state, int const ZoneNum, int const SurfNum, bool AverageWithSurface = true)
Copy link
Member

Choose a reason for hiding this comment

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

You should modify the purpose description in the comment below to reflect the change here.

@matthew-larson matthew-larson marked this pull request as ready for review March 3, 2022 00:17
@mjwitte
Copy link
Contributor

mjwitte commented Mar 15, 2022

@matthew-larson Is this ready for final review?

@nealkruis
Copy link
Member

Yes, I think this one is ready.

@mjwitte
Copy link
Contributor

mjwitte commented Mar 16, 2022

@nealkruis This branch has conflicts now. An attempt to merge and run unit tests locally shows a couple of unit tests failing. Not sure if it's from incorrect conflict resolution on my part or some other issue. So, leaving this for you to update.

@mitchute
Copy link
Collaborator

I merged develop in on this work locally, ran unit tests, and ran regression tests. Unit tests all run and pass, and the regression tests are consistent with the previous runs. I'm good with this going in right away. I'll hold it for 20 minutes or so, then merge unless someone chimes in.

@mitchute mitchute merged commit bb97cef into NREL:develop Mar 18, 2022
@matthew-larson matthew-larson deleted the kiva-conduction branch March 23, 2022 20:16
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 NotIDDChange Code does not impact IDD (can be merged after IO freeze)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Kiva zone heat balance around inner surface boundary using conduction not balancing
10 participants