Skip to content

Improve external builds/update polytope#337

Merged
ldowen merged 43 commits into
developfrom
bugfix/improve_external_builds
May 12, 2025
Merged

Improve external builds/update polytope#337
ldowen merged 43 commits into
developfrom
bugfix/improve_external_builds

Conversation

@ldowen
Copy link
Copy Markdown
Collaborator

@ldowen ldowen commented Apr 9, 2025

Summary

  • This PR updates polytope and the documentation regarding building Spheral
  • It does the following:
    • Uses the find_package logic on polytope
    • Improves the documentation for building Spheral
    • Improvements to tpl-manager and host-config-build scripts

ToDo :

  • Annotate RELEASE_NOTES.md with notable changes.
  • Create LLNLSpheral PR pointing at this branch. (PR#131)
  • LLNLSpheral PR has passed all tests.

…cretizes by default, added cmake logic to use find_package on polytope, updated polytoper version, added check for spheralutils python version, overhaul docs, changed collections module to collections.abc
@ldowen ldowen requested review from jmikeowen and mdavis36 April 9, 2025 00:18
@ldowen ldowen self-assigned this Apr 9, 2025
ldowen added 26 commits April 8, 2025 17:19
…system library to ignore warnings during the build
…pec to environments, removed renaming export of polytope target
…s, added SpheralConfig timer flag, only turn off mpi finalize if timers are enabled for the build
…hange SpheralConfigs, turn off mpi4py finalize if LEOS is enabled
… packages for spack to search for in the tpl-manager
…they cause issues with newer python versions
…t set mpi install that is found as the provider instead of modifying the spec
jmikeowen
jmikeowen previously approved these changes May 6, 2025
Copy link
Copy Markdown
Collaborator

@jmikeowen jmikeowen left a comment

Choose a reason for hiding this comment

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

Looks good to me -- nice job updating the docs!

@mdavis36 mdavis36 added this to the 2025.06.0 Release milestone May 7, 2025
Comment thread CMakeLists.txt
@@ -1,6 +1,6 @@
# CMakeLists to build the Spheral library.

cmake_minimum_required(VERSION 3.14)
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.

Is there an explicit cmake 3.21 feature we need to bump this?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

You'll need CMake 3.21 to work with Hip and the latest BLT release.

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.

Probably but this PR isn't bringing in any HIP compiled code, we tried bringing this version up from 3.14 to 3.18 a while ago and had issues with BLT and CUDA. The consensus after speaking with the BLT folks was to revert back to 3.14 until we had a necessity to push the version. When HIP executing changes get pushed to Spheral is probably when we will revisit our minimum version constraint.

Copy link
Copy Markdown
Contributor

@mdavis36 mdavis36 left a comment

Choose a reason for hiding this comment

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

I tried this out today on a fresh Fedora 42 install. I was unable to install the Spheral dependencies through spack:

  1. Fedora 42 is on Python 3.13. Our current version of Spack fails to bootstrap clingo. (Since this isn't necessarily our fault I manually built and installed Python 3.11 to get past this).
  2. dnf on Fedora seems to install packages in 3 places e.g. w/ cmake /bin/cmake, /usr/bin/cmake and /usr/sbin/cmake. Spack successfully finds all 3, however the paths it stores in the yaml file are (/bin, /usr, /usr/sbin). During the builds spack tries to build with the path /usr/sbin/bin/cmake which does not exist and causes all packages that build with cmake to fail.
  3. spack isn't picking up some packages I would expect ncurses, curl, wget, zlib many of which are later failing to build with the gcc@15.1.1 for Fedora 42. When I explicitly spack find some of these it is able to pick them up.

This is a shame because it seems most of these issues are out of our control and are problems with spack / it's package recipes.

There are a few improvements that I think we can address in the near future:

  1. When building externally it's not always clear which packages spack has found and exactly where it is pointing to them. It would be nice to have some output communicating this.
  2. I think instead of letting spack implicitly find everything on the system we should define a set of packages we always expect to be on the system like ncurses and explicitly search for them (like we do for python perkl and mpi right now), especially since we have users install some of those packages with their respective package managers.

This PR is a step in the right direction for external users but we aren't there yet unfortunately. I don't think we need to address any of the above right now. We should update to the spack 1.0 and see where things settle, but we will probably need to work with the spack team to resolve some of the above.

Comment thread docs/build_guide/external/index.rst
Comment thread docs/build_guide/external/index.rst
@ldowen ldowen merged commit 1edf186 into develop May 12, 2025
2 checks passed
@ldowen ldowen deleted the bugfix/improve_external_builds branch May 12, 2025 22:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants