Avoid duplicate view factor calculations (Plan B) by mjwitte · Pull Request #11364 · NatLabRockies/EnergyPlus

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.

rraustad · 2026-01-07T02:12:05Z

src/EnergyPlus/HeatBalanceIntRadExchange.cc


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


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.

github-actions · 2026-01-08T00:34:19Z

⚠️ Regressions detected on macos-14 for commit `57b943a`

Regression Summary

EIO: 28
Table Big Diffs: 20
ERR: 1

github-actions · 2026-01-08T00:34:36Z

⚠️ Regressions detected on ubuntu-24.04 for commit `57b943a`

Regression Summary

EIO: 28
Table Big Diffs: 20
ERR: 1

rraustad · 2026-01-09T16:33:29Z

@mitchute do you know what is causing the linux UnitTestsCoverage CI failure?

mitchute · 2026-01-09T16:36:01Z

@rraustad not currently. I did build it on my personal linux box in this config yesterday, and it all was successful. I haven't had the chance to try it on the CI machines directly yet.

github-actions · 2026-01-10T18:56:18Z

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

Regression Summary

EIO: 28
Table Big Diffs: 20
ERR: 1

github-actions · 2026-01-10T19:12:12Z

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

Regression Summary

EIO: 28
Table Big Diffs: 20
ERR: 1

mitchute · 2026-01-12T15:57:05Z

@rraustad I didn't change anything related to CI, so I guess there was something else going on. We'll merge this now.

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

rraustad reviewed Jan 7, 2026

View reviewed changes

Merge branch 'develop' into avoiddupVFCalcsPlanB

187f00d

Merge branch 'develop' into avoiddupVFCalcsPlanB

d94d5bc

mitchute merged commit f608cce into develop Jan 12, 2026
11 checks passed

mitchute deleted the avoiddupVFCalcsPlanB branch January 12, 2026 15:57

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!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Jan 8, 2026

⚠️ Regressions detected on macos-14 for commit 57b943a

Uh oh!

github-actions bot commented Jan 8, 2026

⚠️ Regressions detected on ubuntu-24.04 for commit 57b943a

Uh oh!

rraustad commented Jan 9, 2026

Uh oh!

mitchute commented Jan 9, 2026

Uh oh!

github-actions bot commented Jan 10, 2026

⚠️ Regressions detected on macos-14 for commit c705727

Uh oh!

github-actions bot commented Jan 10, 2026

⚠️ Regressions detected on ubuntu-24.04 for commit c705727

Uh oh!

mitchute commented Jan 12, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

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`

⚠️ Regressions detected on macos-14 for commit `57b943a`

⚠️ Regressions detected on ubuntu-24.04 for commit `57b943a`

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

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