Skip to content

Avoid duplicate view factor calculations (Plan B) #11364

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.

Sign up for GitHub

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

Merged
mitchute merged 13 commits into from
Jan 12, 2026
+104 −98
Merged

Avoid duplicate view factor calculations (Plan B) #11364

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
2e877f4
InitInteriorRadExchange efficiencies
mjwitte Mar 5, 2024
aefcf34
Tweak useSolarViewFactors
mjwitte May 20, 2024
9fbc815
Merge remote-tracking branch 'remotes/origin/develop' into avoidDupVF…
mjwitte Nov 25, 2025
8b110fd
Fix unit test
mjwitte Nov 25, 2025
8d3bf6f
Merge remote-tracking branch 'remotes/origin/develop' into avoidDupVF…
mjwitte Dec 9, 2025
ec49778
Copy solar view factors unless CarrollMRT or UseRepresentativeSurface…
mjwitte Dec 9, 2025
a9704b4
Merge remote-tracking branch 'remotes/origin/develop' into avoiddupVF…
mjwitte Dec 16, 2025
ffc52e4
Fix misspelled name for AnyShadingControlInModel
mjwitte Dec 17, 2025
6c617dd
Recalc radiant view factors if any inside light shelf
mjwitte Dec 17, 2025
4989108
Added comment
mjwitte Dec 17, 2025
cdc5cfd
Allow for possible user view factors when useSolarViewFactors is false
mjwitte Dec 17, 2025
187f00d
Merge branch 'develop' into avoiddupVFCalcsPlanB
mitchute Jan 7, 2026
d94d5bc
Merge branch 'develop' into avoiddupVFCalcsPlanB
mitchute Jan 10, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/EnergyPlus/DataGlobals.hh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,8 @@ struct DataGlobal : BaseGlobalStruct
bool AnyEnergyManagementSystemInModel = false; // true if there is any EMS or Erl in model. otherwise false
bool AnySurfPropOverridesInModel = false; // true if there is any EMS or Erl overriding the surface properties for any surface.
bool AnyConstrOverridesInModel = false; // true if there is any EMS or Erl overriding the constructions for any surface.
bool AndShadingControlInModel = false; // true if there is any window shading control for any fenestration surface
bool AnyShadingControlInModel = false; // true if there is any window shading control for any fenestration surface
bool AnyInsideShelf = false; // true if these is any inside daylighting shelf
Comment on lines -152 to +153
Copy link
Contributor Author

@mjwitte mjwitte Dec 17, 2025

Choose a reason for hiding this comment

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

Add new flag to track if there are any inside light shelves.
Fixed the name of "AndShadingControlInModel" to "AnyShading...."

bool AnyLocalEnvironmentsInModel = false; // true if there is any local environmental data objected defined in model, otherwise false
bool AnyPlantInModel = false; // true if there are any plant or condenser loops in model, otherwise false
bool AnyIdealCondEntSetPointInModel = false; // true if there is any ideal condenser entering set point manager in model.
Expand Down
1 change: 1 addition & 0 deletions src/EnergyPlus/DaylightingDevices.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -377,6 +377,7 @@ namespace Dayltg {
if (ShelfSurf > 0) {
// Double surface area so that both sides of the shelf are treated as internal mass
state.dataSurface->Surface(ShelfSurf).Area *= 2.0;
state.dataGlobal->AnyInsideShelf = true; // Set this to force recalc for radiant view factors due to area change
Copy link
Contributor Author

@mjwitte mjwitte Dec 17, 2025

Choose a reason for hiding this comment

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

Set the flag here. This is where the shelf area gets doubled (after InitSolarViewFactors and before InitInteriorRadExchange).

}

ShelfSurf = state.dataDaylightingDevicesData->Shelf(ShelfNum).OutSurf;
Expand Down
182 changes: 99 additions & 83 deletions src/EnergyPlus/HeatBalanceIntRadExchange.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -459,15 +459,13 @@ namespace HeatBalanceIntRadExchange {

// SUBROUTINE LOCAL VARIABLE DECLARATIONS:
static constexpr std::string_view RoutineName("InitInteriorRadExchange: ");
bool NoUserInputF; // Logical flag signifying no input F's for zone
bool ErrorsFound(false);
Real64 CheckValue1;
Real64 CheckValue2;
Real64 FinalCheckValue;
Real64 CheckValue1 = 0.0;
Real64 CheckValue2 = 0.0;
Real64 FinalCheckValue = 0.0;
Array2D<Real64> SaveApproximateViewFactors; // Save for View Factor reporting
Real64 RowSum;
Real64 FixedRowSum;
int NumIterations;
Real64 FixedRowSum = 0.0;
int NumIterations = 0;
std::string Option1; // view factor report option

auto &ViewFactorReport = state.dataHeatBalIntRadExchg->ViewFactorReport;
Expand Down Expand Up @@ -524,8 +522,10 @@ namespace HeatBalanceIntRadExchange {
thisEnclosure.Emissivity.dimension(numEnclosureSurfaces, 0.0);
thisEnclosure.Azimuth.dimension(numEnclosureSurfaces, 0.0);
thisEnclosure.Tilt.dimension(numEnclosureSurfaces, 0.0);
thisEnclosure.Fp.dimension(numEnclosureSurfaces, 1.0);
thisEnclosure.FMRT.dimension(numEnclosureSurfaces, 0.0);
if (state.dataHeatBalIntRadExchg->CarrollMethod) {
thisEnclosure.Fp.dimension(numEnclosureSurfaces, 1.0);
thisEnclosure.FMRT.dimension(numEnclosureSurfaces, 0.0);
Comment on lines +525 to +527
Copy link
Contributor Author

@mjwitte mjwitte Dec 17, 2025

Choose a reason for hiding this comment

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

These are only used for CarrollMethod.

Copy link
Collaborator

@mitchute mitchute Dec 18, 2025

Choose a reason for hiding this comment

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

And the statement is just to save initialization when not needed? The values you are initializing to don't seem to matter since they are not used until they are set below. That's fine if correct. Just noting it. Let me know otherwise.

Copy link
Contributor Author

@mjwitte mjwitte Dec 18, 2025

Choose a reason for hiding this comment

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

Fp and FMRT are not even dimensioned now unless the CarrollMethod is being used. Minor, but seems reasonable, esp for a large model.

Copy link
Collaborator

@mitchute mitchute Dec 18, 2025

Choose a reason for hiding this comment

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

Right. Make sense.

}
thisEnclosure.SurfacePtr.dimension(numEnclosureSurfaces, 0);

// Initialize the enclosure surface arrays
Expand Down Expand Up @@ -577,8 +577,10 @@ namespace HeatBalanceIntRadExchange {
// If there is only one surface in a zone, then there is no radiant exchange
thisEnclosure.F = 0.0;
thisEnclosure.ScriptF = 0.0;
thisEnclosure.Fp = 0.0;
thisEnclosure.FMRT = 0.0;
if (state.dataHeatBalIntRadExchg->CarrollMethod) {
thisEnclosure.Fp = 0.0;
thisEnclosure.FMRT = 0.0;
}
if (state.dataGlobal->DisplayAdvancedReportVariables) {
print(state.files.eio, "Surface View Factor Check Values,{},0,0,0,-1,0,0\n", thisEnclosure.Name);
}
Expand All @@ -596,53 +598,63 @@ namespace HeatBalanceIntRadExchange {
CalcFMRT(state, thisEnclosure.NumOfSurfaces, thisEnclosure.Area, thisEnclosure.FMRT);
CalcFp(thisEnclosure.NumOfSurfaces, thisEnclosure.Emissivity, thisEnclosure.FMRT, thisEnclosure.Fp);
} else {
// Get user supplied view factors if available in idf.

NoUserInputF = true;
// For radiant enclosures:
// If UseRepresentativeSurfaceCalculations is true and there are no user input view factors, then calc approx view factors
// (If User supplied view factors are present, then UseRepresentativeSurfaceCalculations is skipped in SufaceGeometry::GetSurfaceData)
// If there are any inside light shelfs then calc approx view factors (because the light shelf area is doubled for heat transfer)
// Otherwise, copy final view factors from the solar enclosure
Comment on lines 601 to 605
Copy link
Contributor Author

@mjwitte mjwitte Dec 17, 2025

Choose a reason for hiding this comment

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

These are the new rules to decide if the solar view factors can be used for radiant exchange or if they need to be recalculated.
oops - need to add a line about the daylighting inside shelf.

Copy link
Contributor Author

@mjwitte mjwitte Dec 17, 2025

Choose a reason for hiding this comment

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

Added.


constexpr std::string_view cCurrentModuleObject = "ZoneProperty:UserViewFactors:BySurfaceName";
int NumZonesWithUserFbyS = state.dataInputProcessing->inputProcessor->getNumObjectsFound(state, cCurrentModuleObject);
if (NumZonesWithUserFbyS > 0) {

GetInputViewFactorsbyName(state,
Copy link
Contributor Author

@mjwitte mjwitte Dec 17, 2025

Choose a reason for hiding this comment

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

These are gotten in InitSolarViewFactors so no need to get them again.
Hmm, but what if there are user input view factors and an inside light shelf? Argh. May need to revert this part.

Copy link
Contributor Author

@mjwitte mjwitte Dec 17, 2025

Choose a reason for hiding this comment

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

Fixed, see below.

thisEnclosure.Name,
thisEnclosure.NumOfSurfaces,
thisEnclosure.F,
thisEnclosure.SurfacePtr,
NoUserInputF,
ErrorsFound); // Obtains user input view factors from input file
}

if (NoUserInputF) {
bool useSolarViewFactors = ((!state.dataSurface->UseRepresentativeSurfaceCalculations || NumZonesWithUserFbyS > 0) &&
!state.dataGlobal->AnyInsideShelf && !state.dataViewFactor->EnclSolInfo(enclosureNum).F.empty());
Copy link
Collaborator

@rraustad rraustad Jan 7, 2026

Choose a reason for hiding this comment

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

This is where the new flag AnyInsideShelf is used to avoid a new view factor calculation. At least that part of the change is apparent.


// Calculate the view factors and make sure they satisfy reciprocity
CalcApproximateViewFactors(state,
thisEnclosure.NumOfSurfaces,
thisEnclosure.Area,
thisEnclosure.Azimuth,
thisEnclosure.Tilt,
thisEnclosure.F,
thisEnclosure.SurfacePtr);
}
if (useSolarViewFactors) {
thisEnclosure.F = state.dataViewFactor->EnclSolInfo(enclosureNum).F;
} else {
bool NoUserInputF = true;
if (NumZonesWithUserFbyS > 0) {

GetInputViewFactorsbyName(state,
thisEnclosure.Name,
thisEnclosure.NumOfSurfaces,
thisEnclosure.F,
thisEnclosure.SurfacePtr,
NoUserInputF,
ErrorsFound); // Obtains user input view factors from input file
}

if (ViewFactorReport) { // Allocate and save user or approximate view factors for reporting.
SaveApproximateViewFactors.allocate(thisEnclosure.NumOfSurfaces, thisEnclosure.NumOfSurfaces);
SaveApproximateViewFactors = thisEnclosure.F;
}
if (NoUserInputF) {
// Calculate the view factors and make sure they satisfy reciprocity
CalcApproximateViewFactors(state,
thisEnclosure.NumOfSurfaces,
thisEnclosure.Area,
thisEnclosure.Azimuth,
thisEnclosure.Tilt,
thisEnclosure.F,
thisEnclosure.SurfacePtr);
}

bool anyIntMassInZone = DoesZoneHaveInternalMass(state, thisEnclosure.NumOfSurfaces, thisEnclosure.SurfacePtr);
FixViewFactors(state,
thisEnclosure.NumOfSurfaces,
thisEnclosure.Area,
thisEnclosure.F,
thisEnclosure.Name,
thisEnclosure.spaceNums,
CheckValue1,
CheckValue2,
FinalCheckValue,
NumIterations,
FixedRowSum,
anyIntMassInZone);
if (ViewFactorReport) { // Allocate and save user or approximate view factors for reporting.
SaveApproximateViewFactors.allocate(thisEnclosure.NumOfSurfaces, thisEnclosure.NumOfSurfaces);
SaveApproximateViewFactors = thisEnclosure.F;
}

bool anyIntMassInZone = DoesZoneHaveInternalMass(state, thisEnclosure.NumOfSurfaces, thisEnclosure.SurfacePtr);
FixViewFactors(state,
thisEnclosure.NumOfSurfaces,
thisEnclosure.Area,
thisEnclosure.F,
thisEnclosure.Name,
thisEnclosure.spaceNums,
CheckValue1,
CheckValue2,
FinalCheckValue,
NumIterations,
FixedRowSum,
anyIntMassInZone);
}

// Calculate the script F factors
CalcScriptF(state, thisEnclosure.NumOfSurfaces, thisEnclosure.Area, thisEnclosure.F, thisEnclosure.Emissivity, thisEnclosure.ScriptF);
Expand All @@ -668,24 +680,26 @@ namespace HeatBalanceIntRadExchange {
print(state.files.eio, "\n");
}

print(state.files.eio, "Approximate or User Input ViewFactors,To Surface,Surface Class,RowSum");
for (int SurfNum : thisEnclosure.SurfaceReportNums) {
print(state.files.eio, ",{}", state.dataSurface->Surface(thisEnclosure.SurfacePtr(SurfNum)).Name);
}
print(state.files.eio, "\n");

for (int Findex : thisEnclosure.SurfaceReportNums) {
RowSum = sum(SaveApproximateViewFactors(_, Findex));
print(state.files.eio,
"{},{},{},{:.4R}",
"View Factor",
state.dataSurface->Surface(thisEnclosure.SurfacePtr(Findex)).Name,
cSurfaceClass(state.dataSurface->Surface(thisEnclosure.SurfacePtr(Findex)).Class),
RowSum);
if (!useSolarViewFactors) {
print(state.files.eio, "Approximate or User Input ViewFactors,To Surface,Surface Class,RowSum");
for (int SurfNum : thisEnclosure.SurfaceReportNums) {
print(state.files.eio, ",{:.4R}", SaveApproximateViewFactors(SurfNum, Findex));
print(state.files.eio, ",{}", state.dataSurface->Surface(thisEnclosure.SurfacePtr(SurfNum)).Name);
}
print(state.files.eio, "\n");

for (int Findex : thisEnclosure.SurfaceReportNums) {
Real64 RowSum = sum(SaveApproximateViewFactors(_, Findex));
print(state.files.eio,
"{},{},{},{:.4R}",
"View Factor",
state.dataSurface->Surface(thisEnclosure.SurfacePtr(Findex)).Name,
cSurfaceClass(state.dataSurface->Surface(thisEnclosure.SurfacePtr(Findex)).Class),
RowSum);
for (int SurfNum : thisEnclosure.SurfaceReportNums) {
print(state.files.eio, ",{:.4R}", SaveApproximateViewFactors(SurfNum, Findex));
}
print(state.files.eio, "\n");
}
}

print(state.files.eio, "Final ViewFactors,To Surface,Surface Class,RowSum");
Expand All @@ -695,7 +709,7 @@ namespace HeatBalanceIntRadExchange {
print(state.files.eio, "\n");

for (int Findex : thisEnclosure.SurfaceReportNums) {
RowSum = sum(thisEnclosure.F(_, Findex));
Real64 RowSum = sum(thisEnclosure.F(_, Findex));
print(state.files.eio,
"{},{},{},{:.4R}",
"View Factor",
Expand Down Expand Up @@ -767,22 +781,24 @@ namespace HeatBalanceIntRadExchange {
SaveApproximateViewFactors.deallocate();
}

RowSum = 0.0;
for (int Findex : thisEnclosure.SurfaceReportNums) {
RowSum += sum(thisEnclosure.F(_, Findex));
}
RowSum = std::abs(RowSum - thisEnclosure.NumOfSurfaces);
FixedRowSum = std::abs(FixedRowSum - thisEnclosure.NumOfSurfaces);
if (state.dataGlobal->DisplayAdvancedReportVariables) {
print(state.files.eio,
"Surface View Factor Check Values,{},{:.6R},{:.6R},{:.6R},{},{:.6R},{:.6R}\n",
thisEnclosure.Name,
CheckValue1,
CheckValue2,
FinalCheckValue,
NumIterations,
FixedRowSum,
RowSum);
if (!useSolarViewFactors) {
Real64 RowSum = 0.0;
for (int Findex : thisEnclosure.SurfaceReportNums) {
RowSum += sum(thisEnclosure.F(_, Findex));
}
RowSum = std::abs(RowSum - thisEnclosure.NumOfSurfaces);
FixedRowSum = std::abs(FixedRowSum - thisEnclosure.NumOfSurfaces);
if (state.dataGlobal->DisplayAdvancedReportVariables) {
print(state.files.eio,
"Surface View Factor Check Values,{},{:.6R},{:.6R},{:.6R},{},{:.6R},{:.6R}\n",
thisEnclosure.Name,
CheckValue1,
CheckValue2,
FinalCheckValue,
NumIterations,
FixedRowSum,
RowSum);
}
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/EnergyPlus/SolarShading.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10287,7 +10287,7 @@ void CheckGlazingShadingStatusChange(EnergyPlusData &state)
for (int enclosureNum = 1; enclosureNum <= state.dataViewFactor->NumOfRadiantEnclosures; ++enclosureNum) {
state.dataViewFactor->EnclRadInfo(enclosureNum).radReCalc = false;
}
if (!state.dataGlobal->AndShadingControlInModel) {
if (!state.dataGlobal->AnyShadingControlInModel) {
return;
}
for (int enclosureNum = 1; enclosureNum <= state.dataViewFactor->NumOfSolarEnclosures; ++enclosureNum) {
Expand Down
2 changes: 1 addition & 1 deletion src/EnergyPlus/SurfaceGeometry.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10089,7 +10089,7 @@ namespace SurfaceGeometry {
int curShadedConstruction = state.dataSurface->WindowShadingControl(iShadeCtrl).getInputShadedConstruction;
for (int jFeneRef = 1; jFeneRef <= state.dataSurface->WindowShadingControl(iShadeCtrl).FenestrationCount; ++jFeneRef) {
if (Util::SameString(state.dataSurface->WindowShadingControl(iShadeCtrl).FenestrationName(jFeneRef), surfTemp.Name)) {
state.dataGlobal->AndShadingControlInModel = true;
state.dataGlobal->AnyShadingControlInModel = true;
surfTemp.HasShadeControl = true;
surfTemp.windowShadingControlList.push_back(iShadeCtrl);
surfTemp.activeWindowShadingControl = iShadeCtrl;
Expand Down
12 changes: 0 additions & 12 deletions tst/EnergyPlus/unit/HeatBalanceAirManager.unit.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -849,18 +849,6 @@ TEST_F(EnergyPlusFixture, HeatBalanceAirManager_GetMixingAndCrossMixing)
" ** ~~~ ** So, when there are three or less surfaces in a zone, EnergyPlus will make sure there are no losses of energy but",
" ** ~~~ ** it will not exchange the full amount of radiation with the rest of the zone as it would if there was a completed "
"enclosure.",
" ** Warning ** CalcApproximateViewFactors: Zero area for all other zone surfaces.", // Why are these errors being reported twice, because
// something is being calculated twice?
" ** ~~~ ** Happens for Surface=\"DUMMY SPACE 1A FLOOR\" in Zone=ZONE 1",
" ** Warning ** CalcApproximateViewFactors: Zero area for all other zone surfaces.",
" ** ~~~ ** Happens for Surface=\"DUMMY SPACE 1B FLOOR\" in Zone=ZONE 1",
" ** Warning ** Surfaces in Zone/Enclosure=\"ZONE 1\" do not define an enclosure.",
" ** ~~~ ** Number of surfaces <= 3, view factors are set to force reciprocity but may not fulfill completeness.",
" ** ~~~ ** Reciprocity means that radiant exchange between two surfaces will match and not lead to an energy loss.",
" ** ~~~ ** Completeness means that all of the view factors between a surface and the other surfaces in a zone add up to unity.",
" ** ~~~ ** So, when there are three or less surfaces in a zone, EnergyPlus will make sure there are no losses of energy but",
" ** ~~~ ** it will not exchange the full amount of radiation with the rest of the zone as it would if there was a completed "
"enclosure.",
" ************* Testing Individual Branch Integrity",
Copy link
Collaborator

@mitchute mitchute Dec 18, 2025

Choose a reason for hiding this comment

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

Nice to have the duplicate error message gone.

" ************* All Branches passed integrity testing",
" ************* Testing Individual Supply Air Path Integrity",
Expand Down
Loading