-
Notifications
You must be signed in to change notification settings - Fork 400
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
Fix HVAC radiant heat gain with CondFD #10310
Conversation
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 walkthru.
@@ -89,6 +89,15 @@ namespace DataHeatBalFanSys { | |||
int DualPMVErrIndex = 0; // Dual PMV setpoint error index | |||
}; | |||
|
|||
struct SurfQRadFromHVACData |
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.
New struct to hold the old Array1Ds for SurfQHTRadSys, SurfQHWBaseboard, etc.
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.
:) :) :)
bool anySurfacesWithoutSpace = false; // True if any surfaces in a zone do not have a space assigned in input | ||
bool anySurfacesWithSpace = false; // True if any surfaces in a zone have a space assigned in input |
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.
Unrelated to the main defect, but moved these out of 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.
👍
std::vector<int> allInsideSourceSurfaceList; // List of all surfaces with SurfaceProperty:HeatBalanceSourceTerm for inside face | ||
std::vector<int> allOutsideSourceSurfaceList; // List of all surfaces with SurfaceProperty:HeatBalanceSourceTerm for outside face | ||
std::vector<int> allGetsRadiantHeatSurfaceList; // List of all surfaces that receive radiant HVAC output |
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.
New surface lists for special cases.
@@ -409,6 +409,7 @@ namespace ElectricBaseboardRadiator { | |||
} | |||
if (elecBaseboard.SurfacePtr(SurfNum) != 0) { | |||
state.dataSurface->surfIntConv(elecBaseboard.SurfacePtr(SurfNum)).getsRadiantHeat = true; | |||
state.dataSurface->allGetsRadiantHeatSurfaceList.emplace_back(elecBaseboard.SurfacePtr(SurfNum)); |
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.
Example of building the allGetsRadiantHeatSurfaceList
vector.
state.dataHeatBalFanSys->SurfQElecBaseboard = 0.0; | ||
for (auto &elecBaseboard : state.dataElectBaseboardRad->ElecBaseboard) { | ||
for (int radSurfNum = 1; radSurfNum <= elecBaseboard.TotSurfToDistrib; ++radSurfNum) { | ||
int surfNum = elecBaseboard.SurfacePtr(radSurfNum); | ||
state.dataHeatBalFanSys->surfQRadFromHVAC(surfNum).ElecBaseboard = 0.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.
Instead of zeroing this for all surfaces, only need to touch the surfaces that actually receive radiant heat from electric baseboards. Similar changes for the other four equipment types (chilled panel, HW baseboard, Steam baseboard, high temp radiant).
@@ -7571,11 +7568,12 @@ void CalcHeatBalanceInsideSurf(EnergyPlusData &state, | |||
state.dataHeatBalSurfMgr->calcHeatBalInsideSurEnvrnFlag = true; | |||
} | |||
|
|||
sumSurfQdotRadHVAC(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.
This is the main fix for the defect. A new function to gather all of the radiant impacts on the pertinent surfaces. One call here no matter the surface type or the heat transfer algorithm.
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.
👍
EnergyPlus::ScheduleManager::GetCurrentScheduleValue(state, state.dataSurface->Surface(SurfNum).InsideHeatSourceTermSchedule); | ||
} | ||
} | ||
for (int surfNum : state.dataSurface->allInsideSourceSurfaceList) { |
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.
Streamline inside source.
if (state.dataHeatBalSurf->AnyRadiantSystems(SurfNum)) | ||
state.dataHeatBalSurf->SurfQdotRadHVACInPerArea(SurfNum) = GetSurfQdotRadHVACInPerArea(state, SurfNum); |
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.
Replaced with new call to sumSurfQdotRadHVAC
, here and several other places.
return state.dataHeatBalFanSys->SurfQHTRadSys(SurfNum) + state.dataHeatBalFanSys->SurfQHWBaseboard(SurfNum) + | ||
state.dataHeatBalFanSys->SurfQSteamBaseboard(SurfNum) + state.dataHeatBalFanSys->SurfQElecBaseboard(SurfNum) + | ||
state.dataHeatBalFanSys->SurfQCoolingPanel(SurfNum); | ||
for (int surfNum : state.dataSurface->allGetsRadiantHeatSurfaceList) { |
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.
New function sumSurfQdotRadHVAC
sums radiant fluxes only for pertinent surfaces.
EPVector<bool> anySurfacesWithSpace; // True if any surfaces in a zone do not have a space assigned in input | ||
EPVector<bool> anySurfacesWithoutSpace; // True if any surfaces in a zone have a space assigned in input |
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 from state. Not related to the defect.
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.
❤️
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.
This is wonderful. Quite a few small cleanups to go along with the actual defect fix. I am getting a little blurry about all the different lists of surfaces, but I understand it is helping the code move in the right direction. I would be happy to brainstorm where these should go in the future to avoid the list becoming excessive, but I am not complaining about where it is at.
I'll go ahead and confirm CI, and build/test locally, and see if this can go in.
@@ -89,6 +89,15 @@ namespace DataHeatBalFanSys { | |||
int DualPMVErrIndex = 0; // Dual PMV setpoint error index | |||
}; | |||
|
|||
struct SurfQRadFromHVACData |
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.
:) :) :)
bool anySurfacesWithoutSpace = false; // True if any surfaces in a zone do not have a space assigned in input | ||
bool anySurfacesWithSpace = false; // True if any surfaces in a zone have a space assigned in input |
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.
👍
thisSurfQRadFromHVAC.SteamBaseboard = 0.0; | ||
thisSurfQRadFromHVAC.ElecBaseboard = 0.0; | ||
thisSurfQRadFromHVAC.CoolingPanel = 0.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.
This is fine as is. But if this is actually zeroing out every member variable, it could be nice to include a "reset" function on the struct that zeroes out all the members there. It's not functionally different, but could future proof things. If we add more member variables to the struct in the future, having the reset function right there in the struct would make it easier to remember to initialize them. Don't sweat it now unless there are more changes needed.
@@ -7571,11 +7568,12 @@ void CalcHeatBalanceInsideSurf(EnergyPlusData &state, | |||
state.dataHeatBalSurfMgr->calcHeatBalInsideSurEnvrnFlag = true; | |||
} | |||
|
|||
sumSurfQdotRadHVAC(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.
👍
EPVector<bool> anySurfacesWithSpace; // True if any surfaces in a zone do not have a space assigned in input | ||
EPVector<bool> anySurfacesWithoutSpace; // True if any surfaces in a zone have a space assigned in input |
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.
❤️
CI was totally happy here when it ran. I built and tested locally and it's happy. Let's get this merged in as well. Thanks @mjwitte ! |
Pull request overview
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.