Skip to content

refactor: Add instance generators for constitutive drivers#3687

Open
dkachuma wants to merge 10 commits into
developfrom
dkachuma/refactor/auto-generate-pvt-driver-kernels
Open

refactor: Add instance generators for constitutive drivers#3687
dkachuma wants to merge 10 commits into
developfrom
dkachuma/refactor/auto-generate-pvt-driver-kernels

Conversation

@dkachuma

@dkachuma dkachuma commented Jun 2, 2025

Copy link
Copy Markdown
Contributor

This PR replaces hardcoded constitutive driver test instances with an auto-generated CMake/JSON workflow. This significantly reduces boilerplate and makes adding new fluid or relative permeability models as simple as adding a single line to a JSON file.

  • Converted hardcoded driver test files into generic CMake templates (PVTDriverRunTestFluid.cpp.template, RelpermDriverRunTestRelPerm.cpp.template, and DriverDispatchTypeList.hpp.template).
  • Added kernelSpecs.json to centrally define and manage all fluid and relative permeability model combinations.
  • Modified the generateKernels function in GeosxMacros.cmake to accept a LIST_TEMPLATE argument for custom dispatch type lists.
  • Deleted redundant .cpp files in the constitutiveDrivers module, as these are now dynamically generated during the build step.

@dkachuma dkachuma self-assigned this Jun 2, 2025
@dkachuma dkachuma added ci: run CUDA builds Allows to triggers (costly) CUDA jobs ci: run integrated tests Allows to run the integrated tests in GEOS CI flag: no rebaseline Does not require rebaseline ci: run code coverage enables running of the code coverage CI jobs labels Jun 2, 2025
@dkachuma dkachuma marked this pull request as ready for review June 2, 2025 20:16
@codecov

codecov Bot commented Jun 2, 2025

Copy link
Copy Markdown

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 57.56%. Comparing base (c2768e6) to head (9aa9dbd).

Additional details and impacted files
@@           Coverage Diff            @@
##           develop    #3687   +/-   ##
========================================
  Coverage    57.56%   57.56%           
========================================
  Files         1235     1235           
  Lines       106973   106973           
========================================
  Hits         61578    61578           
  Misses       45395    45395           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@dkachuma dkachuma marked this pull request as draft June 17, 2026 00:32
@dkachuma dkachuma marked this pull request as ready for review June 17, 2026 00:56

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR refactors constitutive driver “runTest” explicit-instantiation boilerplate by switching from many hand-written .cpp instantiation translation units to CMake-driven generation from .cpp.template files plus a JSON spec. This is intended to make adding new constitutive models simpler and more uniform.

Changes:

  • Replace per-model explicit instantiation .cpp files (PVT + relperm) with generated sources driven by kernelSpecs.json.
  • Add driver-specific dispatch typelist template generation (DriverDispatchTypeList.hpp.template) and wire it into constitutiveDrivers/CMakeLists.txt.
  • Extend generateKernels() to accept an optional LIST_TEMPLATE for dispatch type list generation.

Reviewed changes

Copilot reviewed 23 out of 23 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
src/coreComponents/constitutiveDrivers/relativePermeability/RelpermDriverVanGenuchtenStone2RunTest.cpp Removed explicit instantiation TU (now intended to be generated).
src/coreComponents/constitutiveDrivers/relativePermeability/RelpermDriverVanGenuchtenBakerRunTest.cpp Removed explicit instantiation TU (now intended to be generated).
src/coreComponents/constitutiveDrivers/relativePermeability/RelpermDriverTableRelativeRunTest.cpp Removed explicit instantiation TU (now intended to be generated).
src/coreComponents/constitutiveDrivers/relativePermeability/RelpermDriverTableRelativeHysteresisRunTest.cpp Removed explicit instantiation TU (previously also carried hysteresis trait specialization).
src/coreComponents/constitutiveDrivers/relativePermeability/RelpermDriverRunTestRelPerm.cpp.template New template for generated relperm runTest explicit instantiation.
src/coreComponents/constitutiveDrivers/relativePermeability/RelpermDriverBrooksCoreyStone2RunTest.cpp Removed explicit instantiation TU (now intended to be generated).
src/coreComponents/constitutiveDrivers/relativePermeability/RelpermDriverBrooksCoreyRunTest.cpp Removed explicit instantiation TU (now intended to be generated).
src/coreComponents/constitutiveDrivers/kernelSpecs.json Adds JSON spec listing relperm + fluid model instantiation combinations.
src/coreComponents/constitutiveDrivers/fluid/multiFluid/PVTDriverRunTestFluid.cpp.template New template for generated PVT runTest explicit instantiation.
src/coreComponents/constitutiveDrivers/fluid/multiFluid/constant/PVTDriverRunTestInvariantImmiscibleFluid.cpp Removed explicit instantiation TU (now intended to be generated).
src/coreComponents/constitutiveDrivers/fluid/multiFluid/compositional/PVTDriverRunTestCompositionalTwoPhasePhillipsBrine.cpp Removed explicit instantiation TU (now intended to be generated).
src/coreComponents/constitutiveDrivers/fluid/multiFluid/compositional/PVTDriverRunTestCompositionalTwoPhaseLohrenzBrayClarkViscosity.cpp Removed explicit instantiation TU (now intended to be generated).
src/coreComponents/constitutiveDrivers/fluid/multiFluid/compositional/PVTDriverRunTestCompositionalTwoPhaseConstantViscosity.cpp Removed explicit instantiation TU (now intended to be generated).
src/coreComponents/constitutiveDrivers/fluid/multiFluid/compositional/PVTDriverRunTestCompositionalThreePhaseLohrenzBrayClarkViscosity.cpp Removed explicit instantiation TU (now intended to be generated).
src/coreComponents/constitutiveDrivers/fluid/multiFluid/compositional/PVTDriverRunTestCompositionalKValuePhillipsBrine.cpp Removed explicit instantiation TU (now intended to be generated).
src/coreComponents/constitutiveDrivers/fluid/multiFluid/compositional/PVTDriverRunTestCompositionalKValueLohrenzBrayClarkViscosity.cpp Removed explicit instantiation TU (now intended to be generated).
src/coreComponents/constitutiveDrivers/fluid/multiFluid/CO2Brine/PVTDriverRunTestCO2BrinePhillipsThermalFluid.cpp Removed explicit instantiation TU (now intended to be generated).
src/coreComponents/constitutiveDrivers/fluid/multiFluid/CO2Brine/PVTDriverRunTestCO2BrinePhillipsFluid.cpp Removed explicit instantiation TU (now intended to be generated).
src/coreComponents/constitutiveDrivers/fluid/multiFluid/CO2Brine/PVTDriverRunTestCO2BrineEzrokhiThermalFluid.cpp Removed explicit instantiation TU (now intended to be generated).
src/coreComponents/constitutiveDrivers/fluid/multiFluid/blackOil/PVTDriverRunTestDeadOilFluid.cpp Removed explicit instantiation TU (now intended to be generated).
src/coreComponents/constitutiveDrivers/DriverDispatchTypeList.hpp.template Adds constitutiveDrivers-specific dispatch typelist template for generated headers.
src/coreComponents/constitutiveDrivers/CMakeLists.txt Wires kernel generation into constitutiveDrivers build and removes per-model instantiation sources.
src/cmake/GeosxMacros.cmake Extends generateKernels() with LIST_TEMPLATE selection for dispatch type list generation.
Comments suppressed due to low confidence (1)

src/coreComponents/constitutiveDrivers/relativePermeability/RelpermDriverRunTestRelPerm.cpp.template:27

  • RelpermDriver::runTest relies on the HasHysteresis<RELPERM_TYPE> trait to decide whether to populate historical saturation fields. The previous dedicated instantiation TU for TableRelativePermeabilityHysteresis provided a specialization setting this trait to true, but the new generated instantiation file template does not, so hysteresis run-tests will silently run with the non-hysteresis path.

Comment thread src/coreComponents/constitutiveDrivers/kernelSpecs.json
dkachuma and others added 2 commits June 16, 2026 20:16
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
* Initial plan

* Fix typos in kernelSpecs embedded docs

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 24 out of 24 changed files in this pull request and generated 2 comments.

relativePermeability/RelpermDriverRunTestRelPerm.cpp.template)

foreach( kernelTemplateFile ${kernelTemplateFileList} )
get_filename_component( jsonKey ${kernelTemplateFile} NAME_WE )
Comment on lines 28 to +33
// Hysteresis traits
template< typename RELPERM_TYPE >
template< typename RELPERM_TYPE, typename = void >
struct HasHysteresis : std::false_type {};

template< typename RELPERM_TYPE >
struct HasHysteresis< RELPERM_TYPE, std::void_t< decltype(RELPERM_TYPE::viewKeyStruct::phaseHasHysteresisString()) > > : std::true_type {};

@joshua-white joshua-white left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci: run code coverage enables running of the code coverage CI jobs ci: run CUDA builds Allows to triggers (costly) CUDA jobs ci: run integrated tests Allows to run the integrated tests in GEOS CI flag: no rebaseline Does not require rebaseline flag: ready for review type: cleanup / refactor Non-functional change (NFC)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants