Fix the ground slab horizontal insulation thickness issue (Issues 7881 and 8800)

[jcyuan2020]

Pull request overview

• Fixes Site:GroundDomain:Slab is not using the material thickness of the horizontal insulation material #7881 and fixes Vertical insulation thickness misused as Horizontal Insulation thickness in ground coupled slabs domain partitioning #8800;
• The original issue about an insensitive horizontal insulation thickness (in Site:GroundDomain:Slab) was reported in a couple of github issues and several helpdesk tickets. The users specified changed the insulation materials' thickness for the horizontal slab insulation but the resulted energy results (such as heating energy in winter, or cooling energy in summer) do not change even very drastic changes occurred between the different cases. The root cause was found to be a code error in the Finite Difference meshing for the slab calculation, where a horizontal thickness should have been used but instead in the original code the vertical insulation thickness was used. These would cause an invariant grid partitioning for the cells used for finite difference soil heat transfer calculation around the slab. The error is now corrected in this fix.
• Two unit tests with different horizontal thickness values were added to test out the grid partitioning portion of the related code;
• An example file was created based upon an existing Site:GroundDomain:Slab test example IDF file, for an increased horizontal slab insulation thickness.

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

[mjwitte]

@jcyuan2020 I was puzzled that the regression tests showed no diffs, but then I see that the test files which have horizontal insulation, also have vertical insulation using the same material (so the insulation thickness is the same for both). If @Myoldmopar agrees with this change, then go ahead an prepare a unit test.

[jcyuan2020]

@mjwitte Thanks! The defect files would demonstrate the difference, where the different horizontal insulation thickness is used.

[Myoldmopar]

I agree this change looks reasonable and would suggest modifying an example file to use a different horizontal insulation thickness as well as forging ahead on a unit test.

[jcyuan2020]

Thanks! 👍

[nrel-bot]

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

[Myoldmopar]

I actually would've preferred modifying an existing example file, but I'm not holding this up for now. This is a great fix and the unit test looks good. Building and testing locally now.

[Myoldmopar]

Builds and tests fine locally. Good fix here @jcyuan2020, thanks.