Surface Window data struct & heat balance routines refactoring #8257

xuanluo113 · 2020-09-07T18:16:21Z

Pull request overview

This branch includes refactoring of the HB related data structures and scope reduction in InitSolarHeatGains. In detail, the changes include:

Refactoring – naming convention (var name prefix)
e.g. Surf (for all surfaces), SurfWin, SurfOpaq
Reduce the scope of function level local variables.
e.g. Surface(SurfNum).FrameDivider > 0
Reduce scope of loops (Separating into shading loops, opaq surface loops and window surface loops)
Reduce redundant branches
e.g. CalcSolRefl
Move branches out of the loop applying MCCF
e.g. TDD diffusers and TDD shelves

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.

… into window-struct

…dow-struct

…yPlus into window-struct

…Plus into window-struct2

…dow-struct2

…dow-struct

…yPlus into window-struct

…dow-struct

mjwitte

@xuanluo113 I've merged in develop, resolved some minor conflicts and updated this branch.

Unit tests run locally all pass.

CI is looking good so far. In general, this all looks good - variable naming clarifications and loop variable scope reductions. A few comments/questions:

mjwitte · 2020-10-09T16:39:54Z

src/EnergyPlus/DataGlobals.cc

@@ -257,6 +257,7 @@ namespace DataGlobals {
    std::function<void(EnergyPlus::Error e, const std::string &)> errorCallback;

    bool eplusRunningViaAPI;
+    double timer_1(0.0);


Did you intend to keep this here?

No. Left by mistake. Just deleted these.

mjwitte · 2020-10-09T16:47:30Z

src/EnergyPlus/DataHeatBalSurface.cc

+    Array1D<Real64> ZoneVMULT;             // 1/(Sum Of A Zone's Inside Surfaces Area*Absorptance)
+    Array1D<Real64> ZoneVCONV;             // Fraction Of Short-Wave Radiation From Lights Converted To Convection


These (and others) are really enclosure variables, not zone, indexed on enclosureNum (or similar). Also, solar and radiant enclosures can be different. Seems like we should use different prefixes for enclosures? SolEncl... RadEncl... ?

I renamed ZoneVMULT and QD's to use SolEnclprefix. I realizedQC, QDVandVCONV` are allocated but not used anymore, so I deleted them.

Looking at this now, I realize that this should be EnclSol to match the pattern of SurfOpaq SurfWin etc. It's your call whether to change that in this branch or in your next phase. Sorry I didn't see that before.

Makes sense. I renamed it at the latest commit.

src/EnergyPlus/DataSurfaces.cc

src/EnergyPlus/HeatBalanceSurfaceManager.cc

mjwitte · 2020-10-09T17:12:05Z

src/EnergyPlus/HeatBalanceSurfaceManager.cc

+            if (BuildingShadingCount || FixedShadingCount || AttachedShadingCount) {
+                for (int SurfNum = 1; SurfNum <= TotSurfaces; ++SurfNum) {
+                    if (!Surface(SurfNum).ShadowingSurf) continue;


Since all shading surfaces are at the top of the surface list, we should be able to add a new variable for TotShadingSurfaces (or ShadingSurfaceLast or similar name) and loop here from 1 to TotShadingSurfaces.

I'm actually working on this at the other branch, but haven't fully tested this piece yet. Do we want to pull that piece in this one? I was hoping this can go first so I'm not blocking Matt.

This is ok here. It can wait for your next phase.

src/EnergyPlus/HeatBalanceSurfaceManager.cc

mjwitte · 2020-10-09T18:10:38Z

src/EnergyPlus/DataHeatBalSurface.cc

    // for reporting (W)
-    Array1D<Real64> TempSurfOut; // Temperature of the Outside Surface for each heat transfer surface
+    Array1D<Real64> SurfTempSurfOut; // Temperature of the Outside Surface for each heat transfer surface


Minor, but names like this which already have Surf in them could simply be SurfTempOut.

I renamed this globally.

third_party/ObjexxFCL/src/ObjexxFCL/Array1.hh

mjwitte · 2020-10-09T19:52:22Z

@xuanluo113 Also, please edit the PR description at the top with a brief summary of the changes.

xuanluo113 · 2020-10-10T04:39:58Z

@mjwitte Thanks for the review. I address them and updated the PR overview.

mjwitte

@xuanluo113 Thanks for addressing my comments.

@Myoldmopar I'm finished reviewing this. Let me know if you want to review before merging.

Myoldmopar · 2020-10-12T15:30:28Z

Let it go in, I don't have anything further.

xuanluo113 · 2020-10-12T15:40:20Z

@mjwitte @Myoldmopar thanks!

xuanluo113 added 30 commits August 30, 2020 18:28

transolar

5363b0a

Merge branch 'window-test' of https://github.com/energy-plus/EnergyPlus…

0ec01de

… into window-struct

more surf win

e82f4a2

Merge branch 'develop' of https://github.com/NREL/EnergyPlus into win…

f4f285b

…dow-struct

Merge branch 'develop' of https://github.com/NREL/EnergyPlus into win…

6064516

…dow-struct

Merge branch 'develop' of https://github.com/NREL/EnergyPlus into win…

f93e803

…dow-struct

more surfwin

147075c

Merge branch 'develop' of https://github.com/NREL/EnergyPlus into win…

8a5b32f

…dow-struct

fix tdd

b5d30ed

Merge branch 'develop' of https://github.com/NREL/EnergyPlus into win…

013e992

…dow-struct

locals

aaa0d4c

test inithb

05e3b4e

Merge branch 'develop' of https://github.com/NREL/EnergyPlus into win…

15dbe85

…dow-struct

test init

7618438

test init

3bfd63d

revert some tests

df84b1a

reduce test 1

48d0093

shading

ef6900a

Merge branch 'develop' of https://github.com/NREL/EnergyPlus into win…

2cdd30b

…dow-struct

Merge branch 'window-struct2' of https://github.com/energy-plus/Energ…

ff797a1

…yPlus into window-struct

tdd

1c47c9e

Merge branch 'window-struct' of https://github.com/energy-plus/Energy…

347bb62

…Plus into window-struct2

test reduce

d2451f6

revert

cb37a59

Merge branch 'develop' of https://github.com/NREL/EnergyPlus into win…

5597b57

…dow-struct2

framediv

b227d8c

Merge branch 'develop' of https://github.com/NREL/EnergyPlus into win…

51449d5

…dow-struct

Merge branch 'window-struct2' of https://github.com/energy-plus/Energ…

5699572

…yPlus into window-struct

Merge branch 'develop' of https://github.com/NREL/EnergyPlus into win…

75ff0e7

…dow-struct

reduce

aadcdd1

xuanluo113 added 4 commits September 11, 2020 09:36

scope

36ce354

Merge branch 'develop' of https://github.com/NREL/EnergyPlus into win…

142c031

…dow-struct

tdd shelf tubular

15813fd

test

c136df5

xuanluo113 added NotIDDChange Code does not impact IDD (can be merged after IO freeze) Refactoring Includes code changes that don't change the functionality of the program, just perform refactoring labels Sep 12, 2020

xuanluo113 added this to the EnergyPlus Future milestone Sep 12, 2020

xuanluo113 added 8 commits September 12, 2020 10:37

seperate

eeb94b5

Merge branch 'develop' of https://github.com/NREL/EnergyPlus into win…

a792481

…dow-struct

fix dome

9291a8c

Merge branch 'develop' of https://github.com/NREL/EnergyPlus into win…

7788635

…dow-struct

clean up

c45388f

Merge branch 'develop' of https://github.com/NREL/EnergyPlus into win…

8fa7d45

…dow-struct

merge

5b9773e

merge

3c58f50

mjwitte marked this pull request as ready for review October 5, 2020 19:45

mjwitte mentioned this pull request Oct 8, 2020

Global DataConstruction, VariableSpeedCoils, and VentilatedSlab #8334

Merged

20 tasks

Merge remote-tracking branch 'remotes/origin/develop' into window-struct

90bab9d

mjwitte reviewed Oct 9, 2020

View reviewed changes

xuanluo113 added 2 commits October 9, 2020 21:11

address review

8a581b9

address review

ca7d940

xuanluo113 added the Performance Includes code changes that are directed at improving the runtime performance of EnergyPlus label Oct 10, 2020

encl naming

96f3999

mjwitte approved these changes Oct 11, 2020

View reviewed changes

mjwitte merged commit ed7cef8 into NREL:develop Oct 12, 2020

xuanluo113 deleted the window-struct branch October 14, 2020 17:34

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Surface Window data struct & heat balance routines refactoring #8257

Surface Window data struct & heat balance routines refactoring #8257

xuanluo113 commented Sep 7, 2020 •

edited by mjwitte

Loading

mjwitte left a comment

mjwitte Oct 9, 2020

xuanluo113 Oct 10, 2020

mjwitte Oct 9, 2020

xuanluo113 Oct 10, 2020

mjwitte Oct 10, 2020

xuanluo113 Oct 10, 2020

mjwitte Oct 9, 2020

xuanluo113 Oct 10, 2020

mjwitte Oct 10, 2020

mjwitte Oct 9, 2020

xuanluo113 Oct 10, 2020

mjwitte commented Oct 9, 2020

xuanluo113 commented Oct 10, 2020

mjwitte left a comment

Myoldmopar commented Oct 12, 2020

xuanluo113 commented Oct 12, 2020

		Array1D<Real64> ZoneVMULT; // 1/(Sum Of A Zone's Inside Surfaces Area*Absorptance)
		Array1D<Real64> ZoneVCONV; // Fraction Of Short-Wave Radiation From Lights Converted To Convection

Surface Window data struct & heat balance routines refactoring #8257

Surface Window data struct & heat balance routines refactoring #8257

Conversation

xuanluo113 commented Sep 7, 2020 • edited by mjwitte Loading

Pull request overview

Pull Request Author

Reviewer

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

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 Oct 9, 2020

xuanluo113 commented Oct 10, 2020

mjwitte left a comment

Choose a reason for hiding this comment

Myoldmopar commented Oct 12, 2020

xuanluo113 commented Oct 12, 2020

xuanluo113 commented Sep 7, 2020 •

edited by mjwitte

Loading