Skip to content

Conversation

@chapman39
Copy link
Collaborator

@chapman39 chapman39 commented Dec 16, 2025

this pr

about CUDA

  • i fixed to build issues, but we run out of space on github runners, so it's still disabled
  • however, the container works and is up to date, so i'll leave it in this PR

related PRs

next

  • update smith cz and lido
  • update tribol's docker containers to include cuda as well

@chapman39 chapman39 self-assigned this Dec 16, 2025
@chapman39 chapman39 added the TPL Third-party libraries label Dec 16, 2025
@chapman39 chapman39 force-pushed the feature/chapman39/tpl-2025-12-15 branch from a44e1d3 to 09bb31f Compare December 18, 2025 17:23
@chapman39 chapman39 force-pushed the feature/chapman39/tpl-2025-12-15 branch from 09bb31f to 18cf423 Compare December 18, 2025 17:25
@white238
Copy link
Member

Heads up for you @chapman39 , Axom changed their default type for index:

https://github.com/llnl/axom/blob/623a1d9d139176737e2ce60c17c06cc2d2d40e89/RELEASE-NOTES.md?plain=1#L169

You will get a lot of warnings as errors.

@chapman39 chapman39 mentioned this pull request Dec 30, 2025
@chapman39 chapman39 marked this pull request as ready for review January 13, 2026 19:18
version: [7.2.6]
externals:
- spec: py-sphinx@7.2.6
- spec: py-sphinx@7.2.6 ^python
Copy link
Member

Choose a reason for hiding this comment

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

This is weird.

prefix: /usr
openblas:
buildable: false
# Spack is insistent on building openblas in the CUDA container, so allow building from source
Copy link
Member

Choose a reason for hiding this comment

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

Should this be removed then? Why have an external when we are building it?

prefix: /usr/tce
- spec: cmake@3.30.5
modules:
- cmake/3.30.5
Copy link
Member

Choose a reason for hiding this comment

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

Is this better than putting a hard-coded path like everything else?

Comment on lines 10 to 14
"""Axom provides a robust, flexible software infrastructure for the development
of multi-physics applications and computational tools."""

maintainers("white238")

homepage = "https://github.com/LLNL/axom"
git = "https://github.com/LLNL/axom.git"
Copy link
Member

Choose a reason for hiding this comment

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

Nitpick: The description, homepage, and git can be removed.


class Fmt(BuiltinFmt):
# Fix NVCC error
patch("system_error_cuda.patch", when="@12.1.0:")
Copy link
Member

Choose a reason for hiding this comment

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

Is this the same issue Alan and Axom was running into? Btw Axom just updated their fmt to 12.1.0 as well.

with when("+sundials"):
# Going to sundials@7: causes 80%+ test failures
depends_on("sundials@:6.999")
depends_on("sundials")
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
depends_on("sundials")

w/o the version this line isn't needed anymore.

Comment on lines +197 to +198
depends_on("caliper+mpi~papi", when="+caliper")
depends_on("caliper+adiak", when="+caliper+adiak")
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
depends_on("caliper+mpi~papi", when="+caliper")
depends_on("caliper+adiak", when="+caliper+adiak")
with when("+caliper")
depends_on("caliper+mpi~papi")
depends_on("caliper+adiak", when="+adiak")

I think this is cleaner...


set_source_files_properties(functional_tet_quality PROPERTIES LANGUAGE CXX)

# Then add the examples/tests
Copy link
Member

Choose a reason for hiding this comment

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

This is great you verified that overiding these languages were no lnoger needed.. thanks!

{
const auto currentConductivity =
conductivity_offset_ + get<0>(parameter) + d_conductivity_d_temperature_ * temperature;
#if defined(SMITH_USE_CUDA) || defined(SMITH_USE_HIP)
Copy link
Member

Choose a reason for hiding this comment

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

We might want to create a SMITH_USE_GPU that encompasses both of these. We should review RAJA's patterns

Copy link
Member

@white238 white238 left a comment

Choose a reason for hiding this comment

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

All my comments were nitpicking and I don't want to hold up this being merged after someone else's review. Thanks for the hard slog @chapman39 !

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

Labels

TPL Third-party libraries

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants