Update spack_build.yml to fix cmake pulling llvm #698

cameronrutherford · 2024-11-06T20:01:00Z

Fix failing spack pipelines

cameronrutherford · 2024-11-06T20:01:30Z

@nychiang found this potential fix by looking at why llvm was building and disabling relevant package features

cameronrutherford · 2024-11-06T21:20:11Z

.github/workflows/spack_build.yml

@@ -129,7 +129,7 @@ jobs:
          cat << EOF > spack.yaml
          spack:
            specs:
-            - ${{ matrix.spack_spec }} target=x86_64_v2
+            - ${{ matrix.spack_spec }} ^openblas ^cmake~qtgui~ncurses ^libevent~openssl target=x86_64_v2


@nychiang seems like my specification of CMake variant worked for the HiOp package, but not for low level packages like openblas.

Do you want me to try and fix this, or can you try and take it from here?

I can see what you did change here, and it seems like solving the problem.
However, now it introduce more problems, complaining that some packages are not installed:
[email protected]/bp54t5w: PackageNotInstalledError: package not installed
These packages have been pre-builded, haven't them? @cameronrutherford

…, and re-enable build cache

cameronrutherford · 2024-11-07T17:41:37Z

@nychiang now it seems like my openblas specification didn't work properly, so we will have to do a better job enforcing openblas / revert a change I made.

Do you want me to keep debugging this? Are you following what I'm doing/how I'm fixing these issues?

cameronrutherford

I am working on getting the final build to use the right openblas...

cameronrutherford · 2024-11-07T00:30:38Z

.github/workflows/spack_build.yml

@@ -141,10 +141,15 @@ jobs:
                padded_length: False
            mirrors:
              local-buildcache: oci://${{ env.REGISTRY }}/${{ env.IMAGE_NAME }}
-            # spack: https://binaries.spack.io/develop
+              spack: https://binaries.spack.io/develop


Maybe this is a bad idea

cameronrutherford · 2024-11-07T19:23:49Z

.github/workflows/spack_build.yml

-          - hiop@develop~mpi~raja~shared~kron~sparse ^openblas ^libevent~openssl
-          - hiop@develop~mpi+raja~shared~kron~sparse ^openblas ^libevent~openssl
+          - hiop@develop+mpi~raja~shared~kron~sparse ^openmpi 
+          - hiop@develop~mpi~raja~shared~kron~sparse


@nychiang this build is failing because for some reason it's trying to build libflame, and not openblas.

Looks at the concretization comparison between builds, and you'll see that a 3 hour build would make no sense for the smallest build of the batch here.

Why would that spec possibly need python for this build?

python is required via
hiop -> amdlibflame -> aocl-utils -> doxygen -> graphviz -> gtkplus -> meson -> python-xxx
why do we need amdlibflame? Concretization shows that it installs a lot of packages.
@cameronrutherford

cameronrutherford · 2024-11-07T19:34:04Z

.github/workflows/spack_build.yml

-              blas:
-                require: "openblas"


I don't understand why this didn't work. It worked for cmake.

I would consider reporting this as a spack bug, but it's near impossible to debug GitHub actions runners sometimes since they run in a special VM

This didn't work either... Trying more extreme measures.

cameronrutherford · 2024-11-07T19:45:18Z

.github/workflows/spack_build.yml

+            - ${{ matrix.spack_spec }} target=x86_64_v2
+            - openblas
+            - libevent~openssl
            concretizer:
-              reuse: dependencies
+              reuse: false
+              unify: true


Hopefully this gets openblas to always be built...

I got what I wanted... I meant to just have openblas be a part of the concretization, but it appears as though it's always building from source now, and not pulling from the cache. I think reverting to ^openblas in the spec might fix this... I also am trying to better remember where libevent gets pulled in, but I digress.

cameronrutherford · 2024-11-08T03:43:58Z

Guess it's working...

nychiang · 2024-11-08T06:44:08Z

Guess it's working...

A mystery for me. Thanks a lot!

nychiang

LGTM. Just some questions.

nychiang · 2024-11-08T06:46:49Z

.github/workflows/spack_build.yml

+              # https://spack.readthedocs.io/en/latest/packages_yaml.html#setting-requirements-on-virtual-specs
+              cmake:
+                require: "~qtgui~ncurses"


@cameronrutherford how did you figure out these parameters?

So this was part of my initial debugging.

As we are using spack@develop, we are always getting the latest and "greatest" changes. In this case we inherited some changes to CMake that seemed to make it build these options when it wasn't previously.

This forces any build of CMake to have these options disabled.

See this job's concretization step for an example of what I mean. Compare that to a later passing pipeline here. Notice how the updated config results in a much smaller build.

Sure we could probably get the more complicated builds to work too, but a minimal build is faster, and less error prone - we shouldn't build things we don't need.

What pointed me to this was noticing the delta in the concretizations. I looked through the logs, noticed that we were having builds fail on a dependency we don't normally build, and then figured out what package was to blame. Then I worked with that package's spack build config to figure out what to enable / disable.

https://packages.spack.io is a great way to get this information quickly.

nychiang · 2024-11-08T06:48:47Z

.github/workflows/spack_build.yml

+              reuse: true
+              unify: true


@cameronrutherford how did you figure out these?

reuse true is just a sane default. It makes sure you always pull a version of a package from a build cache when possible. There are different options as detailed here.

I was having trouble with the blas provider specification, so I decided to put it in the core spack spec. Then, by specifying unify to be true, I was able to ensure the version of hiop was also built with the version of blas that I wanted.

cameronrutherford · 2024-11-08T16:36:18Z

.github/workflows/spack_build.yml

+        blas_provider:
+          - openblas
+        compiler:
+          - gcc


Feel free to add other variants of your current build, or add compiler / blas specifications as wanted.

Would be easy enough to extend this to build / test with different MPI providers as well.

cameronrutherford · 2024-11-08T16:37:05Z

.github/workflows/spack_build.yml

+              cmake:
+                # We don't need CMake GUI features for minimal container builds
+                require: "~qtgui~ncurses"
+              libevent:
+                # Building OpenSSL was causing errors
+                require: "~openssl"


I was finding spack's ^ syntax in the spec to not always specify variants as I wanted. This is a more sustainable way to always enforce certain options on desired pacakges.

cameronrutherford · 2024-11-08T16:44:54Z

Some final notes:

Disable the spack public mirror as you have issues. Normally issues are reported and fixed relatively soon after you notice them, but feel free to ping the spack slack / make an issue
Checking for external packages and concretizing are still the longest parts of the jobs - this is possible to speed up, but the tradeoff is that you wouldn't be getting rolling spack updates
Feel free to extend these builds to other platforms / GPU runners / MPIs / compilers / OSs as you find convenient
Logging is currently quite verbose. You can minimize / maximize it as needed
The GHCR registry you have configured might need to be cleared out once every so often. Re-generating it from scratch isn't so bad, just will have initial builds take a while

Update spack_build.yml to fix cmake pulling llvm

ac20b3d

cameronrutherford commented Nov 6, 2024

View reviewed changes

Update spack_build.yml to specify cmake requirements for all packages…

4315931

…, and re-enable build cache

cameronrutherford commented Nov 7, 2024

View reviewed changes

Move openblas into direct package spec

0c1e55f

cameronrutherford commented Nov 7, 2024

View reviewed changes

Specify openblas in a different way

a486ada

cameronrutherford commented Nov 7, 2024

View reviewed changes

Try re-using builds for quicker pipelines

7ba7b00

nychiang approved these changes Nov 8, 2024

View reviewed changes

Add some comments and move libevent version spec

d07cab2

cameronrutherford commented Nov 8, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update spack_build.yml to fix cmake pulling llvm #698

Update spack_build.yml to fix cmake pulling llvm #698

cameronrutherford commented Nov 6, 2024

cameronrutherford commented Nov 6, 2024

cameronrutherford Nov 6, 2024

nychiang Nov 7, 2024 •

edited

Loading

cameronrutherford commented Nov 7, 2024

cameronrutherford left a comment

cameronrutherford Nov 7, 2024

cameronrutherford Nov 7, 2024

cameronrutherford Nov 7, 2024 •

edited

Loading

nychiang Nov 7, 2024

cameronrutherford Nov 7, 2024

cameronrutherford Nov 7, 2024

cameronrutherford Nov 7, 2024

cameronrutherford Nov 7, 2024

cameronrutherford commented Nov 8, 2024

nychiang commented Nov 8, 2024

nychiang left a comment

nychiang Nov 8, 2024

cameronrutherford Nov 8, 2024

cameronrutherford Nov 8, 2024

nychiang Nov 8, 2024

cameronrutherford Nov 8, 2024

cameronrutherford Nov 8, 2024

cameronrutherford Nov 8, 2024

cameronrutherford commented Nov 8, 2024 •

edited

Loading

Update spack_build.yml to fix cmake pulling llvm #698

Are you sure you want to change the base?

Update spack_build.yml to fix cmake pulling llvm #698

Conversation

cameronrutherford commented Nov 6, 2024

cameronrutherford commented Nov 6, 2024

Choose a reason for hiding this comment

nychiang Nov 7, 2024 • edited Loading

Choose a reason for hiding this comment

cameronrutherford commented Nov 7, 2024

cameronrutherford left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cameronrutherford Nov 7, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cameronrutherford commented Nov 8, 2024

nychiang commented Nov 8, 2024

nychiang left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cameronrutherford commented Nov 8, 2024 • edited Loading

nychiang Nov 7, 2024 •

edited

Loading

cameronrutherford Nov 7, 2024 •

edited

Loading

cameronrutherford commented Nov 8, 2024 •

edited

Loading