[feat] Automatically create a Spack environment when not set

[giordano]

Fix #2028.

[codecov-commenter]

Codecov Report

Merging #2031 (0034464) into master (442e41c) will increase coverage by 0.52%.
The diff coverage is 100.00%.

❗ Current head 0034464 differs from pull request most recent head 3457c2e. Consider uploading reports for the commit 3457c2e to get more accurate results

@@            Coverage Diff             @@
##           master    #2031      +/-   ##
==========================================
+ Coverage   86.23%   86.76%   +0.52%     
==========================================
  Files          53       52       -1     
  Lines        9305     9276      -29     
==========================================
+ Hits         8024     8048      +24     
+ Misses       1281     1228      -53     

Impacted Files	Coverage Δ
reframe/core/buildsystems.py	96.42% <100.00%> (+0.32%)	⬆️
reframe/core/exceptions.py	94.63% <0.00%> (-2.65%)	⬇️
reframe/core/pipeline.py	91.40% <0.00%> (-0.34%)	⬇️
reframe/core/namespaces.py	95.00% <0.00%> (-0.09%)	⬇️
reframe/core/variables.py	96.09% <0.00%> (-0.02%)	⬇️
reframe/core/backends.py	90.00% <0.00%> (ø)
reframe/core/warnings.py	100.00% <0.00%> (ø)
reframe/core/parameters.py	100.00% <0.00%> (ø)
reframe/core/schedulers/sge.py
reframe/frontend/cli.py	76.00% <0.00%> (+0.04%)	⬆️
... and 9 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 442e41c...3457c2e. Read the comment docs.

[jenkins-cscs]

Can I test this patch?

[vkarak]

I tried this PR with the following test and it works:

import reframe as rfm
import reframe.utility.sanity as sn


@rfm.simple_test
class reframe_spack_env_check(rfm.RegressionTest):
    valid_systems = ['*']
    valid_prog_environs = ['*']
    build_system = 'Spack'
    executable = 'bzip2'
    executable_opts = ['--help']

    @run_before('sanity')
    def set_sanity(self):
        self.sanity_patterns = sn.assert_found(r'Version 1.0.7', self.stderr)

    @run_before('compile')
    def setup_build_system(self):
        self.build_system.specs = ['bzip2@1.0.7']

However, when I run the test and then do spack find bzip2, I see 1.0.7 installed in my system. Is this expected? I'm not that familiar with Spack, so I might miss something. @victorusu can you also have a look?

[vkarak]

So the problem with the automatically generated environment is that it doesn't set correctly the installation paths. Ideally, we want this to be installed in the stage directory of the test and not "pollute" the user's installation, unless explicitly required.

[giordano]

How is this different from an environment manually created from the user? I guess I'm missing something, but then we're back to the fact documentation isn't helpful at the moment and I'm trying to reverse engineer what I'm supposed to do 🙂

[victorusu]

@giordano, I think the issue is that we haven't documented enough the current goals of this feature.

AFAIK, what we wanted in the beginning is to test Spack packages and have them installed in the test's stagedir. Then, we would see how it works in the wild and evolve it to achieve other things.
Therefore, we left the environments out-of-the-box to have this discussion later and told people to enable the environment before starting reframe. In this case, the user can set the environment as they want and still leverage the feature from reframe.

One of the reasons for this decision was that we did not know, in principle, which options of the Spack.yaml file we would need to add to the build system.
So we were waiting for two things:

1. To have feedback from he Community
2. As you know Spack evolves very fast and so we would like to wait a bit until environments also become more and more stable in Spack.

So, we postponed environment's inclusion/discussion.

Now, as you have pointed out, the time to have this discussion has come!😄

@vkarak and @giordano, what do you think about adding a build system option to set the install_dir?
In this case, it would create on the stage dir a config file from Spack that would set the install_dir and we could make it point to the stagedir, by default?

[giordano]

To be clear, what you're suggesting is to automatically run something like

spack config add "config:install_tree:${install_dir}"

after activating the environment, where ${install_dir} is the chosen directory, something like ${stagedir}/prefix by default? That'd be fine with me as long as there is a reasonable default which doesn't require the user to do anything extra: as a user and someone who will need to tell other people how to use this, I wouldn't be particularly happy about having to manually create the environment with the right configuration, maybe outside of reframe.

I'd love to have something as simple as this

import reframe as rfm
import reframe.utility.sanity as sn


@rfm.simple_test
class Test(rfm.RegressionTest):
    valid_systems = ['*']
    valid_prog_environs = ['*']
    build_system = 'Spack'
    executable = 'myprogram'

    @run_before('compile')
    def set_compiler_flags(self):
        self.build_system.specs = ['program@1.0']

    @run_before('sanity')
    def set_sanity_patterns(self):
        self.sanity_patterns = sn.assert_found(r'SUCCESS', self.stdout)

to work without anything extra needed. As far as I understand, at the moment this isn't possible, and that's why I'm trying to hack the Spack build system with this pull request 🙂

[victorusu]

To be clear, what you're suggesting is to automatically run something like

spack config add "config:install_tree:${install_dir}"

after activating the environment, where ${install_dir} is the chosen directory, something like ${stagedir}/prefix by default? That'd be fine with me as long as there is a reasonable default which doesn't require the user to do anything extra: as a user and someone who will need to tell other people how to use this, I wouldn't be particularly happy about having to manually create the environment with the right configuration, maybe outside of reframe.

I'd love to have something as simple as this

import reframe as rfm
import reframe.utility.sanity as sn


@rfm.simple_test
class Test(rfm.RegressionTest):
    valid_systems = ['*']
    valid_prog_environs = ['*']
    build_system = 'Spack'
    executable = 'myprogram'

    @run_before('compile')
    def set_compiler_flags(self):
        self.build_system.specs = ['program@1.0']

    @run_before('sanity')
    def set_sanity_patterns(self):
        self.sanity_patterns = sn.assert_found(r'SUCCESS', self.stdout)

to work without anything extra needed. As far as I understand, at the moment this isn't possible, and that's why I'm trying to hack the Spack build system with this pull request 🙂

@giordano, that's exactly what I am saying. @vkarak what do you think?

[vkarak]

@giordano Yes, the rationale from @victorusu is correct about the initial purpose of the feature. We have been seeing it also as feature that would help in CI or to do parameter exploration on different builds. We never thought about it seriously as a means of deployment, because there can be too many parameters to consider there. Also I totally agree with what you'd like to see from a user's perspective. My initial objection was that, as a user, I would not like to see my global environment to be implicitly modified after running a reframe test. So by default the install_dir of the environment you create should point to the stage dir.

@victorusu For this PR, I would say that emitting the spack config add ... should be enough. If users want to customize the environment, they can simply create one by themselver for the moment. As far as I can see if spack config is done inside the environment then it's scoped in that. So that should be fine, imho.

[giordano]

Ok, I added an install_tree (used same name as Spack) to the build system. This can be either a non-empty string or None:

• if environment and install_tree are both None, then install_tree is automatically set to install_tree and this is used to set Spack's install tree
• if environment is not None but install_tree is, nothing happens (I feel like if the user provides their own environment, they're going to configure it themselves)

[giordano]

Ok, I finally got back to this pull request, rebased on latest master, and addressed all comments. Main relevant change now is that the default value of environment and install_tree is an empty string (which is a bit like going the opposite direction of #2095) 🙂

[vkarak]

Lgtm. I just have some minor suggestions. I could also have a look at the tutorial to see if it needs to be adjusted.

[pep8speaks]

Hello @giordano, Thank you for updating!

Cheers! There are no PEP8 issues in this Pull Request!Do see the ReFrame Coding Style Guide

Comment last updated at 2021-08-09 21:08:14 UTC

[giordano]

I mentioned in the tutorial that the environment isn't required and it'll be automatically created by ReFrame if not provided. I didn't mention install_tree there, do you think it's necessary?

[vkarak]

I mentioned in the tutorial that the environment isn't required and it'll be automatically created by ReFrame if not provided. I didn't mention install_tree there, do you think it's necessary?

No, it's not necessary. It's enough the we mention it in the reference.

Lgtm now! Going in.

[vkarak]

ok to test

[giordano]

Thanks!