Skip to content

Remove unused options from "spack stack" extension and add logic to set compiler for "spack stack create env"#1192

Merged
climbfuji merged 33 commits intoJCSDA:developfrom
climbfuji:feature/set_env_compiler
Jul 27, 2024
Merged

Remove unused options from "spack stack" extension and add logic to set compiler for "spack stack create env"#1192
climbfuji merged 33 commits intoJCSDA:developfrom
climbfuji:feature/set_env_compiler

Conversation

@climbfuji
Copy link
Copy Markdown
Collaborator

@climbfuji climbfuji commented Jul 10, 2024

Summary

This PR removes unused options for the "spack stack" extension in an order to clean up the amount of code and facilitate adding the new capability described below.

The new capability requires the user to define a compiler --compiler when running spack stack create env. It further updates the logic for creating common and site environment spack config files for modules and packages. For both common and site configs (in configs/common/ and configs/sites/tier?/*/), the following is true:

  1. For packages: If both packages.yaml and packages_${COMPILER}.yaml exist in the same directory, merge the two (packages_${COMPILER}.yaml overrides packages.yaml if necessary) and write to the environment's common or site config as packages.yaml. See for example the updated blackpearl site config in this PR. If only one of them exists, copy that file to the destination.
  2. For modules: If both modules.yaml and packages_${LUA_OR_LMOD}.yaml exist in the same directory, merge the two (packages_${LUA_OR_LMOD}.yaml overrides modules.yaml if necessary) and write to the environment's common or site config as modules.yaml. This is currently not the case, but maybe we can consolidate some of the contents into one default modules.yaml file?

In the environment spack.yaml, update the compiler definition line for the matrix (if applicable) and set packages:all:require:['%COMPILER']. This requires that packages that must be built with other compilers (e.g. gcc-runtime) have their own require: '%COMPILER' setting to overwrite this. This is already the case for gcc-runtime in configs/common/packages.yaml. The same may be needed for other runtime packages for each of the compilers in the future, and for packages like bison that don't build with oneapi for example. I am basically following what E4S does.

Testing

  • Created unified-env and empty environments with both gcc and oneapi on blackpearl, concretized and verified that the blas/lapack/fftw-api providers are correct, the correct compilers are being used, and that no conflicts are reported (except the known/greenlighted ones).
  • CI (this represents testing new site configs for different compilers and OSs)
  • Built neptune-env on Nautilus with [email protected] using the existing site config (i.e. no changes necessary). Then built NEPTUNE with the same compiler and ran basic tests.
  • Built intel-impi container with jedi-ci specs

Applications affected

None

Systems affected

None

Dependencies

Issue(s) addressed

Resolves #1167
Resolves #1126

Checklist

  • This PR addresses one issue/problem/enhancement, or has a very good reason for not doing so.
  • These changes have been tested on the affected systems and applications.
  • All dependency PRs/issues have been resolved and this PR can be merged.

@climbfuji
Copy link
Copy Markdown
Collaborator Author

@AlexanderRichert-NOAA I am keeping this in draft for the moment, but I would really like to ask you for your feedback as I've been rather aggressive with removing unused options in our spack stack extension.

The new feature to set the compiler in spack stack create env works very well for as far as I have tested.

@climbfuji
Copy link
Copy Markdown
Collaborator Author

Of course, I am going to fix the unit tests and update the documentation. But I won't do that until we've agreed on the actual changes.

@AlexanderRichert-NOAA
Copy link
Copy Markdown
Collaborator

Okay, thanks for the headsup. So far so good I think. I don't really use any of those, and it seems like they're things that can be dealt with manually by copying files around. If you can think of an exception to that let me know.

@climbfuji climbfuji force-pushed the feature/set_env_compiler branch 6 times, most recently from 6dddc02 to 04bfac5 Compare July 10, 2024 22:43
@climbfuji climbfuji force-pushed the feature/set_env_compiler branch from 04bfac5 to e499197 Compare July 10, 2024 22:51
@climbfuji
Copy link
Copy Markdown
Collaborator Author

Thanks @AlexanderRichert-NOAA. This is all looking pretty good. I'll finish the GitHub actions, test container builds and also building locally on an NRL system, push the documentation updates and open it up for reviews. Would be great to get this merged before I go on leave end of this week (for three weeks).

@climbfuji climbfuji force-pushed the feature/set_env_compiler branch from dab56e7 to 44ad542 Compare July 11, 2024 02:06
@climbfuji climbfuji marked this pull request as ready for review July 11, 2024 03:00
@climbfuji
Copy link
Copy Markdown
Collaborator Author

@AlexanderRichert-NOAA This is now ready for review (and additional testing if you feel inclined to do so)

@climbfuji climbfuji force-pushed the feature/set_env_compiler branch from 53e0e37 to 7353d7c Compare July 11, 2024 12:15
Copy link
Copy Markdown
Collaborator

@srherbener srherbener left a comment

Choose a reason for hiding this comment

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

I've tested on Discover and my Mac. Discover successfully made it through the spack-stack build. My Mac made it through concretize and failed with the musl issue during install. I think the Mac issue is limited to apple-clang%15 and not related to this PR, so I'm happy to approve. Thanks!

@climbfuji
Copy link
Copy Markdown
Collaborator Author

Thanks so much for testing @srherbener - @AlexanderRichert-NOAA any chance you can take a look at this PR in the next week or two?

@AlexanderRichert-NOAA
Copy link
Copy Markdown
Collaborator

Sorry @climbfuji this got stuck in the hopper. I'm going to test it out momentarily...

@AlexanderRichert-NOAA
Copy link
Copy Markdown
Collaborator

Looks like py-torch specifically wants [email protected]_2020-12-22, but oneapi support wasn't added until 3.6. Maybe remove the ai piece for now?

@climbfuji
Copy link
Copy Markdown
Collaborator Author

@AlexanderRichert-NOAA CI tests now pass (I turned off the env chaining for the new oneapi workflow). Can you please review again?

@climbfuji climbfuji merged commit 1922057 into JCSDA:develop Jul 27, 2024
@climbfuji climbfuji deleted the feature/set_env_compiler branch July 27, 2024 06:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

No open projects

Development

Successfully merging this pull request may close these issues.

Only use one (principal) compiler per enviroment Error executing setup-meta-modules

3 participants