Submodule Tribol included in default build by white238 · Pull Request #1633 · llnl/smith

white238 · 2026-07-15T19:01:36Z

Gonna be honest... this PR got away from me but here is a list of things that were needed and got cleaned up in what was supposed to be easy:

Swapped Tribol to be a submodule in the default build to assist quick development cycle (can still be provided with setting TRIBOL_DIR to a pre-built Tribol)
Added new cmake option SMITH_ENABLE_TRIBOL to turn off Tribol entirely, which defaults to ON
Improved importing of CMake targets (arpack and mfem)
Fixed long-standing issue where Axom lost it's includes in codevelop build (we were stripping non-existent includes which mistakenly stripped directories guarded by generator expressions)
Unified TPL list to one place
Stopped exporting the mfem target and changed it to recreating the target as an imported library downstream this simplifies the CMake export set
Updated Tribol calls to new API
Improved building skill to not force updating submodules (my bad)
Bumped Tribol (includes @ebchin 's fix and multiple build error/warning improvements)
Update functional_tet_quality to use FiniteElementState and rename it to tet_quality

chapman39 · 2026-07-28T00:30:46Z

should we update tribol in this pr as well?

white238 · 2026-07-28T17:49:40Z

should we update tribol in this pr as well?

Yea i will update this branch.

…y default, still allow Tribol to be separate when given TRIBOL_DIR

…fem, always create a mfem target instead of exporting it

Copilot

Pull request overview

This PR updates Smith’s build and packaging to include Tribol as part of the default (Smith-only) build workflow, adds a toggle to disable Tribol entirely, and refactors several third-party (TPL) integration paths (MFEM/ARPACK/Tribol) to simplify downstream consumption and improve robustness.

Changes:

Make Tribol a default-build submodule with a new SMITH_ENABLE_TRIBOL option to disable Tribol support when desired.
Refactor CMake third-party setup/import logic (MFEM + ARPACK + Tribol) and unify the TPL list in one place.
Update Spack recipes, host-configs, CI config, docs, and call sites to match the new build and Tribol API behavior.

Reviewed changes

Copilot reviewed 25 out of 25 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
src/smith/physics/contact/contact_interaction.cpp	Updates Tribol call sites to new MFEM-facing Tribol API.
src/docs/sphinx/build_guide/build_smith.rst	Documents new `SMITH_ENABLE_TRIBOL` CMake option.
skills/building/SKILL.md	Updates build workflow guidance to reflect new default submodule set.
scripts/spack/spack_repo/smith/packages/tribol/package.py	Bumps Tribol Spack version/commit.
scripts/spack/spack_repo/smith/packages/smith/package.py	Adjusts Smith Spack variant semantics for external Tribol usage.
scripts/spack/configs/versions.yaml	Updates required Tribol version to match bump.
host-configs/tuolumne-toss_4_x86_64_ib_cray-llvm-amdgpu@6.4.2_hip.cmake	Removes external `TRIBOL_DIR` setting in favor of default-included Tribol behavior.
host-configs/rzwhippet-toss_4_x86_64_ib-llvm@19.1.3.cmake	Removes external `TRIBOL_DIR` setting in favor of default-included Tribol behavior.
host-configs/rzwhippet-toss_4_x86_64_ib-gcc@13.3.1.cmake	Removes external `TRIBOL_DIR` setting in favor of default-included Tribol behavior.
host-configs/rzadams-toss_4_x86_64_ib_cray-llvm-amdgpu@6.4.2_hip.cmake	Removes external `TRIBOL_DIR` setting in favor of default-included Tribol behavior.
host-configs/other/firion-macos_sonoma_aarch64-llvm@19.1.7.cmake	Removes external `TRIBOL_DIR` setting in favor of default-included Tribol behavior.
host-configs/docker/llvm@19.1.1.cmake	Removes external `TRIBOL_DIR` setting in favor of default-included Tribol behavior.
host-configs/docker/gcc@14.2.0.cmake	Removes external `TRIBOL_DIR` setting in favor of default-included Tribol behavior.
host-configs/dane-toss_4_x86_64_ib-llvm@19.1.3.cmake	Removes external `TRIBOL_DIR` setting in favor of default-included Tribol behavior.
host-configs/dane-toss_4_x86_64_ib-gcc@13.3.1.cmake	Removes external `TRIBOL_DIR` setting in favor of default-included Tribol behavior.
examples/contact/ironing_2D.cpp	Updates example mesh builder call to new helper name.
CMakeLists.txt	Removes workaround Axom include injection in codevelop install/export path.
cmake/thirdparty/SetupSmithThirdParty.cmake	Reworks submodule vs prebuilt Tribol selection, improves include-dir sanitization, and centralizes TPL list.
cmake/thirdparty/FindMFEM.cmake	Improves MFEM feature detection and MFEM link interface handling (MPI/OpenMP/Tribol flags).
cmake/thirdparty/FindARPACK.cmake	Adds imported target creation for ARPACK for easier downstream linkage.
cmake/SmithConfigHeader.cmake	Uses centralized SMITH TPL list to generate `SMITH_USE_*` variables.
cmake/SmithBasics.cmake	Adds `SMITH_ENABLE_TRIBOL` option definition.
cmake/smith-config.cmake.in	Updates installed package config logic (ARPACK target, MFEM target recreation, conditional Tribol targets).
.gitlab/build_toss4.yml	Updates CI job to disable Tribol via `SMITH_ENABLE_TRIBOL=OFF`.

Suppressed comments (2)

cmake/smith-config.cmake.in:178

This find_dependency call has a duplicated REQUIRED keyword, which will cause a CMake argument parsing error when SMITH_USE_ENZYME is ON.

    find_dependency(Enzyme REQUIRED REQUIRED NO_DEFAULT_PATH PATHS "${ENZYME_DIR}")

cmake/smith-config.cmake.in:161

blt_import_library is being given "@MFEM_LIBRARIES@" via DEPENDS_ON, but those are raw library paths/flags (not CMake targets). Use the LIBRARIES argument to ensure correct linking and consistency with FindMFEM.cmake.

      blt_import_library(NAME mfem
                         INCLUDES "@MFEM_INCLUDE_DIRS@"
                         TREAT_INCLUDES_AS_SYSTEM ON
                         DEPENDS_ON "@MFEM_LIBRARIES@")

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

chapman39 · 2026-08-21T00:28:33Z

+    # Submodule Third Party Libraries
+    #
+    # These are included in the regular build of Smith due to the close and
+    # tied development cycles.


chapman39 · 2026-08-21T00:29:38Z

-        add_subdirectory("${CONTINUATION_SOURCE_DIR}" ${CMAKE_BINARY_DIR}/ContinuationSolvers)
+    # mfem brings in slepc but doesn't include required arpack, add it
+    if(ARPACK_FOUND)
+        target_link_libraries(mfem INTERFACE arpack)


thats a lot cleaner

chapman39 · 2026-08-21T00:40:09Z


 endif()
+
+# This is a full list of possible TPLs in Smith and is used in multiple locations


might be worth moving this to the top of the file and explaining where its being used

chapman39 · 2026-08-21T00:42:36Z

 set(UMPIRE_DIR "${TPL_ROOT}/umpire-2025.12.0-zz6fwo4ec7wgiabtpvzjkab3reoj4qzk" CACHE PATH "")

-set(TRIBOL_DIR "${TPL_ROOT}/tribol-0.1.0.27-chz6xomld64xnob5kh4yk3tkkq5gnroy" CACHE PATH "")
+# Tribol not built


so from now on, will we be not testing tribol as part of the TPL builds? i guess its further motivation for me to add TPL build test in tribol CI.

Correct. Unless we want to build it then turn off the TRIBOL_DIR variable in our host-configs...

chapman39

thanks chris!!

ebchin

😍

…sitives

white238 · 2026-08-26T00:07:19Z

@@ -4,25 +4,26 @@
 //


@kswartz92 Could you verify this is what you meant?

this looks conceptually correct to me. using the FiniteElementState, setting the field directly. We'll see if it addresses the underlying issue.

yeah looks good

ebchin force-pushed the task/white238/submodule_tribol branch from b3ee4ae to 5126142 Compare August 3, 2026 21:51

white238 force-pushed the task/white238/submodule_tribol branch from 57e5bae to 164360a Compare August 17, 2026 20:58

white238 requested review from btalamini, chapman39, ebchin and tupek2 and a lite review from Copilot August 21, 2026 00:02

white238 changed the title ~~WIP: Submodule Tribol included in default build~~ Submodule Tribol included in default build Aug 21, 2026

white238 added 20 commits August 20, 2026 17:08

temporarily quiet warning

9f7dbdb

reorder builtin thirdparty libraries and have Tribol build included b…

6fea538

…y default, still allow Tribol to be separate when given TRIBOL_DIR

turn off external tribol

d3e4b2b

update building skills to not reference tribol in the codevelop build

4d2e6b8

add more flags to quiet

36c50bb

bump tribol

f59e426

pull in some fixes from tribol's find mfem

67a6a4c

update to new tribol api

f8fe16c

turn back on warnings that were temporarily turned of

96b1785

turn off tribol in all host-configs

4e1ff30

change smith tribol variant

ca74b93

bump tribol package sha

78cd8d2

allow tribol to be turned off completely

2de6925

add docs

c6941b2

typo

f4e3fa8

wrong language

4989dd8

bump tribol again

5485bfc

turn off tribol fortran support

c6aa255

do not force submodule udpate

79174c7

more bumping

5b96e8c

white238 added 9 commits August 20, 2026 17:08

api change

e9faa44

attempt to move mfem out of the export set

4b18ab7

use blt macro

22448ef

various TPL clean up, create an arpack target and have it depend on m…

31e7a95

…fem, always create a mfem target instead of exporting it

sigh

6cb2a28

guard against no tribol

b577bce

put mfem target back

b5ee8af

do not remove includes that are surrounded by generator expressions

c39822e

bump tribol for hopefully the last time

da2d9dc

white238 force-pushed the task/white238/submodule_tribol branch from 772bc64 to da2d9dc Compare August 21, 2026 00:08

Copilot started reviewing on behalf of white238 August 21, 2026 00:08 View session

Copilot AI reviewed Aug 21, 2026

View reviewed changes

Comment thread cmake/thirdparty/SetupSmithThirdParty.cmake

Comment thread cmake/thirdparty/FindARPACK.cmake Outdated

Comment thread host-configs/docker/gcc@14.2.0.cmake

Comment thread cmake/smith-config.cmake.in Outdated

chapman39 reviewed Aug 21, 2026

View reviewed changes

Comment thread cmake/thirdparty/FindMFEM.cmake

chapman39 reviewed Aug 21, 2026

View reviewed changes

Comment thread cmake/thirdparty/FindMFEM.cmake

chapman39 reviewed Aug 21, 2026

View reviewed changes

chapman39 approved these changes Aug 21, 2026

View reviewed changes

ebchin approved these changes Aug 21, 2026

View reviewed changes

tupek2 approved these changes Aug 21, 2026

View reviewed changes

white238 added 2 commits August 24, 2026 14:38

address comments

1f72278

turn off warnings as errors when enabling asan in gcc due to false po…

7b0fef7

…sitives

white238 force-pushed the task/white238/submodule_tribol branch 2 times, most recently from 5608acd to 5b2c859 Compare August 25, 2026 17:24

white238 added 2 commits August 25, 2026 16:56

rewrite functional_tet_quality test to use FiniteElementState

6b41edf

use slic and std::format

bb58c9c

white238 force-pushed the task/white238/submodule_tribol branch from 5b2c859 to bb58c9c Compare August 26, 2026 00:02

white238 commented Aug 26, 2026

View reviewed changes


		endif()

		# This is a full list of possible TPLs in Smith and is used in multiple locations

Uh oh!

Conversation

white238 commented Jul 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chapman39 commented Jul 28, 2026

Uh oh!

white238 commented Jul 28, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

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!

chapman39 left a comment

Choose a reason for hiding this comment

Uh oh!

ebchin 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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

white238 commented Jul 15, 2026 •

edited

Loading