[feat] Extend the syntax of valid_systems and valid_prog_environs

[vkarak]

This PR introduces a features attribute in both the environment and system partition configuration and extends the syntax of valid_systems and valid_prog_environs so that system and environment filtering happens based on features and properties rather than on system and environment names only.

Defining features and custom properties in partitions and environments

Currently, you can define custom properties in a system partition of environment through the extras property. A new property features is added in both that you can use for defining boolean features. Examples:

            'partitions': [
                {
                    'name': 'gpu',
                    ...
                    'features': ['cuda', 'mpi'],
                    'extras': {
                        'gpu_arch': 'a100'
                    },
                ...
            ]

    'environments': [
        {
            'name': 'PrgEnv-gnu',
            'modules': ['PrgEnv-gnu'],
            'cc': 'gcc',
            'cxx': 'g++',
            'ftn': 'gfortran',
            'features': ['cxx14'],
            'extras': {
                'foo': 1,
                'bar': 'x'
            },
        },
    ],

Selecting partitions/environment with features and/or extras

Both the valid_systems and valid_prog_environs entries now accept the following syntax:

• +feat: the partition or environment has feat. If this appears in valid_systems, the framework will also pick the test if the current partition defines a resource named feat.
• -feat: the partition or environment do not have feat.
• %key=val: the partition or environment define the property key with value val in their extras. val will be converted to the type of the corresponding key in the partition or environment extras. If such conversion is not possible, there is no match.

Features can be either ANDed or ORed in the following way:

• Multiple entries in valid_systems or valid_prog_environs are ORed. For example, valid_systems = ['+feat1', '+feat2'] is interpreted as "any partition that defines either feat1 or feat2 is valid."
• Multiple feature and/or key/value properties defined in the same entry are ANDed. For example, valid_systems = ['+feat1 +feat2 %x=1'] will select all partitions that define feat1 and feat2 and have the x=1 in their extras.

The current syntax remains valid, but you can't mix them in a single entry. For example, the following are invalid:

valid_prog_environs = ['foo +x']  # invalid
valid_systems = ['+y sys']  # invalid

Generally, the valid_systems and valid_prog_environs syntax is now validated more strictly when you set them, so if the syntax is not correct, ReFrame will not allow you to set them.

Implementation details

The way the valid system and environment combinations are populated is completely different now. In the past it was just an external product of valid_systems and valid_prog_environs values filtered by the list of supported environments at each partition. With the introduction of selectable features and properties, however, an environment might define a feature only on its instantiation for a specific partition (because it sets the target_systems). That means expanding the feature set to environment names and then taking the external product to generate the combinations as before is insufficient. For this reason, now the system/partition combinations are expanded together. This is taken care of by the new reframe.core.runtime.valid_sysenv_comb() function which returns a dictionary of partitions and their valid environments for the current system. This is used also by the fixtures in order to pick a valid partition/environment combination based on their scope. Since this function relies on the values of valid_systems and valid_prog_environs to generate the valid combinations, the --skip-system-check and --skip-prgenv-check mechanisms have changed fundamentally. These options now set the valid_systems and valid_prog_environs to ['*'] after the object is created and before it has its fixtures injected. The reason for that is that before injecting and registering the test's fixtures, the valid_systems and valid_prog_environs must have their final value, in order to generate the right combinations.

Future extensions for discussion

These are a couple of extensions that we consider for the future.

Support relational operators in key/value comparisons

This is about supporting entries such as %x>10, which would mean select all systems/environment that define x greater than 10.

The complication with this feature is that we would have to do implicit type conversions from the values in valid_systems etc. as well as those defined in extras.

Allow selecting tests on partition processor properties

This would allow us to have valid_systems = ['%proc.arch=broadwell'] without having to define the same property in extras. This could feature be combined nicely with the above one and have entries such as valid_systems = ['%proc.num_cores>=4'].

Utilities to extend valid systems/environment entries

It might be common to add more constraints to the valid systems/environments specification of a (library) test that you inherit. Adding an ORed constraint is pretty straightforward, since you simply have to extend valid_systems or valid_prog_environs with +=, but if you want to AND a new constraint you will have to add it to the various entries or just to some of them. So a shortcut to do that could be convenient. An example is the following:

valid_systems.add_constraint(r'.*gpu.*', '+v100')

This would mean add +v100 to all the valid_systems entries that have gpu in them. The above syntax implies that valid_systems becomes a list-like object, that behaves exactly like a list but with some additional functionality. Alternatively we could have the add_constraint as a free function:

valid_systems = add_constraint(valid_systems, r'*.gpu.*', '+v100')

Todos

• Update documentation
• Support implicit type conversion in property values comparisons

Fixes #1987.
Fixes #2312.

[codecov-commenter]

Codecov Report

Merging #2479 (e95c723) into master (83fa082) will increase coverage by 0.02%.
The diff coverage is 96.73%.

@@            Coverage Diff             @@
##           master    #2479      +/-   ##
==========================================
+ Coverage   85.76%   85.79%   +0.02%     
==========================================
  Files          57       57              
  Lines       10603    10686      +83     
==========================================
+ Hits         9094     9168      +74     
- Misses       1509     1518       +9     

Impacted Files	Coverage Δ
reframe/frontend/loader.py	91.25% <61.53%> (-2.71%)	⬇️
reframe/core/decorators.py	55.82% <100.00%> (+0.27%)	⬆️
reframe/core/environments.py	97.29% <100.00%> (+0.10%)	⬆️
reframe/core/fixtures.py	97.35% <100.00%> (-2.65%)	⬇️
reframe/core/meta.py	99.07% <100.00%> (+0.01%)	⬆️
reframe/core/pipeline.py	93.48% <100.00%> (-0.01%)	⬇️
reframe/core/runtime.py	92.96% <100.00%> (+3.08%)	⬆️
reframe/core/systems.py	89.83% <100.00%> (+1.46%)	⬆️
reframe/frontend/cli.py	71.04% <100.00%> (-0.05%)	⬇️
reframe/frontend/executors/__init__.py	97.48% <100.00%> (-0.06%)	⬇️
... and 2 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 83fa082...e95c723. Read the comment docs.

[ekouts]

There is an issue with the fixtures. When the scope='session' this simple test fails:

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


class Fix(rfm.RunOnlyRegressionTest):
    executable = 'hostname'

    @sanity_function
    def san(self):
        return True


@rfm.simple_test
class TestA(rfm.RunOnlyRegressionTest):
    # f = fixture(Fix, scope='test')
    f = fixture(Fix, scope='session')

    executable = 'hostname'
    valid_systems = ['*']
    valid_prog_environs = ['+feat']

    @sanity_function
    def san(self):
        return True

with this error:

  * Reason: index error: list index out of range
Traceback (most recent call last):
  File "/Users/ekoutsaniti/repos/reframe-fork/reframe/frontend/executors/__init__.py", line 293, in _safe_call
    return fn(*args, **kwargs)
  File "/Users/ekoutsaniti/repos/reframe-fork/reframe/core/hooks.py", line 80, in _fn
    func(obj, *args, **kwargs)
  File "/Users/ekoutsaniti/repos/reframe-fork/reframe/core/pipeline.py", line 2556, in setup
    self._resolve_fixtures()
  File "/Users/ekoutsaniti/repos/reframe-fork/reframe/core/pipeline.py", line 1602, in _resolve_fixtures
    deps = deps[0]
IndexError: list index out of range

When scope='test' it runs normally. If I set the partition to '+cpu' it fails for both cases.

Another thing is that I don't like very much that features and extras are simple strings, so empty spaces or "special characters" like + etc are allowed in the configuration, but we cannot somehow select systems/envs with this syntax. If the feature is called +plus, we cannot set the valid systems with ++plus. I think we should either restrict the configuration (if possible) or extend the syntax of the valid systems/environs. Or at least document that this case will not work. What do you think?

[pep8speaks]

Hello @vkarak, 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 2022-04-08 10:57:14 UTC

[vkarak]

@jenkins-cscs retry dom

[ekouts]

Overall it looks good to me. We can restrict further the configuration schema, but it's also fine as it is.

[vkarak]

Overall it looks good to me. We can restrict further the configuration schema, but it's also fine as it is.

I'm working on that an I have an updated schema. I only need to check if it is possible to impose a pattern to the properties of a JSON object. I will push today.