Allow GPU-based shading calculations #7302

nealkruis · 2019-05-20T14:49:56Z

Add the ability to use GPU-based shading calculations using the pixel counting methodology. See the design doc.

Review Checklist

macumber · 2019-05-22T03:35:47Z

design/FY2019/GPU-Shading.md

+
+### Interior Solar Distribution ###
+
+We will count pixels when viewing the from the perspective of the sun through a window with each internal surface assigned a different color, then use the histogramming functions in OpenGL to report the number of pixels of each color that are visible. This functionality will be added to Penumbra.


Does this place a limit on the maximum number of surfaces (e.g. maximum number of colors)?

How will this interior calculation work with blinds and shades on windows that are controlled at each timestep? What about shading surfaces with transmittance schedules.

I'll have to check, but I think we aren't limited to the number of colors. Even still, 255x255x255 = 16,581,375. Hard to imagine there would be more surfaces in a model (let alone a single zone).

Blinds/shades will work the same as they currently do. They aren't part of the shading calculations they affect the transmittance (of the solar through the window).

Scheduled transmittance is a different challenge discussed below in the document.

macumber · 2019-05-22T03:42:11Z

idd/Energy+.idd.in

       \note Advanced Feature.  Internal default is SutherlandHodgman
       \note Refer to InputOutput Reference and Engineering Reference for more information
       \type choice
       \key ConvexWeilerAtherton
       \key SutherlandHodgman
       \default SutherlandHodgman
-  A3 , \field Sky Diffuse Modeling Algorithm
+  A4 , \field Sky Diffuse Modeling Algorithm


Are there any parameters needed to control the pixel counting algorithm? I would think that number of pixels would be a parameter?

Would an option to write out images or renderings be useful for debugging? Ability to write out shading schedules? Any other reporting that should be considered?

I think adding a resolution input would be good. The rendering is mapped to a square, so maybe:

\field Pixel Counting Resolution \note Number of pixels in both dimensions of the surface rendering \type integer \default 512

The library has the ability to open a window rendering of the scene, but it is currently used only for debugging the C++ code (not for general simulations). This could be added later if needed.

How about "Pixel Maximum Length" instead? Using the same number of pixels on a small surface as a large surface seems unnecessary.

Does the pixel counting method cover diffuse shading?

The method stretches any surface, regardless of its size, to fill as much of the rendered view as possible and provide the maximum possible resolution. What is rendered is often stretched in the X or Y direction to fit within the square of the determined size (that is, even a rectangular surface will be stretched to an aspect ratio of 1:1). The algorithm is aware that the projected dimensions are not necessarily equal and accounts for this appropriately.

Yes, diffuse shading is done using the pixel counting method for each location in the sky dome grid.

So - does this field have any impact when using the GPU method? Ultimately, when this is complete, IDD notes and I/O ref will need to highlight which fields apply only to certain shading methods.

It will still have an impact. Ideally we will figure out how to handle variable transmittance surfaces and this will be handled the same way it is for the polygon clipping method.

JasonGlazer · 2019-05-24T11:38:16Z

Have you done any benchmarking?

hongtz68 · 2019-05-30T01:41:39Z

idd/Energy+.idd.in

-       \type choice
-       \key ScheduledShading
-       \key InternalCalculation
-       \key ImportedShading


Are you proposing to remove this field "External Shading Calculation Method"? We need to keep it to import shading calculation results by external tools.

@hongtz68 I'm not proposing to remove it. I'm proposing moving the capability to a new field: "Shading Calculation Method"

mjwitte · 2019-06-04T21:52:56Z

idd/Energy+.idd.in

       \note Advanced Feature.  Internal default is SutherlandHodgman
       \note Refer to InputOutput Reference and Engineering Reference for more information
       \type choice
       \key ConvexWeilerAtherton
       \key SutherlandHodgman
       \default SutherlandHodgman
-  A3 , \field Sky Diffuse Modeling Algorithm
+  A4 , \field Sky Diffuse Modeling Algorithm


How about "Pixel Maximum Length" instead? Using the same number of pixels on a small surface as a large surface seems unnecessary.

mjwitte · 2019-06-04T22:02:21Z

idd/Energy+.idd.in

@@ -526,18 +540,18 @@ ShadowCalculation,
       \note only really applicable to RunPeriods


This line is misleading, because SizingPeriod:WeatherFileDays would also use this. I'd probably just delete this note

For the notes above this line . . .
AverageOverDaysInFrequency --> Periodic
~~0=Use Default Periodic Calculation~~ (minimum is 1, so zero isn't valid since who know when).

Agreed. This is all existing text, though I can clean it up here.

mjwitte · 2019-06-04T22:08:24Z

idd/Energy+.idd.in

       \note Advanced Feature.  Internal default is SutherlandHodgman
       \note Refer to InputOutput Reference and Engineering Reference for more information
       \type choice
       \key ConvexWeilerAtherton
       \key SutherlandHodgman
       \default SutherlandHodgman
-  A3 , \field Sky Diffuse Modeling Algorithm
+  A4 , \field Sky Diffuse Modeling Algorithm


Does the pixel counting method cover diffuse shading?

nealkruis · 2019-06-05T19:26:03Z

idd/Energy+.idd.in

+       \note If Imported is chosen, the Schedule:File:Shading object is required.
+       \type choice
+       \key PolygonClipping
+       \key PixelCounting


"GPUPixelCounting"?

jasondegraw · 2019-06-06T14:18:01Z

CMakeLists.txt

@@ -25,6 +25,7 @@ set( ENERGYPLUS_VERSION "${CMAKE_VERSION_MAJOR}.${CMAKE_VERSION_MINOR}.${CMAKE_V

 set( CMAKE_VERSION_BUILD "Unknown" CACHE STRING "Build number" )
 find_package(Git)
+find_package(OpenGL)


@nealkruis Is this requiring that OpenGL be available to build? If so, wouldn't it be better to have a fallback that doesn't require the availability of OpenGL?

I put this in here with the idea that I would disable some of the unit tests if the machine doesn't have OpenGL. I'm not sure it's really necessary though.

Just to make sure we're all clear of the requirements on this.

We will not make OpenGL, or any other graphical rendering library, a hard requirement for building or running EnergyPlus, on any platform. We will need CMake logic in place to have fallbacks for building EnergyPlus where it doesn't attempt to find a rendering library. And in EnergyPlus itself, we will need to have a switch for enabling specialized GPU-dependent calculations. I would love if a user who doesn't have OpenGL could still take advantage of pixel counting, if not for performance, then for proper shading evaluation of complicated zone geometry. With this in mind, I would like the input field for enabling this to specifically reflect "turning this on requires a graphics library on this machine", and then if E+ tries to run and doesn't find it, it gracefully fatals out and gives the user enough info to continue (either disable that feature or install the lib and re-run).

For the CI machines, as discussed yesterday, I am going to continue to push back against installing any additional libraries until we understand the consequences and packaging issues. We have in the past installed stuff on the machines without thinking about about these consequences, and ended up having a difficult time getting packages back to normal (Visual Studio drama of very late March 2017). Once we have all these issues answered, we can decide how to proceed. Eventually it seems important that we have a build on a clean machine that doesn't have any optional build dependencies on it, and then one that has all the optional build dependencies on it, but that would be a significant increase in burden, so I'm not sure we can completely support that. We'll figure it out.

Just know that I am in support of this new feature, I am just going to be playing slow and tight with the dependencies and the builds.

This makes sense to me.

Building: It will likely require some preprocessor directives to disable portions of code if the build machine doesn't have the linking graphics libraries. We'll likely also want to put something in the --version info to indicate whether a build does not support graphics processing. I can also have CMake issue a nice loud warning if OpenGL isn't found.

Running: This is already handled, but the messaging could use clean up. We'll need different messages depending on why it fails:

the local machine doesn't have the required libraries, or

the build wasn't linked to a graphics library.

Thanks @nealkruis, I should also point out that I don't expect any user to ever encounter a build of EnergyPlus that they find to not be built with support for the feature. The builds we produce as packages will have the capability built in so that we don't have stray packages lying around and confusion about whether a package has it or not.

The build issue is purely for developers. I don't want developers to have to install additional dependencies just to get a build going. This is the same thing as how our build allows BUILD_DOCS and BUILD_FORTRAN so that if a developer doesn't need to build those features, they don't have to install the stuff to do it. I imagine this is something that will be like BUILD_FOR_OPENGL or whatever, defaults to OFF, but can be flipped on easily. And our CI build scripts will set -DBUILD_FOR_OPENGL=ON. There's probably a niche case where an actual user wants to make a custom build that doesn't want this dependency also, but that would be the same as the developer use-case really.

No, I don't think BUILD_FOR_OPENGL is the right nomenclature 😆

For user alert, how about:

$ ./energyplus --config { "program_name": "EnergyPlus", "version_number": "9.2.0", "build_sha": "abc123", "architecture": "64bit", "compiled_with": "Visual Studio 2019", "compile_date": "UTC 8601 Date", "build_options": [ "open_gl_dependency": "true/false", ] }

How cool would that be!!??

I might lean toward making builds with OpenGL by default (since most developer machines will have the necessary libraries). It's really only Linux machines where there will be a problem.

What if we have BUILD_OPENGL_LINKS (working nomenclature) ON by default. If it's ON and CMake can't find OpenGL, CMake issues an error with a suggestion that the user either install a working implementation or turn BUILD_OPENGL_LINKS to OFF.

Yeah, I could be convinced to default it to ON. In that case, I don't want it to be an advanced cmake variable so that it is easily accessible for disabling if someone should need to do it.

BUILD_OPENGL_LINKS seems like an improvement. We can keep noodling on that, but it seems to represent what we're doing.

mjwitte · 2020-02-20T22:55:33Z

@nealkruis @mbadams5 This needs a transition rule, and it's got a conflict now with SolarShading.cc

mjwitte · 2020-02-20T22:59:26Z

And CI results are probably meaningless at this point, given how far develop has moved in the last 24 hrs

jasondegraw · 2020-02-20T23:06:48Z

@mjwitte Mark is working on the conflicts, some unfortunate choices were made in a recent PR

nealkruis · 2020-02-21T04:20:40Z

src/EnergyPlus/SolarShading.cc

@@ -676,7 +676,7 @@ namespace SolarShading {
 #else
                auto error_callback = [](const int messageType, const std::string & message, void * /*contextPtr*/){
                    if (messageType == Pumbra::MSG_ERR) {
-                        ShowFatalError(message);
+                        ShowSevereError(message);


@mbadams5 I'm worried that this won't effectively handle problems within Penumbra.

mjwitte · 2020-02-21T14:39:14Z

This has a build warning.
It still needs text description of the transition in \src\Transition\InputRulesFiles\Rules9-2-0-to-9-3-0.md
And a description of the eio output changes for <Shadowing/Sun Position Calculations Annual Simulations> in \src\Transition\OutputRulesFiles\OutputChanges9-2-0-to-9-3-0.md

mjwitte

Tested transition with RefBldgFullServiceRestaurantNew2004_ChicagoTable, SolarShadingTest_DisableSelfShadingGroupTable, SolarShadingTest_DisableSelfShadingTable, SolarShadingTest_ImportedShadingTable. All transitioned properly and give same results as v9.2 (amazingly).
This will need documentation (post-haste after I/O freeze).
Re-built with OPENGL_REQUIRED, built fine with Win10 VS2019 16.4.5.
Ran SolarShadingTest with PixelCounting option. It ran slower than PolygonClipping. Opened TaskManager and saw that it's using the onboard GPU, not the Nvidia card.

Is there a system setting somewhere to get this to use the other GPU?

nealkruis · 2020-02-21T16:41:52Z

@mjwitte

Don't be too surprised 😉.
Working on documentation now. Almost complete.
Great!
@mbadams5 might have more insight on this.

mjwitte · 2020-02-21T16:46:22Z

Found the setting, had to manually switch the setting for EnergyPlus.exe

But maybe I can change the system default somewhere.

Anyways, with that setting changed I see activity in the Nvidia GPU.

mbadams5 · 2020-02-21T16:54:08Z

@mjwitte There is a way to change it system wide (https://stackoverflow.com/a/30515166), but I would typically allow your laptop to automatically switch between the two for battery savings. In that link, there is a way in code to force nvidia optimus (https://stackoverflow.com/a/27881472) to prefer the more powerful GPU and same for AMD (https://stackoverflow.com/a/28179913). We could add those to EnergyPlus but that could happen during bug fix since i'd want more testing around that added code to make sure nothing breaks.

mjwitte · 2020-02-21T17:05:50Z

@mbadams5 Thanks. I see I can get to application-specific settings in the Nvidia control panel as well. What's puzzling is that auto-select didn't use the Nvidia card even though it's running with the power plugged in. Anyways, I probably wouldn't try to change anything to force this for the 9.3 release, perhaps just highlight these settings in the docs for the PixelCounting option.

…Plus into penumbra-integration

mjwitte · 2020-02-21T17:56:28Z

For a small run, like SolarShadingTest, the PixelCounting method is slower that PolygonClipping, as expected. But for a larger run, like \PerformanceTests\10x_incr_100zones1win10overhangsFullExIn (repaired and transitioned to 9.3, and output:variables turned off).

PolygonClipping:    ************* EnergyPlus Completed Successfully-- Elapsed Time=00hr 03min 21.08sec
PixelCounting:      ************* EnergyPlus Completed Successfully-- Elapsed Time=00hr 02min  6.60sec

That's 1.6x - nice! It was interesting to watch the sawtooth loading on the cpu and gpu as it switched between shading calcs and the rest of the simulation. And this was with shadow calcs every 20 days, it would only get better with more frequent shadow calcs. Will run with timestep frequency and report later (expectng PolygonClipping to take a while for that).

Myoldmopar · 2020-02-21T17:58:45Z

👍 👍 👍 👍 👍 👍

mjwitte · 2020-02-21T19:10:49Z

Results just in . . . 10x_incr_100zones1win10overhangsFullExIn with timestep shadow calculations . . .
PolygonClipping 30:21, PixelCounting 8:42, 3.6x, woohoo!

And results are quite similar. This file has no HVAC, but window heat gain values in the Annual Building Sensible Heat Gain Components report vary by less than 1%.

mjwitte

Good start on the docs. It would be good to expand on the I/O Ref to discuss when it's useful to apply PixelCounting and how to maximize hardware performance, but that can wait until after I/O Freeze.

A few additional items noted below (but they, too, should wait).

mjwitte · 2020-02-21T19:22:26Z

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

+
+This field is used to control how the solar, shading, and daylighting models are calculated with respect to the time of calculations during the simulation. The default and fastest method is selected using the keyword Periodic. A more detailed and slower method can be selected using the keyword Timestep. The Timestep method must be used for modeling dynamic fenestration and shading surfaces.
+
+\paragraph{Field: Shading Calculation Update Frequency}\label{field-calculation-frequency}

 This numeric field will cause the shadowing calculations to be done periodically using the number in the field as the number of days in each period. This field is only used if the default method AverageOverDaysInFrequency is used in the previous field. Using this field will allow you to synchronize the shadowing calculations with changes in shading devices. Using the default of 20 days in each period is the average number of days between significant changes in solar position angles. For these shadowing calculations, an ``average'' (over the time period) of solar angles, position, equation of time are also used.


AverageOverDaysInFrequency --> Periodic

And, above, can you brag that PixelCounting works for FullInterior in non-convex zones? Or is that uncertain yet?

And the engineering reference is a bit slim - ok for now.

mjwitte · 2020-02-21T19:29:28Z

performance_tests/Inactive/60Zone1PVAV.idf

@@ -29,8 +29,10 @@
  HeatBalanceAlgorithm,ConductionTransferFunction;

  ShadowCalculation,
-    AverageOverDaysInFrequency,  !- Calculation Method
-    20;                      !- Calculation Frequency
+    PolygonClipping,         !- Shading Calculation Method


Also - after release - we should revert these (or maybe even remove them altogether @Myoldmopar ?) They aren't current and these changes will fully break them. I guess we try to figure out when they were last working correclyt and re-transition them, but that's low priority.

Note: This comment applies only to the files in performance_tests/Inactive. The active idfs are all good.

mjwitte · 2020-02-21T19:31:59Z

src/Transition/OutputRulesFiles/OutputChanges9-2-0-to-9-3-0.md

+### EIO output for ShadowCalculations
+
+The eio output related to `ShadowCalculations` has been changed to reflect the new input fields. See Rules9-2-0-to-9-3-0.md.
+


For completeness, add the PR reference (later).

mjwitte · 2020-02-21T19:32:12Z

src/Transition/InputRulesFiles/Rules9-2-0-to-9-3-0.md

@@ -48,6 +48,30 @@ a) change "WCS" or "WorldCoordinateSystem" or "Absolute" to "World"

 b) change "Rel\*" or "Relative\*" or "Local" to "Relative"

+# Object Change: ShadowCalculations


Looks good.

nealkruis added 10 commits April 1, 2019 20:52

Link to Penumbra.

2f6f386

Penumbra Cmake fixes (for Windows builds).

9ad3b1f

Merge branch 'develop' into penumbra-integration

3c8e0d4

Initial implementation of Penumbra to do shading calculations.

2a30fc3

Fix reveal calculations.

8aafc93

Update penumbra.

674d389

Modify ShadowCalculation object to allow input for GPU-based shading.

f041b22

Update testfiles.

341047a

Merge branch 'develop' into penumbra-integration

cf37de5

Add design document.

7794e71

nealkruis added NewFeature Includes code to add a new feature to EnergyPlus IDDChange Code changes impact the IDD file (cannot be merged after IO freeze) labels May 20, 2019

nealkruis self-assigned this May 20, 2019

nealkruis added 5 commits May 20, 2019 10:25

Fix unit tests using 'ShadowCalculation' object.

3030f59

Only run Penumbra tests if OpenGL is available.

762b35c

Eliminate function name conflict.

9cd3a17

Update design doc.

aeddb46

Expand description of existing capabilities.

f3d0cab

macumber reviewed May 22, 2019

View reviewed changes

hongtz68 reviewed May 30, 2019

View reviewed changes

mjwitte reviewed Jun 4, 2019

View reviewed changes

nealkruis commented Jun 5, 2019

View reviewed changes

jasondegraw reviewed Jun 6, 2019

View reviewed changes

nealkruis added 5 commits June 6, 2019 10:28

Add Pixel Counting Resolution field.

6f02d69

Add interior solar distribution calculations.

a0bd5c4

Separate shader programs for render and calculate.

ebca86b

Remove extra query check.

5170ab3

Update Penumbra.

5709797

nealkruis added 2 commits February 19, 2020 15:22

Merge develop into penumbra-integration.

a22b1aa

Clean up compiler warnings.

32a411c

mbadams5 added 3 commits February 20, 2020 20:46

Merge remote-tracking branch 'origin/develop' into penumbra-integration

fde9a15

Fix unit test errors since penumbra error shouldn't fatal

353723a

Remove unused using statements

5e06222

nealkruis commented Feb 21, 2020

View reviewed changes

nealkruis and others added 2 commits February 20, 2020 22:56

Add transition rules.

e3fc456

Fixing failing unit test

95b2e30

Myoldmopar mentioned this pull request Feb 21, 2020

C API, Python API, and Python Plugins #7776

Merged

10 tasks

mjwitte added the Performance Includes code changes that are directed at improving the runtime performance of EnergyPlus label Feb 21, 2020

nealkruis and others added 3 commits February 21, 2020 09:05

Add transition rules description.

dbf990b

Add output rules description.

742a5ec

Fix build warning

4b565c3

mjwitte reviewed Feb 21, 2020

View reviewed changes

nealkruis added 2 commits February 21, 2020 10:09

Merge branch 'penumbra-integration' of https://github.com/NREL/Energy…

a8518d9

…Plus into penumbra-integration

Draft documentation.

9618585

mjwitte approved these changes Feb 21, 2020

View reviewed changes

mjwitte merged commit 327add2 into develop Feb 21, 2020

mjwitte deleted the penumbra-integration branch February 21, 2020 20:02


		### Interior Solar Distribution ###

		We will count pixels when viewing the from the perspective of the sun through a window with each internal surface assigned a different color, then use the histogramming functions in OpenGL to report the number of pixels of each color that are visible. This functionality will be added to Penumbra.

		@@ -526,18 +540,18 @@ ShadowCalculation,
		\note only really applicable to RunPeriods

		### EIO output for ShadowCalculations

		The eio output related to `ShadowCalculations` has been changed to reflect the new input fields. See Rules9-2-0-to-9-3-0.md.

		@@ -48,6 +48,30 @@ a) change "WCS" or "WorldCoordinateSystem" or "Absolute" to "World"

		b) change "Rel\" or "Relative\" or "Local" to "Relative"

		# Object Change: ShadowCalculations

Allow GPU-based shading calculations #7302

Allow GPU-based shading calculations #7302

Conversation

nealkruis commented May 20, 2019 • edited Loading

Review Checklist

Choose a reason for hiding this comment

macumber May 22, 2019 • edited Loading

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

JasonGlazer commented May 24, 2019

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mjwitte commented Feb 20, 2020

mjwitte commented Feb 20, 2020

jasondegraw commented Feb 20, 2020

Choose a reason for hiding this comment

mjwitte commented Feb 21, 2020

mjwitte left a comment • edited Loading

Choose a reason for hiding this comment

nealkruis commented Feb 21, 2020

mjwitte commented Feb 21, 2020

mbadams5 commented Feb 21, 2020

mjwitte commented Feb 21, 2020

mjwitte commented Feb 21, 2020

Myoldmopar commented Feb 21, 2020

mjwitte commented Feb 21, 2020

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

nealkruis commented May 20, 2019 •

edited

Loading

macumber May 22, 2019 •

edited

Loading

mjwitte left a comment •

edited

Loading