Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add mixin class to replace mandatory hooks #177

Merged
merged 53 commits into from
Oct 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
53 commits
Select commit Hold shift + click to select a range
71e2ea5
Merge branch 'main' of github.com:EESSI/test-suite
Aug 19, 2024
fbfe08d
Merge branch 'main' of github.com:EESSI/test-suite
Sep 4, 2024
3ae63d8
First attempt at using a mixin class
Sep 4, 2024
56d457b
Expand the number of hooks in the eessi_mixin class. Check that varia…
Sep 4, 2024
24a0256
Comment to make clear how class inheritance works on pipeline hooks
Sep 4, 2024
2bdd84f
Set compute unit in init phase - we already have all info we need there
Sep 4, 2024
3ad1662
Expand comment
Sep 4, 2024
119104e
Add required_mem_per_node comes from self
Sep 4, 2024
67d3e4e
Make sure the required_mem_per_node function returns the value
Sep 4, 2024
f096cb9
More clear SyntaxError message
Sep 4, 2024
bee51d2
Added some more suggestions for the future
Sep 4, 2024
3ffe7eb
Clarify printed message
Sep 19, 2024
c4a0bcc
Add memory hook. Make it useable by setting measure_memory_usage
Sep 19, 2024
2f84687
Leave a breadcrumb for our future selves
Sep 19, 2024
582b7b6
Do dynamic version generation, both at build time and runtime. Use th…
Sep 19, 2024
97e60ab
First try to get version from installed package. If packate is not in…
Sep 19, 2024
bf48c2f
Make sure scm writes a version file. If the version file doesn't exis…
Sep 20, 2024
5095c4f
Of course, the extension shouldn't be part of the import...
Sep 20, 2024
3c2f0c6
Import was failing, try something else
Sep 20, 2024
04ca0ca
Now correctly import __version__ in the global namespace of eessi/tes…
Sep 20, 2024
28376b7
Fix some flake8 errors
Sep 20, 2024
439c425
Fix some more flake8 issues
Sep 20, 2024
72d9a39
Fix more flake8 issues
Sep 20, 2024
7909b2d
Fix more flake8 stuff
Sep 20, 2024
204a819
Remove whitespace on blank line
Sep 20, 2024
a396f6a
Enforce new enough setuptools_scm so that it knows the version_file a…
Sep 20, 2024
c821106
Make usage of write_to or version_file dependent on the version of py…
Sep 20, 2024
536a444
Let setup.py handle the choice of argument for writing the version file
Sep 20, 2024
84178cd
Make things conditional on version of setuptools_scm directly
Sep 20, 2024
d90e831
No longer enforce version v8 or higher for setuptools_scm, it doesn't…
Sep 20, 2024
95a6d6b
Remove stray line
Sep 20, 2024
a79b263
Make flake8 happy again
Sep 20, 2024
3f0d174
Just stick to write_to for everything, and keep the setuptools_scm ve…
Sep 20, 2024
430dc10
Remove import that isn't used
Sep 20, 2024
8bbb582
Fix CI issue for python 3.6
Sep 20, 2024
25f0167
Fix version of setuptools_scm below 8
Sep 20, 2024
618df8e
Stick to setuptools_scm < 7 to be compatible with python 3.6
Sep 20, 2024
e0295ea
Make it < 7 consistently...
Sep 20, 2024
070a549
Also limit version of packaging used for compatibility with python 3.6
Sep 20, 2024
5108a85
Make things consistent..
Sep 20, 2024
c8bf4a6
let setup.py handle the setuptools_scm conditionally, see if that wor…
Sep 20, 2024
d531e41
remove stray line
Sep 20, 2024
645bb18
Fix flake8
Sep 20, 2024
ef985d1
Make sure packaging version isn't too new for Python 3.6
Sep 20, 2024
a6ea23c
Pass list of strings
Sep 20, 2024
1f1ee2d
Now really fix it
Sep 20, 2024
446e4c8
Merge branch 'use_mixin_class' of github.com:casparvl/test-suite into…
Sep 21, 2024
38ffee1
Split off the setuptools_scm versioning into a different pr https://g…
Sep 21, 2024
94d16e7
update mixin class
Oct 6, 2024
b69d1ee
do not override time_limit
Oct 7, 2024
2f6b33a
Merge pull request #2 from smoors/pr177
casparvl Oct 8, 2024
7e66d57
Merge branch 'use_mixin_class' of github.com:casparvl/test-suite into…
Oct 9, 2024
24951c8
Some cleanup
Oct 9, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
153 changes: 153 additions & 0 deletions eessi/testsuite/eessi_mixin.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,153 @@
from reframe.core.builtins import parameter, run_after
from reframe.core.exceptions import ReframeFatalError
from reframe.core.pipeline import RegressionMixin
from reframe.utility.sanity import make_performance_function

from eessi.testsuite import hooks
from eessi.testsuite.constants import DEVICE_TYPES, SCALES, COMPUTE_UNIT


# Hooks from the Mixin class seem to be executed _before_ those of the child class
# Thus, if the Mixin class needs self.X to be defined in after setup, the child class would have to define it before
# setup. That's a disadvantage and might not always be possible - let's see how far we get. It also seems that,
# like normal inheritance, functions with the same in the child and parent class will mean the child class
# will overwrite that of the parent class. That is a plus, as we can use the EESSI_Mixin class as a basis,
# but still overwrite specific functions in case specific tests would require this
# TODO: for this reason, we probably want to put _each_ hooks.something invocation into a seperate function,
# so that each individual one can be overwritten
#
# Note that I don't think we can do anything about the things set in the class body, such as the parameter's.
# Maybe we can move those to an __init__ step of the Mixin, even though that is not typically how ReFrame
# does it anymore?
# That way, the child class could define it as class variables, and the parent can use it in its __init__ method?
class EESSI_Mixin(RegressionMixin):
"""
All EESSI tests should derive from this mixin class unless they have a very good reason not to.
To run correctly, tests inheriting from this class need to define variables and parameters that are used here.
That definition needs to be done 'on time', i.e. early enough in the execution of the ReFrame pipeline.
Here, we list which class attributes need to be defined by the child class, and by (the end of) what phase:

- Init phase: device_type, scale, module_name
- Setup phase: compute_unit, required_mem_per_node

The child class may also overwrite the following attributes:

- Init phase: time_limit, measure_memory_usage
"""

smoors marked this conversation as resolved.
Show resolved Hide resolved
# Set defaults for these class variables, can be overwritten by child class if desired
measure_memory_usage = False
scale = parameter(SCALES.keys())

# Note that the error for an empty parameter is a bit unclear for ReFrame 4.6.2, but that will hopefully improve
# see https://github.com/reframe-hpc/reframe/issues/3254
# If that improves: uncomment the following to force the user to set module_name
# module_name = parameter()

def __init_subclass__(cls, **kwargs):
" set default values for built-in ReFrame attributes "
super().__init_subclass__(**kwargs)
cls.valid_prog_environs = ['default']
cls.valid_systems = ['*']
if not cls.time_limit:
cls.time_limit = '1h'

# Helper function to validate if an attribute is present it item_dict.
# If not, print it's current name, value, and the valid_values
def validate_item_in_dict(self, item, item_dict, check_keys=False):
"""
Check if the item 'item' exist in the values of 'item_dict'.
If check_keys=True, then it will check instead of 'item' exists in the keys of 'item_dict'.
If item is not found, an error will be raised that will mention the valid values for 'item'.
"""
if check_keys:
valid_items = list(item_dict.keys())
else:
valid_items = list(item_dict.values())

value = getattr(self, item)
if value not in valid_items:
if len(valid_items) == 1:
msg = f"The variable '{item}' has value {value}, but the only valid value is {valid_items[0]}"
else:
msg = f"The variable '{item}' has value {value}, but the only valid values are {valid_items}"
raise ReframeFatalError(msg)

@run_after('init')
def validate_init(self):
"""Check that all variables that have to be set for subsequent hooks in the init phase have been set"""
# List which variables we will need/use in the run_after('init') hooks
var_list = ['device_type', 'scale', 'module_name', 'measure_memory_usage']
for var in var_list:
if not hasattr(self, var):
msg = "The variable '%s' should be defined in any test class that inherits" % var
msg += " from EESSI_Mixin in the init phase (or earlier), but it wasn't"
raise ReframeFatalError(msg)

# Check that the value for these variables is valid,
# i.e. exists in their respective dict from eessi.testsuite.constants
self.validate_item_in_dict('device_type', DEVICE_TYPES)
self.validate_item_in_dict('scale', SCALES, check_keys=True)
self.validate_item_in_dict('valid_systems', {'valid_systems': ['*']})
self.validate_item_in_dict('valid_prog_environs', {'valid_prog_environs': ['default']})

@run_after('init')
def run_after_init(self):
"""Hooks to run after init phase"""

# Filter on which scales are supported by the partitions defined in the ReFrame configuration
hooks.filter_supported_scales(self)

hooks.filter_valid_systems_by_device_type(self, required_device_type=self.device_type)

hooks.set_modules(self)

# Set scales as tags
hooks.set_tag_scale(self)

@run_after('init')
def measure_mem_usage(self):
if self.measure_memory_usage:
hooks.measure_memory_usage(self)
# Since we want to do this conditionally on self.measure_mem_usage, we use make_performance_function
# instead of the @performance_function decorator
self.perf_variables['memory'] = make_performance_function(hooks.extract_memory_usage, 'MiB', self)

@run_after('setup')
def validate_setup(self):
"""Check that all variables that have to be set for subsequent hooks in the setup phase have been set"""
var_list = ['compute_unit']
for var in var_list:
if not hasattr(self, var):
msg = "The variable '%s' should be defined in any test class that inherits" % var
msg += " from EESSI_Mixin in the setup phase (or earlier), but it wasn't"
raise ReframeFatalError(msg)

# Check if mem_func was defined to compute the required memory per node as function of the number of
# tasks per node
if not hasattr(self, 'required_mem_per_node'):
msg = "The function 'required_mem_per_node' should be defined in any test class that inherits"
msg += " from EESSI_Mixin in the setup phase (or earlier), but it wasn't. Note that this function"
msg += " can use self.num_tasks_per_node, as it will be called after that attribute"
msg += " has been set."
raise ReframeFatalError(msg)

# Check that the value for these variables is valid
# i.e. exists in their respective dict from eessi.testsuite.constants
self.validate_item_in_dict('compute_unit', COMPUTE_UNIT)

@run_after('setup')
def assign_tasks_per_compute_unit(self):
"""Call hooks to assign tasks per compute unit, set OMP_NUM_THREADS, and set compact process binding"""
hooks.assign_tasks_per_compute_unit(test=self, compute_unit=self.compute_unit)

# Set OMP_NUM_THREADS environment variable
hooks.set_omp_num_threads(self)

# Set compact process binding
hooks.set_compact_process_binding(self)

@run_after('setup')
def request_mem(self):
"""Call hook to request the required amount of memory per node"""
hooks.req_memory_per_node(self, app_mem_req=self.required_mem_per_node())
43 changes: 10 additions & 33 deletions eessi/testsuite/tests/apps/lammps/lammps.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,18 +8,20 @@

from eessi.testsuite import hooks, utils
from eessi.testsuite.constants import * # noqa
from eessi.testsuite.eessi_mixin import EESSI_Mixin


class EESSI_LAMMPS_base(rfm.RunOnlyRegressionTest):
scale = parameter(SCALES.keys())
valid_prog_environs = ['default']
valid_systems = ['*']
class EESSI_LAMMPS_base(rfm.RunOnlyRegressionTest, EESSI_Mixin):
time_limit = '30m'
device_type = parameter([DEVICE_TYPES[CPU], DEVICE_TYPES[GPU]])

# Parameterize over all modules that start with LAMMPS
module_name = parameter(utils.find_modules('LAMMPS'))

def required_mem_per_node(self):
mem = {'slope': 0.07, 'intercept': 0.5}
return (self.num_tasks_per_node * mem['slope'] + mem['intercept']) * 1024

# Set sanity step
@deferrable
def assert_lammps_openmp_treads(self):
Expand Down Expand Up @@ -48,40 +50,15 @@ def assert_run(self):
return sn.assert_eq(n_atoms, 32000)

@run_after('init')
def run_after_init(self):
"""hooks to run after init phase"""

# Filter on which scales are supported by the partitions defined in the ReFrame configuration
hooks.filter_supported_scales(self)

hooks.filter_valid_systems_by_device_type(self, required_device_type=self.device_type)

hooks.set_modules(self)

# Set scales as tags
hooks.set_tag_scale(self)

@run_after('setup')
def run_after_setup(self):
"""hooks to run after the setup phase"""
if self.device_type == 'cpu':
hooks.assign_tasks_per_compute_unit(test=self, compute_unit=COMPUTE_UNIT['CPU'])
self.compute_unit = COMPUTE_UNIT['CPU']
elif self.device_type == 'gpu':
hooks.assign_tasks_per_compute_unit(test=self, compute_unit=COMPUTE_UNIT['GPU'])
self.compute_unit = COMPUTE_UNIT['GPU']
else:
raise NotImplementedError(f'Failed to set number of tasks and cpus per task for device {self.device_type}')

# Set OMP_NUM_THREADS environment variable
hooks.set_omp_num_threads(self)

# Set compact process binding
hooks.set_compact_process_binding(self)

@run_after('setup')
def request_mem(self):
mem = {'slope': 0.07, 'intercept': 0.5}
mem_required = self.num_tasks_per_node * mem['slope'] + mem['intercept']
hooks.req_memory_per_node(self, app_mem_req=mem_required * 1024)
msg = f"No mapping of device type {self.device_type} to a COMPUTE_UNIT was specified in this test"
raise NotImplementedError(msg)


@rfm.simple_test
Expand Down
Loading