Avoid duplicate view factor calculations (Plan B) #11364

mjwitte · 2025-12-09T23:37:56Z

Pull request overview

Prevents duplicate view factor calculations when they would be identical for both the solar and radiant enclosure calculations.
There are two special cases where the view factors are different for radiant vs solar enclosures:
- When PerformancePrecisionTradeoffs "Use Representative Surfaces for Calculations" is Yes, because the solar calcs use all surfaces and the radiant calcs use only the representative surfaces.
- When there are any DaylightingDevice:Shelf "Inside Shelf Name", because the inside shelf area is doubled for heat transfer calculations, but not for solar distribution.

Pull Request Author

Reviewer

…Calcs

…Calculations

github-actions · 2025-12-10T00:02:05Z

⚠️ Regressions detected on ubuntu-24.04 for commit `8b6adab`

Regression Summary

EIO: 29
Table Big Diffs: 20
ERR: 1
ESO Small Diffs: 1
Table Small Diffs: 1

github-actions · 2025-12-10T00:04:44Z

⚠️ Regressions detected on macos-14 for commit `8b6adab`

Regression Summary

EIO: 29
Table Big Diffs: 20
ERR: 1
ESO Small Diffs: 1
Table Small Diffs: 1

…CalcsPlanB

mjwitte

Code walk-thru

mjwitte · 2025-12-17T16:47:28Z

src/EnergyPlus/DataGlobals.hh

-    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


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

mjwitte · 2025-12-17T16:48:51Z

src/EnergyPlus/DaylightingDevices.cc

            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


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

mjwitte · 2025-12-17T16:49:29Z

src/EnergyPlus/HeatBalanceIntRadExchange.cc

+            if (state.dataHeatBalIntRadExchg->CarrollMethod) {
+                thisEnclosure.Fp.dimension(numEnclosureSurfaces, 1.0);
+                thisEnclosure.FMRT.dimension(numEnclosureSurfaces, 0.0);


These are only used for CarrollMethod.

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.

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

Right. Make sense.

mjwitte · 2025-12-17T16:50:56Z

src/EnergyPlus/HeatBalanceIntRadExchange.cc

+                // 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)
+                // Otherwise, copy final view factors from the solar enclosure


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.

mjwitte · 2025-12-17T16:52:49Z

src/EnergyPlus/HeatBalanceIntRadExchange.cc

                int NumZonesWithUserFbyS = state.dataInputProcessing->inputProcessor->getNumObjectsFound(state, cCurrentModuleObject);
-                if (NumZonesWithUserFbyS > 0) {
-
-                    GetInputViewFactorsbyName(state,


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.

Fixed, see below.

mjwitte · 2025-12-17T16:55:29Z

src/EnergyPlus/HeatBalanceIntRadExchange.cc

+                bool useSolarViewFactors = ((!state.dataSurface->UseRepresentativeSurfaceCalculations || NumZonesWithUserFbyS > 0) &&
+                                            !state.dataGlobal->AnyInsideShelf && !state.dataViewFactor->EnclSolInfo(enclosureNum).F.empty());

+                if (useSolarViewFactors) {
+                    thisEnclosure.F = state.dataViewFactor->EnclSolInfo(enclosureNum).F;
+                } else {
                    // Calculate the view factors and make sure they satisfy reciprocity
                    CalcApproximateViewFactors(state,


This is the heart of the change. Either copy the solar view factors or calculate approximate view factors. Plus some later reporting that is skipped when useSolarViewFactors is true.

github-actions · 2025-12-17T17:22:29Z

⚠️ Regressions detected on macos-14 for commit `b84e7b8`

Regression Summary

EIO: 28
Table Big Diffs: 20
ERR: 1

github-actions · 2025-12-17T17:27:16Z

⚠️ Regressions detected on ubuntu-24.04 for commit `b84e7b8`

Regression Summary

EIO: 28
Table Big Diffs: 20
ERR: 1

mjwitte · 2025-12-17T21:59:38Z

A tiny performance improvement.

eso diffs are gone now. eio, table and err diffs are as expected.

github-actions · 2025-12-17T23:06:43Z

⚠️ Regressions detected on macos-14 for commit `ddd6005`

Regression Summary

EIO: 28
Table Big Diffs: 20
ERR: 1

github-actions · 2025-12-17T23:06:46Z

⚠️ Regressions detected on ubuntu-24.04 for commit `ddd6005`

Regression Summary

EIO: 28
Table Big Diffs: 20
ERR: 1

mitchute · 2025-12-18T19:00:59Z

tst/EnergyPlus/unit/HeatBalanceAirManager.unit.cc

-         "   **   ~~~   ** 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",


Nice to have the duplicate error message gone.

mitchute

No further comments from me. Tests, regressions, and diffs all look good.

mjwitte added 6 commits March 5, 2024 14:32

InitInteriorRadExchange efficiencies

2e877f4

Tweak useSolarViewFactors

aefcf34

Merge remote-tracking branch 'remotes/origin/develop' into avoidDupVF…

9fbc815

…Calcs

Fix unit test

8b110fd

Merge remote-tracking branch 'remotes/origin/develop' into avoidDupVF…

8d3bf6f

…Calcs

Copy solar view factors unless CarrollMRT or UseRepresentativeSurface…

ec49778

…Calculations

mjwitte added the Performance Includes code changes that are directed at improving the runtime performance of EnergyPlus label Dec 9, 2025

mjwitte changed the title ~~Avoid duplicate view factor calculations~~ Avoid duplicate view factor calculations (Plan B) Dec 10, 2025

mjwitte added 3 commits December 16, 2025 16:04

Merge remote-tracking branch 'remotes/origin/develop' into avoiddupVF…

a9704b4

…CalcsPlanB

Fix misspelled name for AnyShadingControlInModel

ffc52e4

Recalc radiant view factors if any inside light shelf

6c617dd

mjwitte commented Dec 17, 2025

View reviewed changes

mjwitte marked this pull request as ready for review December 17, 2025 17:00

mjwitte mentioned this pull request Dec 17, 2025

Avoid duplicate view factor calculations #11352

Closed

25 tasks

mjwitte added 2 commits December 17, 2025 16:09

Added comment

4989108

Allow for possible user view factors when useSolarViewFactors is false

cdc5cfd

mitchute reviewed Dec 18, 2025

View reviewed changes

mitchute approved these changes Dec 18, 2025

View reviewed changes

Avoid duplicate view factor calculations (Plan B) #11364

Are you sure you want to change the base?

Avoid duplicate view factor calculations (Plan B) #11364

Uh oh!

Conversation

mjwitte commented Dec 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull request overview

Pull Request Author

Reviewer

Uh oh!

github-actions bot commented Dec 10, 2025

⚠️ Regressions detected on ubuntu-24.04 for commit 8b6adab

Uh oh!

github-actions bot commented Dec 10, 2025

⚠️ Regressions detected on macos-14 for commit 8b6adab

Uh oh!

mjwitte left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Dec 17, 2025

⚠️ Regressions detected on macos-14 for commit b84e7b8

Uh oh!

github-actions bot commented Dec 17, 2025

⚠️ Regressions detected on ubuntu-24.04 for commit b84e7b8

Uh oh!

mjwitte commented Dec 17, 2025

Uh oh!

github-actions bot commented Dec 17, 2025

⚠️ Regressions detected on macos-14 for commit ddd6005

Uh oh!

github-actions bot commented Dec 17, 2025

⚠️ Regressions detected on ubuntu-24.04 for commit ddd6005

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mitchute left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

mjwitte commented Dec 9, 2025 •

edited

Loading

⚠️ Regressions detected on ubuntu-24.04 for commit `8b6adab`

⚠️ Regressions detected on macos-14 for commit `8b6adab`

⚠️ Regressions detected on macos-14 for commit `b84e7b8`

⚠️ Regressions detected on ubuntu-24.04 for commit `b84e7b8`

⚠️ Regressions detected on macos-14 for commit `ddd6005`

⚠️ Regressions detected on ubuntu-24.04 for commit `ddd6005`