GHE Refactoring Work by Myoldmopar · Pull Request #11152 · NatLabRockies/EnergyPlus

Myoldmopar · 2025-08-14T19:10:33Z

Pull request overview

This takes the huge GroundHeatExchangers hh and cc files and breaks them into meaningful pieces. The next steps on this include updating the physics model and including GHEDesigner for GLHE auto sizing. I've got the GHEDesigner stuff underway, but I'm running into an issue with the results, so I want to get this big changeset merged in with (I believe) no diffs. Then it will be easier to feel confident in the model changes, since they may have some small numeric diffs.

Description of the purpose of this PR

Pull Request Author

Reviewer

Myoldmopar

Should be an effectively non-functioning change, isolated to the GLHE codebase. Assuming no diffs, I'd like to merge this right in and continue on the functional changes.

Myoldmopar · 2025-08-14T19:11:03Z

cmake/PythonInstallCLITools.py


 # expecting one command line argument - the path to the python_lib folder to place the pip package
-pkgs = {'energyplus_launch': '3.7.4', 'energyplus_transition_tools': '2.1.4'}
+pkgs = {'energyplus_launch': '3.7.4', 'energyplus_transition_tools': '2.1.4', 'ghedesigner': '1.5'}


This will be more meaningful in the next PR. I could remove it from here if anyone thinks it is necessary.

Myoldmopar · 2025-08-14T19:12:17Z

src/EnergyPlus/UtilityRoutines.cc


-void ShowFatalError(EnergyPlusData &state, std::string const &ErrorMessage, OptionalOutputFileRef OutUnit1, OptionalOutputFileRef OutUnit2)
+[[noreturn]] void
+ShowFatalError(EnergyPlusData &state, std::string const &ErrorMessage, OptionalOutputFileRef OutUnit1, OptionalOutputFileRef OutUnit2)


Marking it [[noreturn]] is really nice because that tells compilers that once you call this function, it's over. And that way you don't have to have workarounds of unused code after calls to ShowFatalError.

Myoldmopar · 2025-08-14T19:36:40Z

Code Integrity failed because the GitHub Action script for Clang Format didn't like that I removed a file. In the calculation of changed files, it was including the deleted files as modified files to check. But those files don't exist anymore. I think adding --diff-filter=AM should be a sufficient addition to fix that. I'll do it in a later commit as needed.

… 2.0

github-actions · 2025-08-29T22:50:35Z

⚠️ Regressions detected on macos-14 for commit `6ecce78`

Regression Summary

EIO: 2
ERR: 2
ESO Big Diffs: 2
Table Small Diffs: 2
Table String Diffs: 2

github-actions · 2025-08-29T23:35:45Z

⚠️ Regressions detected on macos-14 for commit `978ac96`

Regression Summary

EIO: 2
ERR: 2
ESO Big Diffs: 2
Table Small Diffs: 2
Table String Diffs: 2

github-actions · 2025-09-02T03:25:58Z

⚠️ Regressions detected on macos-14 for commit `015e4d7`

Regression Summary

EIO: 2
ERR: 2
ESO Big Diffs: 2
Table Small Diffs: 2
Table String Diffs: 2

Myoldmopar

I laid out a bunch of comments, though the TODO's should all be done now with my latest commit.

Myoldmopar · 2025-09-02T20:17:48Z

.github/workflows/disabled_test_debug_builds.yml.txt

        -DBUILD_PACKAGE:BOOL=OFF
        -DDOCUMENTATION_BUILD:STRING=DoNotBuild
        -DENABLE_OPENMP:BOOL=OFF
-        -DUSE_OpenMP:BOOL=OFF


USE_OpenMP was only here for the CpGFunction library. There is still a separate ENABLE_OPENMP from other third party code.

Myoldmopar · 2025-09-02T20:18:29Z

.github/workflows/test_code_integrity.yml


            begin_group "Run clang-format for changes files"
            # Using \0 as a terminator in case we'd ever have files with spaces
-            git diff -z --name-only HEAD^ HEAD src/ tst/ \


These --diff-filter=AM changes are so that Git doesn't list "removed" files. Removed files should not be checked in the subsequent steps.

Myoldmopar · 2025-09-02T20:18:51Z

cmake/Install.cmake

-  # Otherwise, if either cpgfunctionEP (yes by default) or kiva (no by default) **actually** linked to OpenMP, we need to ship the libs
-  if (NOT ${CMAKE_INSTALL_OPENMP_LIBRARIES} AND (${USE_OpenMP} OR ${ENABLE_OPENMP}))
+  # Otherwise, if kiva (no by default) **actually** linked to OpenMP, we need to ship the libs
+  if (NOT ${CMAKE_INSTALL_OPENMP_LIBRARIES} AND (${ENABLE_OPENMP}))


No more USE_OpenMP, so cleans this logic up.

Myoldmopar · 2025-09-02T20:19:01Z

cmake/PythonInstallCLITools.py


 # expecting one command line argument - the path to the python_lib folder to place the pip package
-pkgs = {'energyplus_launch': '3.7.4', 'energyplus_transition_tools': '2.1.4'}
+pkgs = {'energyplus_launch': '3.7.4', 'energyplus_transition_tools': '2.1.4', 'ghedesigner': '2.0'}


Add in the ghedesigner 2.0 release!

Myoldmopar · 2025-09-02T20:28:32Z

doc/input-output-reference/src/overview/group-condenser-equipment.tex

 \begin{enumerate}
-    \item Use the \hyperref[groundheatexchangerverticalarray]{GroundHeatExchanger:Vertical:Array} object to define a rectangular borehole field. EnergyPlus will compute and cache the g-functions for reuse again in future simulations.
-    \item Use one or more \hyperref[groundheatexchangerverticalsingle]{GroundHeatExchanger:Vertical:Single} objects to define a vertical borehole field with the boreholes located arbitrarily. EnergyPlus will compute and cache the g-functions for reuse again in future simulations.
+    \item Use the \hyperref[groundheatexchangerverticalarray]{GroundHeatExchanger:Vertical:Array} object to define a rectangular borehole field.


OK, so the g function caching would be incredibly problematic.

In the "current" state of things, the g-function depended on the GLHE physical properties. So it was very easy to develop a unique cache key based on the input fields for the GLHE.

But as we are going to be developing GLHE field designs based on plant loads, the field could be different for literally any input that affects the building or systems. The GLHE caching was simply not worth having anymore because every different run would result in a changed g-function. So we have removed g-function caching, which has cleaned up a whole bunch of code.

Myoldmopar · 2025-09-03T12:55:18Z

src/EnergyPlus/GroundHeatExchangers/Base.cc

This file captures as much of the common GLHE base class functionality as possible.

Myoldmopar · 2025-09-03T12:56:35Z

src/EnergyPlus/GroundHeatExchangers/Base.cc

+    }
+}
+
+} // namespace EnergyPlus::GroundHeatExchangers


TODO: Missing newline.

Myoldmopar · 2025-09-03T12:57:19Z

src/EnergyPlus/GroundHeatExchangers/BoreholeArray.cc

+    ShowFatalError(state, "Preceding errors cause program termination");
+}
+
+} // namespace EnergyPlus::GroundHeatExchangers


TODO: Missing newline.

Myoldmopar · 2025-09-03T12:57:58Z

src/EnergyPlus/GroundHeatExchangers/BoreholeSingle.cc

+    ShowFatalError(state, "Preceding errors cause program termination");
+}
+
+} // namespace EnergyPlus::GroundHeatExchangers


TODO: Missing newline...how are these happening?

github-actions · 2025-09-03T14:44:53Z

⚠️ Regressions detected on macos-14 for commit `f0aa8a5`

Regression Summary

EIO: 2
ERR: 2
ESO Big Diffs: 2
Table Small Diffs: 2
Table String Diffs: 2

Myoldmopar · 2025-09-03T15:00:19Z

There are two diff files. I ran the GSHP-GLHE-CalcGFunctions-cpg.idf file in annual mode, reporting things hourly. Here's the borehole temperature response over the year. Noisy plot but take a look at the difference range on the right side y axis:

Over the course of the whole year, the temperature difference peaks at about 0.2 degrees. This appears to simply be because of the slightly different g-function calculated by GHEDesigner compared to CpGFunction, because when I tested this earlier and hardcoded the original CpGFunction-calcualted-g-function, I got no diffs.

I believe this is ready to merge in, so that I can immediately create a new branch that takes this further to allow GHE borefield design. If anyone has thoughts, please let me know soon.

Myoldmopar · 2025-09-04T00:08:33Z

Alright, I'm merging this. I don't anticipate any issue on Windows, but I'll resolve them quickly if anything pops up that wasn't caught here.

Myoldmopar added 6 commits July 28, 2025 08:34

Break GroundHeatExchangers code into a directory of source

9a62239

Break out unit tests into separate source files to match implementation

edb00f7

Cleaning up IDE warnings

7a118a0

Intermediate commit, ready for bringing in new model

f8b7dd8

Cleaning up test failure

20b1084

Merge remote-tracking branch 'origin/develop' into GHEWorkFor252

c51a55a

Myoldmopar commented Aug 14, 2025

View reviewed changes

Myoldmopar added the Refactoring Includes code changes that don't change the functionality of the program, just perform refactoring label Aug 14, 2025

Myoldmopar added 11 commits August 15, 2025 09:43

Just add diff-filter for GHA

3c7b417

Try adding name filter in 2 more places

4226769

Remove commented g-function

2316bb6

Fixups to get ghed g-functions working well

ff136b0

Merge remote-tracking branch 'origin/develop' into GHEWorkFor252

85665f4

Remove cpgfunction entirely

d48ff4d

Eliminating GLHE cache file

ba8945a

Lots of cleanups and robustness added, updated to ghedesigner version…

64ef61e

… 2.0

Merge remote-tracking branch 'origin/develop' into GHEWorkFor252

17d5e12

Remove mentions of USE_OpenMP, as it was a cpgfunction thing

8b92e46

Initialize the cell type variable

7538a7b

Myoldmopar added 4 commits September 1, 2025 20:57

Conditionally run g-function calculation test file and unit test

48b32ed

Merge develop, resolve conflict

e53722c

Compiler warnings

1cd2c44

Unused const variable

b7bf1b3

Myoldmopar added 2 commits September 2, 2025 12:34

Merge remote-tracking branch 'origin/develop' into GHEWorkFor252

16cf8f0

Final polishing, error handling, etc. Should be ready to merge

b194d6d

Myoldmopar commented Sep 3, 2025

View reviewed changes

Myoldmopar merged commit 21a264b into develop Sep 4, 2025
9 checks passed

Myoldmopar deleted the GHEWorkFor252 branch September 4, 2025 00:08

jmarrec mentioned this pull request Sep 5, 2025

Windows fixup for current develop #11191

Merged

20 tasks

Conversation

Myoldmopar commented Aug 14, 2025

Pull request overview

Description of the purpose of this PR

Pull Request Author

Reviewer

Uh oh!

Myoldmopar left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Myoldmopar commented Aug 14, 2025

Uh oh!

github-actions bot commented Aug 29, 2025

⚠️ Regressions detected on macos-14 for commit 6ecce78

Uh oh!

github-actions bot commented Aug 29, 2025

⚠️ Regressions detected on macos-14 for commit 978ac96

Uh oh!

github-actions bot commented Sep 2, 2025

⚠️ Regressions detected on macos-14 for commit 015e4d7

Uh oh!

Myoldmopar left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Sep 3, 2025

⚠️ Regressions detected on macos-14 for commit f0aa8a5

Uh oh!

Myoldmopar commented Sep 3, 2025

Uh oh!

Myoldmopar commented Sep 4, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

⚠️ Regressions detected on macos-14 for commit `6ecce78`

⚠️ Regressions detected on macos-14 for commit `978ac96`

⚠️ Regressions detected on macos-14 for commit `015e4d7`

⚠️ Regressions detected on macos-14 for commit `f0aa8a5`