Bypass interior solar distribution calculation at night

[lgu1234]

Pull request overview

LBL identified 20 hotspots for speed improvement. InitIntSolarDistribution is one of hotspots. The interior solar distribution calculation is performed every time step. The present check-in bypasses the calculation at night, because no solar is available at night.

Pull Request Author

Add to this list or remove from it as applicable. This is a simple templated set of guidelines.

• Title of PR should be user-synopsis style (clearly understandable in a standalone changelog context)
• Label the PR with at least one of: Defect, Refactoring, NewFeature, Performance, and/or DoNoPublish
• Pull requests that impact EnergyPlus code must also include unit tests to cover enhancement or defect repair
• Author should provide a "walkthrough" of relevant code changes using a GitHub code review comment process
• If any diffs are expected, author must demonstrate they are justified using plots and descriptions
• If changes fix a defect, the fix should be demonstrated in plots and descriptions
• If any defect files are updated to a more recent version, upload new versions here or on DevSupport
• If IDD requires transition, transition source, rules, ExpandObjects, and IDFs must be updated, and add IDDChange label
• If structural output changes, add to output rules file and add OutputChange label
• If adding/removing any LaTeX docs or figures, update that document's CMakeLists file dependencies

Reviewer

This will not be exhaustively relevant to every PR.

• Perform a Code Review on GitHub
• If branch is behind develop, merge develop and build locally to check for side effects of the merge
• If defect, verify by running develop branch and reproducing defect, then running PR and reproducing fix
• If feature, test running new feature, try creative ways to break it
• CI status: all green or justified
• Check that performance is not impacted (CI Linux results include performance check)
• Run Unit Test(s) locally
• Check any new function arguments for performance impacts
• Verify IDF naming conventions and styles, memos and notes and defaults
• If new idf included, locally check the err file and other outputs

[rraustad]

Do any variables need to be set to 0 when the sun is down? I wonder if the last time these variables were calculated when the sun was up will be used during the night. Maybe they are set to 0 elsewhere.

[lgu1234]

@rraustad There are 3 variables inside section:

            IntBmIncInsSurfIntensRep(SurfNum) 
            IntBmIncInsSurfAmountRep(SurfNum) 
            IntBmIncInsSurfAmountRepEnergy(SurfNum)

The variables are initialized at 0 in the beginning of InitSolarHeatGains before calling InitIntSolarDistribution. Therefore, no additional initialization is needed.

[Nigusse]

This is clean modification for performance. I will update the branch, run the unit tests locally and push the branch.

[Nigusse]

@Myoldmopar Run the unit test locally and all passed. If the CI machine comes clean, this branch can be merged in.

[Myoldmopar]

Thanks for looking it over and running tests, @Nigusse. CI looks happy so far, I'll take a final peek and if all is well it should merge in the morning.

[Myoldmopar]

Looks like primarily just formatting cleanup here, and that the actual functional changes are minimal. I share @rraustad's concern that skipping over the functionality could end up leaving variables with previous data. I see you have determined which variables are used and that they are properly initialized, so I will take that, and also rely on CI to show whether this causes any unexpected diffs.

[Myoldmopar]

And as I already noted, CI is not showing any diffs at all, so I feel good about these changes. I'm going to pull in develop locally just to verify, but I assume this will go in shortly.

[Nigusse]

The report variables are already initialized earlier. I am okay with the current fix.

[Myoldmopar]

Thanks for the fix @lgu1234 and for reviewing, @Nigusse. The unit tests pass locally with develop pulled in. Merging now.

[Myoldmopar]

@lgu1234 there wasn't an issue or Pivotal ticket for this, was there? If so assign them to me so I can close them.

[lgu1234]

@Myoldmopar This PR does not have an issue and an associated Pivotal ticket. Thanks.

[lgu1234]

@Nigusse I just created an issue as Bypass interior solar distribution calculation at night #8201and a Pivotal ticket as #174304363.