Add ability to automatically detect groups of similar surfaces for efficient heat balance calculations #8739

nealkruis · 2021-04-30T16:54:23Z

Pull request overview

One time saving technique used in EnergyPlus models is the use of Window Multipliers to reduce redundant heat balance calculations on multiple window surfaces. However, this practice requires users to modify their building geometry to be substantially different from the actual architectural features they are trying to represent. This work will explore, propose, and implement an approach where these simplifications can be inferred automatically from user input and performed behind-the-scenes so that models with many windows (and other heat transfer surfaces) can benefit from time-saving techniques without sacrificing geometric fidelity.

See the NFP here.

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.

nrel-bot-3 · 2021-05-29T00:23:40Z

@nealkruis @lgentile it has been 28 days since this pull request was last updated.

nrel-bot-2b · 2021-06-26T00:20:58Z

@nealkruis @lgentile it has been 28 days since this pull request was last updated.

nrel-bot-3 · 2021-07-24T00:20:54Z

@nealkruis @lgentile it has been 28 days since this pull request was last updated.

Myoldmopar · 2021-08-19T23:28:27Z

@nealkruis IIRC you were going to be doing something else to this branch. Adding/modifying the test file? I'm ready to test/review this just let me know. There appears to be a tiny conflict to resolve, I'm happy to do that, but if you want to pull develop in yourself that's fine too. I'm curious how this will play with the spaces PR....... (FYI @mjwitte)

nealkruis · 2021-08-20T14:22:21Z

@Myoldmopar yeah, I'll add an example file. I've been chatting with @mjwitte about the inevitable collision of our two PRs and need to make one other minor change to accommodate the fact that spaces can cause surfaces to be in separate enclosures within the same zone. I'll take care of that this morning.

nealkruis · 2021-08-20T16:40:02Z

@Myoldmopar this should be ready once the CI returns.

Myoldmopar · 2021-08-21T20:10:50Z

@nealkruis I'm not sure if you're working this weekend or not, but with spaces in, please go ahead and update this branch as soon as possible so we can do final review and get it merged.

mbadams5

Only one minor review comment.
Overall this looks good to me. I'll be curious how the conduction work on the windows improves performance.

mbadams5 · 2021-08-22T03:29:37Z

src/EnergyPlus/DataSurfaces.cc

+    // Make hash key for this surface (used to determine uniqueness)
+    state.dataSurface->Surface(SurfNum).make_hash_key(state, SurfNum);
+    // Insert surface key into map. If key already exists, it will not be added.
+    state.dataSurface->RepresentativeSurfaceMap.insert({state.dataSurface->Surface(SurfNum).calcHashKey, SurfNum});


Relatively minor in the scheme of the PR, but insert returns a "pair object whose first element is an iterator pointing either to the newly inserted element in the container or to the element whose key is equivalent". This means you can just use that returned iterator below state.dataSurface->Surface(SurfNum).RepresentativeCalcSurfNum = returned_pair.first;, instead of performing a find again.

amirroth · 2021-08-22T14:26:32Z

src/EnergyPlus/SurfaceGeometry.cc

+            state.dataSurface->Surface(SurfNum).RepresentativeCalcSurfNum = SurfNum;
+            // Automatic Surface Multipliers: Assign representative heat transfer surfaces
+            if (state.dataSurface->UseRepresentativeSurfaceCalculations && state.dataSurface->Surface(SurfNum).HeatTransSurf &&
+                state.dataInputProcessing->inputProcessor->getNumObjectsFound(state, "ZoneProperty:UserViewFactors:BySurfaceName") == 0) {


Probably shouldn't call this for every surface. Call once and set a flag. Better yet, split this loop into two and only do the second part if UseRepresentativeSurfaceCalculations is true and there are no user defined view factors.

mjwitte

@nealkruis This is an interesting approach. I haven't tested it yet to see what kind of impact it has. Some comments at the top of the PR would be useful if you've found a certain file to show x% speedup or % reduction in surfaces in the radiant enclosures. Nice that the changes are very self-contained.

A few comments/questions. Nothing here is a showstopper for I/O Freeze, any changes can happen after that.

mjwitte · 2021-08-23T16:59:08Z

doc/input-output-reference/src/overview/group-simulation-parameters.tex

@@ -1089,28 +1089,28 @@ \subsection{PerformancePrecisionTradeoffs}\label{performanceprecisiontradeoffs}

 \paragraph{Field: Override Mode}\label{override-mode}

-The Override Mode field provides a single field that will override other inputs located in the IDF/epJSON file as well as convergence related values that appear in the remaining fields of the PerformancePrecisionTradeoffs object. The Normal option (default) provides no overrides while the Mode options provide overrides based on the following tables. The Advanced option, allows the remaining fields of the PerformancePrecisionTradeoffs to be used. 
+The Override Mode field provides a single field that will override other inputs located in the IDF/epJSON file as well as convergence related values that appear in the remaining fields of the PerformancePrecisionTradeoffs object. The Normal option (default) provides no overrides while the Mode options provide overrides based on the following tables. The Advanced option, allows the remaining fields of the PerformancePrecisionTradeoffs to be used.


This needs to be more specific now. Only MaxZoneTempDiff and MaxAllowedDelTemp require Override Mode = Advanced. If I'm understanding correctly, the new field for "Use Representative Surfaces for Calculations" isn't affected by Override Mode.

mjwitte · 2021-08-23T17:04:14Z

doc/input-output-reference/src/overview/group-simulation-parameters.tex

+
+Surface are grouped based on the following criteria for similarity:
+
+\begin{itemize}


Adding \tightlist might be nice here (before the first item) to remove the space between each item - nice for short one-liner lists like this.

mjwitte · 2021-08-23T17:17:06Z

src/EnergyPlus/DataSurfaces.cc

+    calcHashKey.TAirRef = state.dataSurface->SurfTAirRef(SurfNum);
+
+    auto extBoundCond = state.dataSurface->Surface(SurfNum).ExtBoundCond;
+    if (extBoundCond > 0) {


This will add ExtZone and ExtEnclindex to the hash if it's an interzone surface, otherwise these are zero. I see checks later on for OSC and OSCM, but what about these outside boundary conditions:

\key Outdoors \key Foundation \key Ground \key GroundFCfactorMethod

Unless I'm missing something it looks like these will all have the same hash.

mjwitte · 2021-08-23T17:19:32Z

doc/input-output-reference/src/overview/group-simulation-parameters.tex

+    \item WindowProperty:AirflowControl (windows only)
+\end{itemize}
+
+Currently, the only calculations performed across all surfaces with a group is the interior radiation exchange, where surfaces within a group are collected into a single area for participating in interior longwave radiation exchange. This will reduce the scale and complexity of the algorithm with minimal impact on accuracy. Additional calculations may be performed for a group of surfaces in future releases.


Type - first "with" --> "within"

mjwitte · 2021-08-23T17:40:20Z

src/EnergyPlus/HeatBalanceIntRadExchange.cc

            int enclosureSurfNum = 0;
            for (int const spaceNum : thisEnclosure.spaceNums) {
                int priorZoneTotEnclSurfs = enclosureSurfNum;
                for (int surfNum : state.dataHeatBal->space(spaceNum).surfaces) {
                    if (state.dataSurface->Surface(surfNum).IsAirBoundarySurf) continue;
-                    ++enclosureSurfNum;
-                    thisEnclosure.SurfacePtr(enclosureSurfNum) = surfNum;
+                    // Automatic Surface Multipliers: Only include representative surfaces


If this loop only adds representative surfaces, can this for loop be skipped if !UseRepresentativeSurfaceCalculations?
Or maybe it's the next loop that isn't necessary if !UseRepresentativeSurfaceCalculations?
The usage for RepresentativeCalcSurfNum is a bit confusing. When I saw it declared in DataSurfaces, I assumed that RepresentativeCalcSurfNum would be zero for normal calculations, but it appears that's not the case.
If I'm following correctly, if UseRepresentativeSurfaceCalculations is off, then state.dataSurface->Surface(surfNum).RepresentativeCalcSurfNum = surfNum for all surfaces?
Might be worth an extra comment in DataSurfaces.

mjwitte · 2021-08-23T18:10:58Z

tst/EnergyPlus/unit/DataSurfaces.unit.cc

+    EXPECT_EQ(state->dataSurface->Surface(2).RepresentativeCalcSurfNum, 2);
+    EXPECT_EQ(state->dataSurface->Surface(3).RepresentativeCalcSurfNum, 3);
+    EXPECT_EQ(state->dataSurface->Surface(4).RepresentativeCalcSurfNum, 1);
+}


File could use one more return at the end.

mjwitte · 2021-08-23T18:41:00Z

Serial unit tests all pass.

Myoldmopar · 2021-08-23T20:42:15Z

This is a great addition. I'm going to merge this in to get us very close to IO freeze, but please open up a ticket right away to address the issues noted by @mjwitte. Thanks @nealkruis !

Add initial NFP.

557b4b6

nealkruis added NewFeature Includes code to add a new feature to EnergyPlus Performance Includes code changes that are directed at improving the runtime performance of EnergyPlus IDDChange Code changes impact the IDD file (cannot be merged after IO freeze) labels Apr 30, 2021

nealkruis added this to the EnergyPlus Future milestone Apr 30, 2021

nealkruis self-assigned this Apr 30, 2021

nealkruis added 6 commits July 27, 2021 13:39

Detect representative surfaces and apply to interior radiation exchange.

21c959d

Add report to EIO.

0e22f2d

Add IDD object and connect user input.

17d9e10

Merge develop.

1aaefc9

Update post-merge.

a75f851

Apply clang format.

12e633f

nealkruis modified the milestones: EnergyPlus 9.6 Release, EnergyPlus 9.6 IOFreeze Aug 4, 2021

nealkruis added 11 commits August 5, 2021 08:58

Don't use alias references to scalars.

ea67660

Change idd field order.

aab08e2

Initial IO-ref documentaiton.

7677a1c

Fix view factor aggregation.

f8b05c7

Temporarily hard-code representative surface calcs.

d53f7fa

Fix errors spotted by CI.

e69298e

Apply clang formatting.

1593f98

Don't use representative calculations with user view factors.

d295c36

Add unit test for representative surface detection.

b7cf7f5

Fix location of surface results update.

e7852f3

Merge remote-tracking branch 'nrel/develop' into auto-window-mult

b83908e

nealkruis marked this pull request as ready for review August 10, 2021 22:57

Remove forced representative surfaces and enhance unit testing.

caa4bcb

nealkruis changed the title ~~Add ability to automatically detect groups of similar window surfaces for efficient heat balance calculations~~ Add ability to automatically detect groups of similar surfaces for efficient heat balance calculations Aug 11, 2021

Apply clang format.

2fea556

nealkruis requested review from mbadams5, mjwitte and Myoldmopar August 11, 2021 18:46

nealkruis added 2 commits August 20, 2021 09:07

Merge develop.

88d58e8

Add enclosure numbers to surface hash. Add example file.

221967a

nealkruis added 2 commits August 21, 2021 20:24

Merge spaces.

bbe7d9a

Merge remote-tracking branch 'nrel/develop' into auto-window-mult

40dec64

mbadams5 approved these changes Aug 22, 2021

View reviewed changes

amirroth reviewed Aug 22, 2021

View reviewed changes

nealkruis added 3 commits August 23, 2021 08:21

Clean up clang warnings.

62f0cc7

More efficient setting or representative surface number.

99449b1

Make separate loop for assigning representative surfaces.

9d20137

mjwitte reviewed Aug 23, 2021

View reviewed changes

Myoldmopar merged commit 75c5e2b into develop Aug 23, 2021

Myoldmopar deleted the auto-window-mult branch August 23, 2021 20:42

nealkruis mentioned this pull request Sep 15, 2021

Representative surface heat balance calculations (follow up) #9065

Merged

19 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add ability to automatically detect groups of similar surfaces for efficient heat balance calculations #8739

Add ability to automatically detect groups of similar surfaces for efficient heat balance calculations #8739

nealkruis commented Apr 30, 2021 •

edited by mjwitte

Loading

nrel-bot-3 commented May 29, 2021

nrel-bot-2b commented Jun 26, 2021

nrel-bot-3 commented Jul 24, 2021

Myoldmopar commented Aug 19, 2021

nealkruis commented Aug 20, 2021

nealkruis commented Aug 20, 2021

Myoldmopar commented Aug 21, 2021

mbadams5 left a comment

mbadams5 Aug 22, 2021

amirroth Aug 22, 2021

mjwitte left a comment

mjwitte Aug 23, 2021

mjwitte Aug 23, 2021

mjwitte Aug 23, 2021

mjwitte Aug 23, 2021

mjwitte Aug 23, 2021

mjwitte Aug 23, 2021

mjwitte commented Aug 23, 2021

Myoldmopar commented Aug 23, 2021


		Surface are grouped based on the following criteria for similarity:

		\begin{itemize}

Add ability to automatically detect groups of similar surfaces for efficient heat balance calculations #8739

Add ability to automatically detect groups of similar surfaces for efficient heat balance calculations #8739

Conversation

nealkruis commented Apr 30, 2021 • edited by mjwitte Loading

Pull request overview

Pull Request Author

Reviewer

nrel-bot-3 commented May 29, 2021

nrel-bot-2b commented Jun 26, 2021

nrel-bot-3 commented Jul 24, 2021

Myoldmopar commented Aug 19, 2021

nealkruis commented Aug 20, 2021

nealkruis commented Aug 20, 2021

Myoldmopar commented Aug 21, 2021

mbadams5 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mjwitte left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mjwitte commented Aug 23, 2021

Myoldmopar commented Aug 23, 2021

nealkruis commented Apr 30, 2021 •

edited by mjwitte

Loading