Refactored CI

[G-Ragghianti]

This is a refactoring of the current CI jobs such that the jobs no longer rely on building the dependencies using spack. The script instead load the dependencies from a pre-built spack environment. This should decrease the failure rate of the CI jobs due to dependency building issues.

[bosilca]

why an absolute path here ?

[G-Ragghianti]

This is where the spack env that is built for the parsec CI is located in the docker container. It is built ahead of time to avoid issues related to trying to build the dependencies on-demand. I don't know of a problem with using absolute paths. There isn't a way to refer to this location without doing so.

[bosilca]

Such a change will prevent the parsec CI from running on any other environment than this (or any parsec forks from having their own CI). Providing an environment variable to point to the location of the installed spack could be one way of solving this. The other one would be to allow parsec to install its own spack, based on a tag/ref.

[G-Ragghianti]

I can put an environment variable defined in the main.yml for the location of the spack environment, but wouldn't that just move the problem to a different location?

[G-Ragghianti]

I see that I can also use a variable that is defined in the github repo config (like the "repo secrets" but not secret). I think this would accomplish what you want, but I won't be able to get it to work until the variable is defined in the repo (since it isn't a part of the PR). How about we use $LOCAL_SPACK_INSTALL_DIR?

[G-Ragghianti]

I tried using a repository variable, but it doesn't work because the variables are not passed to workflows that are triggered from PR from a fork. Therefore, I think the only way is to define it in the workflow yaml.

[bosilca]

I take back what I said yesterday about the secrets, we are not using a secret but instead we use the github slack app to send messages to our slack channel. But, I'm sure I played around with the secrets to have a different SLACK interaction, and the secrets were available in the CI script.

As a test I added an environment variable to parsec and dplasma called SPACK_SETUP_ENV. Can you see if it is available in the CI ?

[G-Ragghianti]

I did this test previously. See the output from the "Setup" stage where I referenced this variable in the "setup.sh" script. I also referenced the variable in the main.yaml via the "vars" context that is recommended in the documentation. The variable is available to the CI only when running on a branch within the repo. It is not available when running on a PR that originated from an external fork (this like PR). You can see this in the most recently executed actions for the PR.

[therault]

LGTM, I'd like to see how we can enable CUDA, HIP, and LEVEL_ZERO/SYCL testing, but that can be part of another PR.

[G-Ragghianti]

After thinking more about the issue of hard-coding the path to the spack install, I've decided that we can require that the environment variable be defined by the compute environment itself (the computer that is actually running the CI scripts). Therefore, I would add an environment variable to the CI runner that the scripts would expect to have already defined. If it isn't defined, it will exit with an informative message. The use can then define the variable in order to run the CI scripts in other environments.

[abouteiller]

This should not be conditional on build type

[G-Ragghianti]

I believe that this is a direct port of the functionality of the old CI. Is it not?

[G-Ragghianti]

OK, I made the conditional also include the running of ctest. Is this what we want, or should we put the conditional in the yaml?

[bosilca]

Having the conditional in the yaml would be nicer.

[G-Ragghianti]

I discussed this with Aurelien before the last update. We thought that it made more sense to have all the logic in one place and that this would also cause the scripts to work the same whether or not they are being executed through the CI or in an interactive session.

[bosilca]

I find it strange to put such logic into helper files instead of having it in the file that was meant for that (aka. the yaml). But I might have a skewed view as all the other projects I'm involved with have the CI driving login in the yaml and the other scripts are just helpers.

[G-Ragghianti]

I think some people must find it nice to have everything in one file or in as few files as possible. I do not find that to be nice to maintain in this case for two major reasons. The first is that having one language or markup embedded in another unnecessarily complicates the process of understanding what the code is doing. You have to keep track of the context of what you are looking at line-by-line in order to know if it is bash script of yaml. You also have to be more careful of the escaping and referencing of variables because the yaml variables are referenced differently than in the shell script. The shell script within the yaml also has to be unnecessarily indented to satisfy the requirements of both the yaml markup and the style of the shell script (this indentation could be different). I find it much cleaner to have one markup or language in a given file. This way, someone looking at the much shorter yaml can see that it just iterates over the matric values and runs the shell scripts in sequence. The shell scripts are divided up in order to encapsulate the functionality. If you are interested in modifying the test or build or configuration process, then it is very clear where you would do that, and you don't have to worry nearly as much about affecting other parts of the CI because the context of the functionality is limited.

[bosilca]

Having the conditional in the yaml would be nicer.