move logic that is not specific to gromacs into hooks functions

[smoors]

fixes #18

[boegel]

@smoors README still mentions singlenode, should be changed too?

[casparvl]

I've put a lot of comments. I suggest that in this PR, we only change stuff that is immediately relevant for the GROMACS test (function names, arguments), or trivial fixes (double indentation of docstring).

The replacement of hardcoded strings by constants from eessi_utils.features should probably be something we do in a separate PR.

[casparvl]

We should add the meaning of the arguments, and probably name the argument required_device rather than device_type, as it is more descriptive. The current device_type argument means: the type of device that this test requires in order to run. For GROMACS, if nb.impl='gpu', it requires a GPU (of any type). Note that device_type should match with the features (i.e. the string in the reframe config that specifies a feature) that can be set on partitions, so it is probably better to only use predefined strings (something like GPU_DEV_NAME, but different - that one is reserved for known device names in a ReFrame config) as arguments.

E.g. if a test requires a generic GPU (i.e. any GPU), then, device_type should be eessi_utils.features.GPU_GENERIC or something like that. If a test requires any CUDA GPU, it could be eessi_utils.features.GPU_CUDA. (etc if we need more specific GPU models)

This means the test itself typically has to do some form of translation. I.e. suppose GROMACS would only have support for CUDA GPUs, one might need to do

# Test specific 'information', in this case an argument, that determines what device is needed
if nb.impl == 'gpu':
    # Specify the required device, as a string constant imported from utils
    required_device = eessi_utils.features.GPU_CUDA
else:
    required_device = eessi_utils.features.CPU  # eessi_utils.features.CPU = 'cpu'
filter_tests_by_device_type(self, required_device)

Then, a ReFrame config file could do

from eessi_utils import utils
...
   'features': [
       eessi_utils.features.GPU_CUDA
       eessi_utils.features.CPU
    ]

In this way, we can standardize which 'features' are 'known'.

[smoors]

i see your point, but required_device could be interpreted as the actual device.
let's settle on required_device_type?

for the cuda/generic features stuff, let's do that in a separate PR

[casparvl]

This should use eessi_utils.features.GPU_GENERIC, to make sure the same string is consistently used.

In the future, we'll probably have to make this more specific and test is_cuda_module if eessi_utils.features.GPU_CUDA (or more specific, i.e. eessi_utils.features.GPU_CUDA_A100) is requested. The 'or more specific' probably also means we need to define that hierarchy somewhere, when the time comes we can think about how to do that. But, let's worry about that later...

[smoors]

let's do that in a separate PR

[smoors]

issue created for replacing strings with constants #30

[boegel]

lgtm

[casparvl]

Lgtm now