From 3ae63d86cde62089f905424823f9f3a23167f2d5 Mon Sep 17 00:00:00 2001 From: Caspar van Leeuwen Date: Wed, 4 Sep 2024 21:23:54 +0200 Subject: [PATCH 01/48] First attempt at using a mixin class --- eessi/testsuite/eessi_mixin.py | 35 +++++++++++++++++++++ eessi/testsuite/tests/apps/lammps/lammps.py | 30 +++++++++--------- 2 files changed, 50 insertions(+), 15 deletions(-) create mode 100644 eessi/testsuite/eessi_mixin.py diff --git a/eessi/testsuite/eessi_mixin.py b/eessi/testsuite/eessi_mixin.py new file mode 100644 index 00000000..259fcc65 --- /dev/null +++ b/eessi/testsuite/eessi_mixin.py @@ -0,0 +1,35 @@ +from reframe.core.pipeline import RegressionMixin +from reframe.core.exceptions import ReframeSyntaxError + +from eessi.testsuite import hooks + +class EESSI_Mixin(RegressionMixin): + """ + All EESSI tests should derive from this mixin class unless they have a very good reason not to. + """ + + # We have to make sure that these gets set in any test that inherits + # device_type = variable(str) + # scale = variable(str) + # module_name = variable(str) + + @run_after('init') + def validate(self): + """Check that all variables that have to be set for subsequent hooks in the init phase have been set""" + if not hasattr(self, 'device_type'): + raise ReframeSyntaxError("device_type should be defined in any class that inherits from EESSI_Mixin, but wasn't") + + + @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) diff --git a/eessi/testsuite/tests/apps/lammps/lammps.py b/eessi/testsuite/tests/apps/lammps/lammps.py index b071d787..b92e03b8 100644 --- a/eessi/testsuite/tests/apps/lammps/lammps.py +++ b/eessi/testsuite/tests/apps/lammps/lammps.py @@ -8,9 +8,9 @@ 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): +class EESSI_LAMMPS_base(rfm.RunOnlyRegressionTest, EESSI_Mixin): scale = parameter(SCALES.keys()) valid_prog_environs = ['default'] valid_systems = ['*'] @@ -47,19 +47,19 @@ 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('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): From 56d457b4a0ca009c59d618b00c00dc30a0bc2cff Mon Sep 17 00:00:00 2001 From: Caspar van Leeuwen Date: Wed, 4 Sep 2024 22:56:23 +0200 Subject: [PATCH 02/48] Expand the number of hooks in the eessi_mixin class. Check that variables that are needed for those hooks are defined in time, and if not, print a clear message --- eessi/testsuite/eessi_mixin.py | 81 +++++++++++++++++++-- eessi/testsuite/tests/apps/lammps/lammps.py | 26 +++++-- 2 files changed, 94 insertions(+), 13 deletions(-) diff --git a/eessi/testsuite/eessi_mixin.py b/eessi/testsuite/eessi_mixin.py index 259fcc65..faea601a 100644 --- a/eessi/testsuite/eessi_mixin.py +++ b/eessi/testsuite/eessi_mixin.py @@ -2,23 +2,56 @@ from reframe.core.exceptions import ReframeSyntaxError from eessi.testsuite import hooks +from eessi.testsuite.constants import DEVICE_TYPES, CPU, GPU, SCALES, COMPUTE_UNIT + 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, and by (the end of) what phase: + + - Init phase: device_type, scale, module_name + - Setup phase: compute_unit, required_mem_per_node """ - + + # 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: + valid_items_str = (', '.join("'" + item + "'" for item in valid_items)) + raise ReframeSyntaxError("The variable '%s' had value '%s', but the only valid values are %s" % (item, value, valid_items_str)) + # We have to make sure that these gets set in any test that inherits # device_type = variable(str) # scale = variable(str) # module_name = variable(str) @run_after('init') - def validate(self): + def validate_init(self): """Check that all variables that have to be set for subsequent hooks in the init phase have been set""" - if not hasattr(self, 'device_type'): - raise ReframeSyntaxError("device_type should be defined in any class that inherits from EESSI_Mixin, but wasn't") - + # List which variables we will need/use in the run_after('init') hooks + var_list = ['device_type', 'scale', 'module_name'] + for var in var_list: + if not hasattr(self, var): + raise ReframeSyntaxError("The variable '%s' should be defined in any test class that inherits from EESSI_Mixin in the init phase (or earlier), but it wasn't" % var) + + # Check that the value for these variables is valid, i.e. exists in their respetive dict from eessi.testsuite.constants + self.validate_item_in_dict('device_type', DEVICE_TYPES) + self.validate_item_in_dict('scale', SCALES, check_keys=True) + @run_after('init') def run_after_init(self): @@ -33,3 +66,41 @@ def run_after_init(self): # Set scales as tags hooks.set_tag_scale(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): + raise ReframeSyntaxError("The variable '%s' should be defined in any test class that inherits from EESSI_Mixin in the setup phase (or earlier), but it wasn't" % var) + + # 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 from EESSI_Mixin in the setup phase (or earlier), " + msg += "but it wasn't. It should be a function that takes the number of tasks per node as argument, and return the memory per node required" + raise ReframeSyntaxError(msg) + + # Check that the value for these variables is valid, i.e. exists in their respetive dict from eessi.testsuite.constants + self.validate_item_in_dict('compute_unit', COMPUTE_UNIT) + + @run_after('setup') + def assign_tasks_per_compute_unit(self): + """hooks to run after the setup phase""" + hooks.assign_tasks_per_compute_unit(test=self, compute_unit=self.compute_unit) +# if self.device_type == 'cpu': +# hooks.assign_tasks_per_compute_unit(test=self, compute_unit=COMPUTE_UNIT['CPU']) +# elif self.device_type == 'gpu': +# hooks.assign_tasks_per_compute_unit(test=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): + hooks.req_memory_per_node(self, app_mem_req=required_mem_per_node(self.num_tasks_per_node)) diff --git a/eessi/testsuite/tests/apps/lammps/lammps.py b/eessi/testsuite/tests/apps/lammps/lammps.py index b92e03b8..509f8e12 100644 --- a/eessi/testsuite/tests/apps/lammps/lammps.py +++ b/eessi/testsuite/tests/apps/lammps/lammps.py @@ -11,10 +11,14 @@ from eessi.testsuite.eessi_mixin import EESSI_Mixin class EESSI_LAMMPS_base(rfm.RunOnlyRegressionTest, EESSI_Mixin): + # scales = list(SCALES.keys()) + # scales.append('blabla') + # scale = parameter(scales) scale = parameter(SCALES.keys()) valid_prog_environs = ['default'] valid_systems = ['*'] time_limit = '30m' + #device_type = parameter([DEVICE_TYPES[CPU], DEVICE_TYPES[GPU], 'bla']) device_type = parameter([DEVICE_TYPES[CPU], DEVICE_TYPES[GPU]]) # Parameterize over all modules that start with LAMMPS @@ -61,27 +65,33 @@ def assert_run(self): # # Set scales as tags # hooks.set_tag_scale(self) - @run_after('setup') + @run_before('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'] + # hooks.assign_tasks_per_compute_unit(test=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'] + # hooks.assign_tasks_per_compute_unit(test=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) + # hooks.set_omp_num_threads(self) # Set compact process binding - hooks.set_compact_process_binding(self) + # hooks.set_compact_process_binding(self) - @run_after('setup') + def required_mem_per_node(num_tasks_per_node): + mem = {'slope': 0.07, 'intercept': 0.5} + self.mem_required = (num_tasks_per_node * mem['slope'] + mem['intercept']) * 1024 + + @run_before('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) + # self.mem_required = (self.num_tasks_per_node * mem['slope'] + mem['intercept']) * 1024 + # hooks.req_memory_per_node(self, app_mem_req=mem_required * 1024) @rfm.simple_test From 24a0256632ca6890a5a9a74a25eaf568dfa91834 Mon Sep 17 00:00:00 2001 From: Caspar van Leeuwen Date: Wed, 4 Sep 2024 23:02:25 +0200 Subject: [PATCH 03/48] Comment to make clear how class inheritance works on pipeline hooks --- eessi/testsuite/eessi_mixin.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/eessi/testsuite/eessi_mixin.py b/eessi/testsuite/eessi_mixin.py index faea601a..aa743c8f 100644 --- a/eessi/testsuite/eessi_mixin.py +++ b/eessi/testsuite/eessi_mixin.py @@ -4,7 +4,12 @@ from eessi.testsuite import hooks from eessi.testsuite.constants import DEVICE_TYPES, CPU, GPU, 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 class EESSI_Mixin(RegressionMixin): """ All EESSI tests should derive from this mixin class unless they have a very good reason not to. From 2bdd84f5032d55e00758ef75937a8be112639957 Mon Sep 17 00:00:00 2001 From: Caspar van Leeuwen Date: Wed, 4 Sep 2024 23:08:28 +0200 Subject: [PATCH 04/48] Set compute unit in init phase - we already have all info we need there --- eessi/testsuite/tests/apps/lammps/lammps.py | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/eessi/testsuite/tests/apps/lammps/lammps.py b/eessi/testsuite/tests/apps/lammps/lammps.py index 509f8e12..f53a7715 100644 --- a/eessi/testsuite/tests/apps/lammps/lammps.py +++ b/eessi/testsuite/tests/apps/lammps/lammps.py @@ -24,6 +24,10 @@ class EESSI_LAMMPS_base(rfm.RunOnlyRegressionTest, EESSI_Mixin): # Parameterize over all modules that start with LAMMPS module_name = parameter(utils.find_modules('LAMMPS')) + def required_mem_per_node(num_tasks_per_node): + mem = {'slope': 0.07, 'intercept': 0.5} + self.mem_required = (num_tasks_per_node * mem['slope'] + mem['intercept']) * 1024 + # Set sanity step @deferrable def assert_lammps_openmp_treads(self): @@ -65,7 +69,7 @@ def assert_run(self): # # Set scales as tags # hooks.set_tag_scale(self) - @run_before('setup') + @run_after('init') def run_after_setup(self): """hooks to run after the setup phase""" if self.device_type == 'cpu': @@ -83,13 +87,10 @@ def run_after_setup(self): # Set compact process binding # hooks.set_compact_process_binding(self) - def required_mem_per_node(num_tasks_per_node): - mem = {'slope': 0.07, 'intercept': 0.5} - self.mem_required = (num_tasks_per_node * mem['slope'] + mem['intercept']) * 1024 - @run_before('setup') - def request_mem(self): - mem = {'slope': 0.07, 'intercept': 0.5} + #@run_after('init') + #def request_mem(self): + #mem = {'slope': 0.07, 'intercept': 0.5} # self.mem_required = (self.num_tasks_per_node * mem['slope'] + mem['intercept']) * 1024 # hooks.req_memory_per_node(self, app_mem_req=mem_required * 1024) From 3ad1662725be8d75bf374ff463d047fb07b21cbe Mon Sep 17 00:00:00 2001 From: Caspar van Leeuwen Date: Wed, 4 Sep 2024 23:09:44 +0200 Subject: [PATCH 05/48] Expand comment --- eessi/testsuite/eessi_mixin.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/eessi/testsuite/eessi_mixin.py b/eessi/testsuite/eessi_mixin.py index aa743c8f..8df8e227 100644 --- a/eessi/testsuite/eessi_mixin.py +++ b/eessi/testsuite/eessi_mixin.py @@ -10,6 +10,8 @@ # 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 class EESSI_Mixin(RegressionMixin): """ All EESSI tests should derive from this mixin class unless they have a very good reason not to. From 119104e8f4cbfb55131cf0e840dcbe1f52489617 Mon Sep 17 00:00:00 2001 From: Caspar van Leeuwen Date: Wed, 4 Sep 2024 23:11:43 +0200 Subject: [PATCH 06/48] Add required_mem_per_node comes from self --- eessi/testsuite/eessi_mixin.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/eessi/testsuite/eessi_mixin.py b/eessi/testsuite/eessi_mixin.py index 8df8e227..882a081c 100644 --- a/eessi/testsuite/eessi_mixin.py +++ b/eessi/testsuite/eessi_mixin.py @@ -110,4 +110,4 @@ def assign_tasks_per_compute_unit(self): @run_after('setup') def request_mem(self): - hooks.req_memory_per_node(self, app_mem_req=required_mem_per_node(self.num_tasks_per_node)) + hooks.req_memory_per_node(self, app_mem_req=self.required_mem_per_node(self.num_tasks_per_node)) From 67d3e4e40ffab5da3b6e306e430ddb3231950520 Mon Sep 17 00:00:00 2001 From: Caspar van Leeuwen Date: Wed, 4 Sep 2024 23:13:59 +0200 Subject: [PATCH 07/48] Make sure the required_mem_per_node function returns the value --- eessi/testsuite/eessi_mixin.py | 2 +- eessi/testsuite/tests/apps/lammps/lammps.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/eessi/testsuite/eessi_mixin.py b/eessi/testsuite/eessi_mixin.py index 882a081c..e15ea4d2 100644 --- a/eessi/testsuite/eessi_mixin.py +++ b/eessi/testsuite/eessi_mixin.py @@ -110,4 +110,4 @@ def assign_tasks_per_compute_unit(self): @run_after('setup') def request_mem(self): - hooks.req_memory_per_node(self, app_mem_req=self.required_mem_per_node(self.num_tasks_per_node)) + hooks.req_memory_per_node(self, app_mem_req=self.required_mem_per_node()) diff --git a/eessi/testsuite/tests/apps/lammps/lammps.py b/eessi/testsuite/tests/apps/lammps/lammps.py index f53a7715..5e44debe 100644 --- a/eessi/testsuite/tests/apps/lammps/lammps.py +++ b/eessi/testsuite/tests/apps/lammps/lammps.py @@ -24,9 +24,9 @@ class EESSI_LAMMPS_base(rfm.RunOnlyRegressionTest, EESSI_Mixin): # Parameterize over all modules that start with LAMMPS module_name = parameter(utils.find_modules('LAMMPS')) - def required_mem_per_node(num_tasks_per_node): + def required_mem_per_node(self): mem = {'slope': 0.07, 'intercept': 0.5} - self.mem_required = (num_tasks_per_node * mem['slope'] + mem['intercept']) * 1024 + return (self.num_tasks_per_node * mem['slope'] + mem['intercept']) * 1024 # Set sanity step @deferrable From f096cb9ab57815070e1d3e34b88c504f84209481 Mon Sep 17 00:00:00 2001 From: Caspar van Leeuwen Date: Wed, 4 Sep 2024 23:15:49 +0200 Subject: [PATCH 08/48] More clear SyntaxError message --- eessi/testsuite/eessi_mixin.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/eessi/testsuite/eessi_mixin.py b/eessi/testsuite/eessi_mixin.py index e15ea4d2..80a1e770 100644 --- a/eessi/testsuite/eessi_mixin.py +++ b/eessi/testsuite/eessi_mixin.py @@ -85,7 +85,8 @@ def validate_setup(self): # 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 from EESSI_Mixin in the setup phase (or earlier), " - msg += "but it wasn't. It should be a function that takes the number of tasks per node as argument, and return the memory per node required" + msg += "but it wasn't. Note that this function can use self.num_tasks_per_node, as it will be called after that attribute " + msg += "has been set" raise ReframeSyntaxError(msg) # Check that the value for these variables is valid, i.e. exists in their respetive dict from eessi.testsuite.constants From bee51d2b43b1c76f1aaabee4cb02c9c1f5240605 Mon Sep 17 00:00:00 2001 From: Caspar van Leeuwen Date: Wed, 4 Sep 2024 23:26:30 +0200 Subject: [PATCH 09/48] Added some more suggestions for the future --- eessi/testsuite/eessi_mixin.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/eessi/testsuite/eessi_mixin.py b/eessi/testsuite/eessi_mixin.py index 80a1e770..1b040704 100644 --- a/eessi/testsuite/eessi_mixin.py +++ b/eessi/testsuite/eessi_mixin.py @@ -12,6 +12,10 @@ # 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. From 3ffe7eba508eb08f99b1a8fbfee33d840b353795 Mon Sep 17 00:00:00 2001 From: Caspar van Leeuwen Date: Thu, 19 Sep 2024 12:13:21 +0200 Subject: [PATCH 10/48] Clarify printed message --- eessi/testsuite/tests/apps/lammps/lammps.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/eessi/testsuite/tests/apps/lammps/lammps.py b/eessi/testsuite/tests/apps/lammps/lammps.py index 5e44debe..f8e3642b 100644 --- a/eessi/testsuite/tests/apps/lammps/lammps.py +++ b/eessi/testsuite/tests/apps/lammps/lammps.py @@ -79,7 +79,7 @@ def run_after_setup(self): self.compute_unit = COMPUTE_UNIT['GPU'] # hooks.assign_tasks_per_compute_unit(test=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}') + raise NotImplementedError(f'No mapping of device type {self.device_type} to a COMPUTE_UNIT was specified in this test') # Set OMP_NUM_THREADS environment variable # hooks.set_omp_num_threads(self) From c4a0bccdc09ad812840183462fe83235023fea70 Mon Sep 17 00:00:00 2001 From: Caspar van Leeuwen Date: Thu, 19 Sep 2024 15:30:28 +0200 Subject: [PATCH 11/48] Add memory hook. Make it useable by setting measure_memory_usage --- eessi/testsuite/eessi_mixin.py | 18 +++++++++++++++++- eessi/testsuite/tests/apps/lammps/lammps.py | 6 +----- 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/eessi/testsuite/eessi_mixin.py b/eessi/testsuite/eessi_mixin.py index 1b040704..7438e853 100644 --- a/eessi/testsuite/eessi_mixin.py +++ b/eessi/testsuite/eessi_mixin.py @@ -1,5 +1,6 @@ from reframe.core.pipeline import RegressionMixin from reframe.core.exceptions import ReframeSyntaxError +from reframe.utility.sanity import make_performance_function from eessi.testsuite import hooks from eessi.testsuite.constants import DEVICE_TYPES, CPU, GPU, SCALES, COMPUTE_UNIT @@ -27,6 +28,12 @@ class EESSI_Mixin(RegressionMixin): - Setup phase: compute_unit, required_mem_per_node """ + measure_memory_usage = False +# valid_prog_environs = ['default'] +# valid_systems = ['*'] +# time_limit = '30m' + scale = parameter(SCALES.keys()) + # 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): @@ -54,7 +61,7 @@ def validate_item_in_dict(self, item, item_dict, check_keys=False): 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'] + var_list = ['device_type', 'scale', 'module_name', 'measure_memory_usage'] for var in var_list: if not hasattr(self, var): raise ReframeSyntaxError("The variable '%s' should be defined in any test class that inherits from EESSI_Mixin in the init phase (or earlier), but it wasn't" % var) @@ -78,6 +85,14 @@ def run_after_init(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""" @@ -116,3 +131,4 @@ def assign_tasks_per_compute_unit(self): @run_after('setup') def request_mem(self): hooks.req_memory_per_node(self, app_mem_req=self.required_mem_per_node()) + diff --git a/eessi/testsuite/tests/apps/lammps/lammps.py b/eessi/testsuite/tests/apps/lammps/lammps.py index f8e3642b..a8ea018d 100644 --- a/eessi/testsuite/tests/apps/lammps/lammps.py +++ b/eessi/testsuite/tests/apps/lammps/lammps.py @@ -11,14 +11,10 @@ from eessi.testsuite.eessi_mixin import EESSI_Mixin class EESSI_LAMMPS_base(rfm.RunOnlyRegressionTest, EESSI_Mixin): - # scales = list(SCALES.keys()) - # scales.append('blabla') - # scale = parameter(scales) - scale = parameter(SCALES.keys()) valid_prog_environs = ['default'] valid_systems = ['*'] time_limit = '30m' - #device_type = parameter([DEVICE_TYPES[CPU], DEVICE_TYPES[GPU], 'bla']) + device_type = parameter([DEVICE_TYPES[CPU], DEVICE_TYPES[GPU]]) # Parameterize over all modules that start with LAMMPS From 2f8468733ffbaf8558e00682e919551ffea3ab1c Mon Sep 17 00:00:00 2001 From: Caspar van Leeuwen Date: Thu, 19 Sep 2024 15:40:50 +0200 Subject: [PATCH 12/48] Leave a breadcrumb for our future selves --- eessi/testsuite/eessi_mixin.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/eessi/testsuite/eessi_mixin.py b/eessi/testsuite/eessi_mixin.py index 7438e853..7529c47a 100644 --- a/eessi/testsuite/eessi_mixin.py +++ b/eessi/testsuite/eessi_mixin.py @@ -34,6 +34,11 @@ class EESSI_Mixin(RegressionMixin): # time_limit = '30m' 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() + # 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): From 582b7b6359678232bd9dbb96ea727fbea4ef07d4 Mon Sep 17 00:00:00 2001 From: Caspar van Leeuwen Date: Thu, 19 Sep 2024 23:59:01 +0200 Subject: [PATCH 13/48] Do dynamic version generation, both at build time and runtime. Use the dynamic version determined at runtime and store it as a variable in the eessi_mixin class, so it ends up in all tests that are run. This way, you can alwyas see from the run report of reframe which EESSI test-suite version was used --- eessi/testsuite/__init__.py | 10 ++++++++++ eessi/testsuite/eessi_mixin.py | 8 +++++++- pyproject.toml | 8 ++++++-- setup.cfg | 5 ++++- setup.py | 7 ++++++- 5 files changed, 33 insertions(+), 5 deletions(-) diff --git a/eessi/testsuite/__init__.py b/eessi/testsuite/__init__.py index e69de29b..0cc13fa5 100644 --- a/eessi/testsuite/__init__.py +++ b/eessi/testsuite/__init__.py @@ -0,0 +1,10 @@ +try: + from setuptools_scm import get_version + # Using a relative path for relative_to doesn't work, because it will be relative to the current working + # directory (which could be anywhere) + # __file__ is the location of this init file (a full path), and this gives us a predictable path to the root + # (namely: two levels up) + # Note that if we ever move this __init__ file relative to the root of the git tree, we'll need to adjust this + __version__ = get_version(root='../..', relative_to=__file__) +except ImportError: + __version__ = "0.0.0" # fallback version if setuptools_scm is not available diff --git a/eessi/testsuite/eessi_mixin.py b/eessi/testsuite/eessi_mixin.py index 7529c47a..88573b88 100644 --- a/eessi/testsuite/eessi_mixin.py +++ b/eessi/testsuite/eessi_mixin.py @@ -1,10 +1,13 @@ -from reframe.core.pipeline import RegressionMixin +from reframe.core.builtins import variable from reframe.core.exceptions import ReframeSyntaxError +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, CPU, GPU, SCALES, COMPUTE_UNIT +from eessi.testsuite import __version__ as EESSI_TESTSUITE_VERSION + # 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 @@ -28,6 +31,9 @@ class EESSI_Mixin(RegressionMixin): - Setup phase: compute_unit, required_mem_per_node """ + # Current version of the EESSI test suite + eessi_testsuite_version = variable(str, value=EESSI_TESTSUITE_VERSION) + measure_memory_usage = False # valid_prog_environs = ['default'] # valid_systems = ['*'] diff --git a/pyproject.toml b/pyproject.toml index 445232af..df46752e 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,10 +1,9 @@ [build-system] -requires = ["setuptools"] +requires = ["setuptools>=42", "wheel", "setuptools_scm[toml]"] build-backend = "setuptools.build_meta" [project] name = "eessi-testsuite" -version = "0.3.2" description = "Test suite for the EESSI software stack" readme = "README.md" license = {file = "LICENSE"} @@ -12,6 +11,7 @@ classifiers = [ "Programming Language :: Python" ] requires-python = ">=3.6" +dynamic = ["version"] [project.urls] "Homepage" = "https://eessi.io/docs/test-suite" @@ -19,3 +19,7 @@ requires-python = ">=3.6" [tool.setuptools.packages.find] include = ["eessi*"] + +[tool.setuptools_scm] +version_scheme = "guess-next-dev" +local_scheme = "node-and-date" diff --git a/setup.cfg b/setup.cfg index af7a3b6a..a966abf2 100644 --- a/setup.cfg +++ b/setup.cfg @@ -1,6 +1,5 @@ [metadata] name = eessi-testsuite -version = 0.3.2 description = Test suite for the EESSI software stack long_description = file: README.md long_description_content_type = text/markdown @@ -21,6 +20,10 @@ namespace_packages = eessi [options.packages.find] include = eessi* +[tool:setuptools_scm] +version_scheme = guess-next-dev +local_scheme = node-and-date + [flake8] max-line-length = 120 # ignore star imports (F403, F405) diff --git a/setup.py b/setup.py index a4f49f92..1984c6fd 100644 --- a/setup.py +++ b/setup.py @@ -1,2 +1,7 @@ import setuptools -setuptools.setup() +import os + +setuptools.setup( + use_scm_version=True, + setup_requires=['setuptools_scm'], +) From 97e60abb37126bb7165ec66197459f0b08320f88 Mon Sep 17 00:00:00 2001 From: Caspar van Leeuwen Date: Fri, 20 Sep 2024 00:11:09 +0200 Subject: [PATCH 14/48] First try to get version from installed package. If packate is not installed, try to get it from the git tree --- eessi/testsuite/__init__.py | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/eessi/testsuite/__init__.py b/eessi/testsuite/__init__.py index 0cc13fa5..0e08b1a1 100644 --- a/eessi/testsuite/__init__.py +++ b/eessi/testsuite/__init__.py @@ -1,10 +1,14 @@ +from importlib.metadata import version, PackageNotFoundError try: - from setuptools_scm import get_version - # Using a relative path for relative_to doesn't work, because it will be relative to the current working - # directory (which could be anywhere) - # __file__ is the location of this init file (a full path), and this gives us a predictable path to the root - # (namely: two levels up) - # Note that if we ever move this __init__ file relative to the root of the git tree, we'll need to adjust this - __version__ = get_version(root='../..', relative_to=__file__) -except ImportError: - __version__ = "0.0.0" # fallback version if setuptools_scm is not available + __version__ = version("eessi-testsuite") +except PackageNotFoundError: + try: + from setuptools_scm import get_version + # Using a relative path for relative_to doesn't work, because it will be relative to the current working + # directory (which could be anywhere) + # __file__ is the location of this init file (a full path), and this gives us a predictable path to the root + # (namely: two levels up) + # Note that if we ever move this __init__ file relative to the root of the git tree, we'll need to adjust this + __version__ = get_version(root='../..', relative_to=__file__) + except ImportError: + __version__ = "0.0.0" # fallback version if setuptools_scm is not available From bf48c2f477b9ef202e2d9ea43db7eb16df14e481 Mon Sep 17 00:00:00 2001 From: Caspar van Leeuwen Date: Fri, 20 Sep 2024 21:29:32 +0200 Subject: [PATCH 15/48] Make sure scm writes a version file. If the version file doesn't exist, we're probably in a git-cloned repo, and we resort to using setuptools_scm at runtime to determine the version --- eessi/testsuite/__init__.py | 9 +++++++-- pyproject.toml | 1 + setup.cfg | 2 +- setup.py | 2 +- 4 files changed, 10 insertions(+), 4 deletions(-) diff --git a/eessi/testsuite/__init__.py b/eessi/testsuite/__init__.py index 0e08b1a1..7ed1ba00 100644 --- a/eessi/testsuite/__init__.py +++ b/eessi/testsuite/__init__.py @@ -1,7 +1,12 @@ from importlib.metadata import version, PackageNotFoundError try: - __version__ = version("eessi-testsuite") -except PackageNotFoundError: + from . import _version.py +except ImportError: +# We probably no longer needed this: if it is an installed package, we can read the version from _version.py +# try: +# __version__ = version("eessi-testsuite") +# except PackageNotFoundError: + # Fallback for when it is not an installed package, but is a git clone try: from setuptools_scm import get_version # Using a relative path for relative_to doesn't work, because it will be relative to the current working diff --git a/pyproject.toml b/pyproject.toml index df46752e..49266368 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -23,3 +23,4 @@ include = ["eessi*"] [tool.setuptools_scm] version_scheme = "guess-next-dev" local_scheme = "node-and-date" +version_file = "eessi/testsuite/_version.py" diff --git a/setup.cfg b/setup.cfg index a966abf2..3ef11c6e 100644 --- a/setup.cfg +++ b/setup.cfg @@ -20,7 +20,7 @@ namespace_packages = eessi [options.packages.find] include = eessi* -[tool:setuptools_scm] +[setuptools_scm] version_scheme = guess-next-dev local_scheme = node-and-date diff --git a/setup.py b/setup.py index 1984c6fd..00ce383e 100644 --- a/setup.py +++ b/setup.py @@ -2,6 +2,6 @@ import os setuptools.setup( - use_scm_version=True, + use_scm_version={"version_file": "eessi/testsuite/_version.py"}, setup_requires=['setuptools_scm'], ) From 5095c4ff760d37aca961029d44f51b19d3b8392e Mon Sep 17 00:00:00 2001 From: Caspar van Leeuwen Date: Fri, 20 Sep 2024 21:30:51 +0200 Subject: [PATCH 16/48] Of course, the extension shouldn't be part of the import... --- eessi/testsuite/__init__.py | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/eessi/testsuite/__init__.py b/eessi/testsuite/__init__.py index 7ed1ba00..5b4ac640 100644 --- a/eessi/testsuite/__init__.py +++ b/eessi/testsuite/__init__.py @@ -1,12 +1,10 @@ from importlib.metadata import version, PackageNotFoundError +# If this is an installed package, setuptools_scm should have written the file _version.py in the current directory try: - from . import _version.py + from . import _version except ImportError: -# We probably no longer needed this: if it is an installed package, we can read the version from _version.py -# try: -# __version__ = version("eessi-testsuite") -# except PackageNotFoundError: - # Fallback for when it is not an installed package, but is a git clone + # Fallback for when the package is not installed, but git cloned. Note that this requires setuptools_scm to be + # available as a runtime dependency try: from setuptools_scm import get_version # Using a relative path for relative_to doesn't work, because it will be relative to the current working From 3c2f0c638bd368bf2b94fbd81b65bdb2a95e264c Mon Sep 17 00:00:00 2001 From: Caspar van Leeuwen Date: Fri, 20 Sep 2024 21:35:30 +0200 Subject: [PATCH 17/48] Import was failing, try something else --- eessi/testsuite/__init__.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/eessi/testsuite/__init__.py b/eessi/testsuite/__init__.py index 5b4ac640..3af390c7 100644 --- a/eessi/testsuite/__init__.py +++ b/eessi/testsuite/__init__.py @@ -1,7 +1,8 @@ from importlib.metadata import version, PackageNotFoundError # If this is an installed package, setuptools_scm should have written the file _version.py in the current directory try: - from . import _version + print("try importing version") + from _version import __version__ except ImportError: # Fallback for when the package is not installed, but git cloned. Note that this requires setuptools_scm to be # available as a runtime dependency From 04ca0cafd42a159f9bc5eb065193bf73f3d579e0 Mon Sep 17 00:00:00 2001 From: Caspar van Leeuwen Date: Fri, 20 Sep 2024 21:43:09 +0200 Subject: [PATCH 18/48] Now correctly import __version__ in the global namespace of eessi/testsuite/__init__.py --- eessi/testsuite/__init__.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/eessi/testsuite/__init__.py b/eessi/testsuite/__init__.py index 3af390c7..8bb9e2a5 100644 --- a/eessi/testsuite/__init__.py +++ b/eessi/testsuite/__init__.py @@ -1,8 +1,6 @@ -from importlib.metadata import version, PackageNotFoundError # If this is an installed package, setuptools_scm should have written the file _version.py in the current directory try: - print("try importing version") - from _version import __version__ + from ._version import __version__ except ImportError: # Fallback for when the package is not installed, but git cloned. Note that this requires setuptools_scm to be # available as a runtime dependency From 28376b73f152a8be1af6fad425f1def420d20984 Mon Sep 17 00:00:00 2001 From: Caspar van Leeuwen Date: Fri, 20 Sep 2024 21:54:59 +0200 Subject: [PATCH 19/48] Fix some flake8 errors --- eessi/testsuite/eessi_mixin.py | 52 ++++++++++++++++++---------------- 1 file changed, 27 insertions(+), 25 deletions(-) diff --git a/eessi/testsuite/eessi_mixin.py b/eessi/testsuite/eessi_mixin.py index 88573b88..fa51d2ec 100644 --- a/eessi/testsuite/eessi_mixin.py +++ b/eessi/testsuite/eessi_mixin.py @@ -1,24 +1,25 @@ -from reframe.core.builtins import variable +from reframe.core.builtins import variable, parameter, run_after from reframe.core.exceptions import ReframeSyntaxError 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, CPU, GPU, SCALES, COMPUTE_UNIT +from eessi.testsuite.constants import DEVICE_TYPES, SCALES, COMPUTE_UNIT from eessi.testsuite import __version__ as EESSI_TESTSUITE_VERSION # 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 +# 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? +# 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): """ @@ -61,7 +62,8 @@ def validate_item_in_dict(self, item, item_dict, check_keys=False): value = getattr(self, item) if value not in valid_items: valid_items_str = (', '.join("'" + item + "'" for item in valid_items)) - raise ReframeSyntaxError("The variable '%s' had value '%s', but the only valid values are %s" % (item, value, valid_items_str)) + msg = "The variable '%s' had value '%s', but the only valid values are %s" % (item, value, valid_items_str) + raise ReframeSyntaxError(msg) # We have to make sure that these gets set in any test that inherits # device_type = variable(str) @@ -75,9 +77,12 @@ def validate_init(self): var_list = ['device_type', 'scale', 'module_name', 'measure_memory_usage'] for var in var_list: if not hasattr(self, var): - raise ReframeSyntaxError("The variable '%s' should be defined in any test class that inherits from EESSI_Mixin in the init phase (or earlier), but it wasn't" % 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 ReframeSyntaxError(msg) - # Check that the value for these variables is valid, i.e. exists in their respetive dict from eessi.testsuite.constants + # 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) @@ -110,28 +115,26 @@ def validate_setup(self): var_list = ['compute_unit'] for var in var_list: if not hasattr(self, var): - raise ReframeSyntaxError("The variable '%s' should be defined in any test class that inherits from EESSI_Mixin in the setup phase (or earlier), but it wasn't" % 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 ReframeSyntaxError(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 from EESSI_Mixin in the setup phase (or earlier), " - msg += "but it wasn't. Note that this function can use self.num_tasks_per_node, as it will be called after that attribute " - msg += "has been set" + 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 ReframeSyntaxError(msg) - # Check that the value for these variables is valid, i.e. exists in their respetive dict from eessi.testsuite.constants + # 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): """hooks to run after the setup phase""" hooks.assign_tasks_per_compute_unit(test=self, compute_unit=self.compute_unit) -# if self.device_type == 'cpu': -# hooks.assign_tasks_per_compute_unit(test=self, compute_unit=COMPUTE_UNIT['CPU']) -# elif self.device_type == 'gpu': -# hooks.assign_tasks_per_compute_unit(test=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) @@ -142,4 +145,3 @@ def assign_tasks_per_compute_unit(self): @run_after('setup') def request_mem(self): hooks.req_memory_per_node(self, app_mem_req=self.required_mem_per_node()) - From 439c425c808f03b7b0c99e5492376db77516c53c Mon Sep 17 00:00:00 2001 From: Caspar van Leeuwen Date: Fri, 20 Sep 2024 21:58:24 +0200 Subject: [PATCH 20/48] Fix some more flake8 issues --- eessi/testsuite/tests/apps/lammps/lammps.py | 29 --------------------- setup.py | 1 - 2 files changed, 30 deletions(-) diff --git a/eessi/testsuite/tests/apps/lammps/lammps.py b/eessi/testsuite/tests/apps/lammps/lammps.py index a8ea018d..39a62350 100644 --- a/eessi/testsuite/tests/apps/lammps/lammps.py +++ b/eessi/testsuite/tests/apps/lammps/lammps.py @@ -51,45 +51,16 @@ 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('init') def run_after_setup(self): """hooks to run after the setup phase""" if self.device_type == 'cpu': self.compute_unit = COMPUTE_UNIT['CPU'] - # hooks.assign_tasks_per_compute_unit(test=self, compute_unit=COMPUTE_UNIT['CPU']) elif self.device_type == 'gpu': self.compute_unit = COMPUTE_UNIT['GPU'] - # hooks.assign_tasks_per_compute_unit(test=self, compute_unit=COMPUTE_UNIT['GPU']) else: raise NotImplementedError(f'No mapping of device type {self.device_type} to a COMPUTE_UNIT was specified in this test') - # Set OMP_NUM_THREADS environment variable - # hooks.set_omp_num_threads(self) - - # Set compact process binding - # hooks.set_compact_process_binding(self) - - - #@run_after('init') - #def request_mem(self): - #mem = {'slope': 0.07, 'intercept': 0.5} - # self.mem_required = (self.num_tasks_per_node * mem['slope'] + mem['intercept']) * 1024 - # hooks.req_memory_per_node(self, app_mem_req=mem_required * 1024) - @rfm.simple_test class EESSI_LAMMPS_lj(EESSI_LAMMPS_base): diff --git a/setup.py b/setup.py index 00ce383e..b1b0dd55 100644 --- a/setup.py +++ b/setup.py @@ -1,5 +1,4 @@ import setuptools -import os setuptools.setup( use_scm_version={"version_file": "eessi/testsuite/_version.py"}, From 72d9a39371403ec5bc7ef4b7e91c22b9a8db1cf7 Mon Sep 17 00:00:00 2001 From: Caspar van Leeuwen Date: Fri, 20 Sep 2024 22:01:30 +0200 Subject: [PATCH 21/48] Fix more flake8 issues --- eessi/testsuite/eessi_mixin.py | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/eessi/testsuite/eessi_mixin.py b/eessi/testsuite/eessi_mixin.py index fa51d2ec..273b4c1c 100644 --- a/eessi/testsuite/eessi_mixin.py +++ b/eessi/testsuite/eessi_mixin.py @@ -16,9 +16,9 @@ # 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 +# 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): @@ -50,7 +50,7 @@ class EESSI_Mixin(RegressionMixin): # 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'. + 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'. """ @@ -59,12 +59,12 @@ def validate_item_in_dict(self, item, item_dict, check_keys=False): else: valid_items = list(item_dict.values()) - value = getattr(self, item) + value = getattr(self, item) if value not in valid_items: valid_items_str = (', '.join("'" + item + "'" for item in valid_items)) msg = "The variable '%s' had value '%s', but the only valid values are %s" % (item, value, valid_items_str) raise ReframeSyntaxError(msg) - + # We have to make sure that these gets set in any test that inherits # device_type = variable(str) # scale = variable(str) @@ -86,7 +86,6 @@ def validate_init(self): self.validate_item_in_dict('device_type', DEVICE_TYPES) self.validate_item_in_dict('scale', SCALES, check_keys=True) - @run_after('init') def run_after_init(self): """Hooks to run after init phase""" @@ -104,10 +103,10 @@ def run_after_init(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) + 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): @@ -119,7 +118,8 @@ def validate_setup(self): msg += " from EESSI_Mixin in the setup phase (or earlier), but it wasn't" raise ReframeSyntaxError(msg) - # Check if mem_func was defined to compute the required memory per node as function of the number of tasks per node + # 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" From 7909b2d81d3c351c27ac850a689a4efdad803b31 Mon Sep 17 00:00:00 2001 From: Caspar van Leeuwen Date: Fri, 20 Sep 2024 22:03:12 +0200 Subject: [PATCH 22/48] Fix more flake8 stuff --- eessi/testsuite/eessi_mixin.py | 1 + eessi/testsuite/tests/apps/lammps/lammps.py | 4 +++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/eessi/testsuite/eessi_mixin.py b/eessi/testsuite/eessi_mixin.py index 273b4c1c..0404dd59 100644 --- a/eessi/testsuite/eessi_mixin.py +++ b/eessi/testsuite/eessi_mixin.py @@ -8,6 +8,7 @@ from eessi.testsuite import __version__ as EESSI_TESTSUITE_VERSION + # 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, diff --git a/eessi/testsuite/tests/apps/lammps/lammps.py b/eessi/testsuite/tests/apps/lammps/lammps.py index 39a62350..607ed361 100644 --- a/eessi/testsuite/tests/apps/lammps/lammps.py +++ b/eessi/testsuite/tests/apps/lammps/lammps.py @@ -10,6 +10,7 @@ from eessi.testsuite.constants import * # noqa from eessi.testsuite.eessi_mixin import EESSI_Mixin + class EESSI_LAMMPS_base(rfm.RunOnlyRegressionTest, EESSI_Mixin): valid_prog_environs = ['default'] valid_systems = ['*'] @@ -59,7 +60,8 @@ def run_after_setup(self): elif self.device_type == 'gpu': self.compute_unit = COMPUTE_UNIT['GPU'] else: - raise NotImplementedError(f'No mapping of device type {self.device_type} to a COMPUTE_UNIT was specified in this test') + 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 From 204a8197096dcad3317b93855d3d5cdad5b6cc1d Mon Sep 17 00:00:00 2001 From: Caspar van Leeuwen Date: Fri, 20 Sep 2024 22:14:02 +0200 Subject: [PATCH 23/48] Remove whitespace on blank line --- eessi/testsuite/eessi_mixin.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/eessi/testsuite/eessi_mixin.py b/eessi/testsuite/eessi_mixin.py index 0404dd59..deffcd0d 100644 --- a/eessi/testsuite/eessi_mixin.py +++ b/eessi/testsuite/eessi_mixin.py @@ -65,7 +65,7 @@ def validate_item_in_dict(self, item, item_dict, check_keys=False): valid_items_str = (', '.join("'" + item + "'" for item in valid_items)) msg = "The variable '%s' had value '%s', but the only valid values are %s" % (item, value, valid_items_str) raise ReframeSyntaxError(msg) - + # We have to make sure that these gets set in any test that inherits # device_type = variable(str) # scale = variable(str) From a396f6a0c5a817307e67012ccb98070058721d73 Mon Sep 17 00:00:00 2001 From: Caspar van Leeuwen Date: Fri, 20 Sep 2024 22:20:30 +0200 Subject: [PATCH 24/48] Enforce new enough setuptools_scm so that it knows the version_file argument --- pyproject.toml | 2 +- setup.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 49266368..4b2970b3 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,5 +1,5 @@ [build-system] -requires = ["setuptools>=42", "wheel", "setuptools_scm[toml]"] +requires = ["setuptools>=42", "wheel", "setuptools_scm[toml]>=8"] build-backend = "setuptools.build_meta" [project] diff --git a/setup.py b/setup.py index b1b0dd55..907d8052 100644 --- a/setup.py +++ b/setup.py @@ -2,5 +2,5 @@ setuptools.setup( use_scm_version={"version_file": "eessi/testsuite/_version.py"}, - setup_requires=['setuptools_scm'], + setup_requires=['setuptools_scm>=8'], ) From c82110649ea0df46c1c3143e379d299a254fb540 Mon Sep 17 00:00:00 2001 From: Caspar van Leeuwen Date: Fri, 20 Sep 2024 22:32:45 +0200 Subject: [PATCH 25/48] Make usage of write_to or version_file dependent on the version of python (and thus the version of setuptools_scm) used --- setup.py | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/setup.py b/setup.py index 907d8052..0ce645b2 100644 --- a/setup.py +++ b/setup.py @@ -1,6 +1,15 @@ +import sys import setuptools +# write_to got replaced by version_file starting from setuptools_scm v8. +# This version is only supported for python 3.8 and above +python_version = sys.version_info +if python_version < (3, 8): + scm_arg_key = "write_to" +else: + scm_arg_key = "version_file" + setuptools.setup( - use_scm_version={"version_file": "eessi/testsuite/_version.py"}, - setup_requires=['setuptools_scm>=8'], + use_scm_version={scm_arg_key: "eessi/testsuite/_version.py"}, + setup_requires=['setuptools_scm'], ) From 536a4441da9b61d0c50fc1bc5a9355c3a9a0fbe1 Mon Sep 17 00:00:00 2001 From: Caspar van Leeuwen Date: Fri, 20 Sep 2024 22:42:10 +0200 Subject: [PATCH 26/48] Let setup.py handle the choice of argument for writing the version file --- pyproject.toml | 1 - 1 file changed, 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index 4b2970b3..f1ee2ec8 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -23,4 +23,3 @@ include = ["eessi*"] [tool.setuptools_scm] version_scheme = "guess-next-dev" local_scheme = "node-and-date" -version_file = "eessi/testsuite/_version.py" From 84178cd6053e9524babf896a8c6e4b5cc17eceaf Mon Sep 17 00:00:00 2001 From: Caspar van Leeuwen Date: Fri, 20 Sep 2024 22:45:09 +0200 Subject: [PATCH 27/48] Make things conditional on version of setuptools_scm directly --- setup.py | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/setup.py b/setup.py index 0ce645b2..f9951b9c 100644 --- a/setup.py +++ b/setup.py @@ -1,13 +1,21 @@ -import sys import setuptools +import pkg_resources # write_to got replaced by version_file starting from setuptools_scm v8. -# This version is only supported for python 3.8 and above +def get_setuptools_scm_version(): + try: + scm_version = pkg_resources.get_distribution("setuptools_scm").version + return tuple(map(int, scm_version.split(".")[:2])) # Convert version string to tuple, e.g., (8, 0) + except pkg_resources.DistributionNotFound: + return (0, 0) # If setuptools_scm is not found, assume version 0.0 + +scm_version = get_setuptools_scm_version() + python_version = sys.version_info -if python_version < (3, 8): - scm_arg_key = "write_to" -else: +if scm_version >= (8, 0): scm_arg_key = "version_file" +else: + scm_arg_key = "write_to" setuptools.setup( use_scm_version={scm_arg_key: "eessi/testsuite/_version.py"}, From d90e831ebf2b24d6dddc9608d09e2d56a9b34457 Mon Sep 17 00:00:00 2001 From: Caspar van Leeuwen Date: Fri, 20 Sep 2024 22:46:02 +0200 Subject: [PATCH 28/48] No longer enforce version v8 or higher for setuptools_scm, it doesn't work for older python --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index f1ee2ec8..df46752e 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,5 +1,5 @@ [build-system] -requires = ["setuptools>=42", "wheel", "setuptools_scm[toml]>=8"] +requires = ["setuptools>=42", "wheel", "setuptools_scm[toml]"] build-backend = "setuptools.build_meta" [project] From 95a6d6b8bdb2f0474f7bd267f60d005bd3cb3aa6 Mon Sep 17 00:00:00 2001 From: Caspar van Leeuwen Date: Fri, 20 Sep 2024 22:47:18 +0200 Subject: [PATCH 29/48] Remove stray line --- setup.py | 1 - 1 file changed, 1 deletion(-) diff --git a/setup.py b/setup.py index f9951b9c..6ff5bc64 100644 --- a/setup.py +++ b/setup.py @@ -11,7 +11,6 @@ def get_setuptools_scm_version(): scm_version = get_setuptools_scm_version() -python_version = sys.version_info if scm_version >= (8, 0): scm_arg_key = "version_file" else: From a79b263f1130a0055940ee6bf3cc6b2227179d44 Mon Sep 17 00:00:00 2001 From: Caspar van Leeuwen Date: Fri, 20 Sep 2024 22:48:17 +0200 Subject: [PATCH 30/48] Make flake8 happy again --- setup.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/setup.py b/setup.py index 6ff5bc64..ebbc9f03 100644 --- a/setup.py +++ b/setup.py @@ -1,6 +1,7 @@ import setuptools import pkg_resources + # write_to got replaced by version_file starting from setuptools_scm v8. def get_setuptools_scm_version(): try: @@ -9,6 +10,7 @@ def get_setuptools_scm_version(): except pkg_resources.DistributionNotFound: return (0, 0) # If setuptools_scm is not found, assume version 0.0 + scm_version = get_setuptools_scm_version() if scm_version >= (8, 0): From 3f0d174d6cfa943abf5a4a95436d19579b6bc83e Mon Sep 17 00:00:00 2001 From: Caspar van Leeuwen Date: Fri, 20 Sep 2024 22:57:08 +0200 Subject: [PATCH 31/48] Just stick to write_to for everything, and keep the setuptools_scm version below 8 to stay compatible with python 3.6. Once we don't care about 3.6 compatibility anymore, we can upgrade to setuptools_scm and use version_file instead of write_to --- pyproject.toml | 1 + setup.py | 19 +------------------ 2 files changed, 2 insertions(+), 18 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index df46752e..93f2ddeb 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -23,3 +23,4 @@ include = ["eessi*"] [tool.setuptools_scm] version_scheme = "guess-next-dev" local_scheme = "node-and-date" +write_to = "eessi/testsuite/_version.py" diff --git a/setup.py b/setup.py index ebbc9f03..8fa55910 100644 --- a/setup.py +++ b/setup.py @@ -1,24 +1,7 @@ import setuptools import pkg_resources - -# write_to got replaced by version_file starting from setuptools_scm v8. -def get_setuptools_scm_version(): - try: - scm_version = pkg_resources.get_distribution("setuptools_scm").version - return tuple(map(int, scm_version.split(".")[:2])) # Convert version string to tuple, e.g., (8, 0) - except pkg_resources.DistributionNotFound: - return (0, 0) # If setuptools_scm is not found, assume version 0.0 - - -scm_version = get_setuptools_scm_version() - -if scm_version >= (8, 0): - scm_arg_key = "version_file" -else: - scm_arg_key = "write_to" - setuptools.setup( - use_scm_version={scm_arg_key: "eessi/testsuite/_version.py"}, + use_scm_version={'write_to': 'eessi/testsuite/_version.py'}, setup_requires=['setuptools_scm'], ) From 430dc10e5f5a4426bf1c07150641c346d0884d3f Mon Sep 17 00:00:00 2001 From: Caspar van Leeuwen Date: Fri, 20 Sep 2024 22:57:49 +0200 Subject: [PATCH 32/48] Remove import that isn't used --- setup.py | 1 - 1 file changed, 1 deletion(-) diff --git a/setup.py b/setup.py index 8fa55910..1445b65f 100644 --- a/setup.py +++ b/setup.py @@ -1,5 +1,4 @@ import setuptools -import pkg_resources setuptools.setup( use_scm_version={'write_to': 'eessi/testsuite/_version.py'}, From 8bbb58278cb474c1585ef600a8f895fed157887a Mon Sep 17 00:00:00 2001 From: Caspar van Leeuwen Date: Fri, 20 Sep 2024 22:58:42 +0200 Subject: [PATCH 33/48] Fix CI issue for python 3.6 --- pyproject.toml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index 93f2ddeb..3b55df84 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,5 +1,6 @@ [build-system] -requires = ["setuptools>=42", "wheel", "setuptools_scm[toml]"] +# Setuptools_scm<7 to be compatible with Python 3.6 +requires = ["setuptools>=42", "wheel", "setuptools_scm[toml]<7"] build-backend = "setuptools.build_meta" [project] From 25f0167e60bb7ecb97d9c2900e76d68c54b39f1c Mon Sep 17 00:00:00 2001 From: Caspar van Leeuwen Date: Fri, 20 Sep 2024 23:00:23 +0200 Subject: [PATCH 34/48] Fix version of setuptools_scm below 8 --- pyproject.toml | 2 +- setup.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 3b55df84..fe12bfdb 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [build-system] # Setuptools_scm<7 to be compatible with Python 3.6 -requires = ["setuptools>=42", "wheel", "setuptools_scm[toml]<7"] +requires = ["setuptools>=42", "wheel", "setuptools_scm[toml]<8"] build-backend = "setuptools.build_meta" [project] diff --git a/setup.py b/setup.py index 1445b65f..4e03d25c 100644 --- a/setup.py +++ b/setup.py @@ -2,5 +2,5 @@ setuptools.setup( use_scm_version={'write_to': 'eessi/testsuite/_version.py'}, - setup_requires=['setuptools_scm'], + setup_requires=['setuptools_scm<8'], ) From 618df8e9a714b3f044153769b9788bc84029915e Mon Sep 17 00:00:00 2001 From: Caspar van Leeuwen Date: Fri, 20 Sep 2024 23:02:27 +0200 Subject: [PATCH 35/48] Stick to setuptools_scm < 7 to be compatible with python 3.6 --- pyproject.toml | 2 +- setup.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index fe12bfdb..3b55df84 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [build-system] # Setuptools_scm<7 to be compatible with Python 3.6 -requires = ["setuptools>=42", "wheel", "setuptools_scm[toml]<8"] +requires = ["setuptools>=42", "wheel", "setuptools_scm[toml]<7"] build-backend = "setuptools.build_meta" [project] diff --git a/setup.py b/setup.py index 4e03d25c..41500aa3 100644 --- a/setup.py +++ b/setup.py @@ -2,5 +2,5 @@ setuptools.setup( use_scm_version={'write_to': 'eessi/testsuite/_version.py'}, - setup_requires=['setuptools_scm<8'], + setup_requires=['setuptools_scm<6'], ) From e0295ea57a845e61aa3b557bdc372557bd65d92f Mon Sep 17 00:00:00 2001 From: Caspar van Leeuwen Date: Fri, 20 Sep 2024 23:04:12 +0200 Subject: [PATCH 36/48] Make it < 7 consistently... --- setup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.py b/setup.py index 41500aa3..78bd69be 100644 --- a/setup.py +++ b/setup.py @@ -2,5 +2,5 @@ setuptools.setup( use_scm_version={'write_to': 'eessi/testsuite/_version.py'}, - setup_requires=['setuptools_scm<6'], + setup_requires=['setuptools_scm<7'], ) From 070a549a351557967982c34c92356f43dfbca13f Mon Sep 17 00:00:00 2001 From: Caspar van Leeuwen Date: Fri, 20 Sep 2024 23:08:22 +0200 Subject: [PATCH 37/48] Also limit version of packaging used for compatibility with python 3.6 --- pyproject.toml | 2 +- setup.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 3b55df84..0293d867 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [build-system] # Setuptools_scm<7 to be compatible with Python 3.6 -requires = ["setuptools>=42", "wheel", "setuptools_scm[toml]<7"] +requires = ["setuptools>=42", "wheel", "setuptools_scm[toml]<7", "packaging<=21.3"] build-backend = "setuptools.build_meta" [project] diff --git a/setup.py b/setup.py index 78bd69be..19b57a91 100644 --- a/setup.py +++ b/setup.py @@ -2,5 +2,5 @@ setuptools.setup( use_scm_version={'write_to': 'eessi/testsuite/_version.py'}, - setup_requires=['setuptools_scm<7'], + setup_requires=['setuptools_scm<7','packaging<21.3'], ) From 5108a85d9cb16efb16b7aaa0c5923f9fc6740374 Mon Sep 17 00:00:00 2001 From: Caspar van Leeuwen Date: Fri, 20 Sep 2024 23:09:27 +0200 Subject: [PATCH 38/48] Make things consistent.. --- setup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.py b/setup.py index 19b57a91..2033dcd2 100644 --- a/setup.py +++ b/setup.py @@ -2,5 +2,5 @@ setuptools.setup( use_scm_version={'write_to': 'eessi/testsuite/_version.py'}, - setup_requires=['setuptools_scm<7','packaging<21.3'], + setup_requires=['setuptools_scm<7','packaging<=21.3'], ) From c8bf4a674b1d46a1d3cdd4a003d3f30209f80767 Mon Sep 17 00:00:00 2001 From: Caspar van Leeuwen Date: Fri, 20 Sep 2024 23:29:24 +0200 Subject: [PATCH 39/48] let setup.py handle the setuptools_scm conditionally, see if that works... --- pyproject.toml | 2 +- setup.py | 16 ++++++++++++++-- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 0293d867..10b7c7e6 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [build-system] # Setuptools_scm<7 to be compatible with Python 3.6 -requires = ["setuptools>=42", "wheel", "setuptools_scm[toml]<7", "packaging<=21.3"] +requires = ["setuptools>=42", "wheel"] build-backend = "setuptools.build_meta" [project] diff --git a/setup.py b/setup.py index 2033dcd2..80e0b070 100644 --- a/setup.py +++ b/setup.py @@ -1,6 +1,18 @@ import setuptools +import sys + +scm_version = get_setuptools_scm_version() + +python_version = sys.version_info +if python_version < (3, 8): + scm_require='setuptools_scm<7' + scm_arg_key = "write_to" +else: + scm_require='setuptools_scm>=8' + scm_arg_key = "version_file" + setuptools.setup( - use_scm_version={'write_to': 'eessi/testsuite/_version.py'}, - setup_requires=['setuptools_scm<7','packaging<=21.3'], + use_scm_version={scm_arg_key: 'eessi/testsuite/_version.py'}, + setup_requires=[scm_require], ) From d531e415846979ec1206d0c3f9066102b9406d37 Mon Sep 17 00:00:00 2001 From: Caspar van Leeuwen Date: Fri, 20 Sep 2024 23:30:12 +0200 Subject: [PATCH 40/48] remove stray line --- setup.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/setup.py b/setup.py index 80e0b070..4de6fbed 100644 --- a/setup.py +++ b/setup.py @@ -1,8 +1,6 @@ import setuptools import sys -scm_version = get_setuptools_scm_version() - python_version = sys.version_info if python_version < (3, 8): scm_require='setuptools_scm<7' From 645bb18f5ffe2f7c6d3db6087ff95492ced1fb19 Mon Sep 17 00:00:00 2001 From: Caspar van Leeuwen Date: Fri, 20 Sep 2024 23:31:08 +0200 Subject: [PATCH 41/48] Fix flake8 --- setup.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/setup.py b/setup.py index 4de6fbed..0685a6ac 100644 --- a/setup.py +++ b/setup.py @@ -3,10 +3,10 @@ python_version = sys.version_info if python_version < (3, 8): - scm_require='setuptools_scm<7' + scm_require = 'setuptools_scm<7' scm_arg_key = "write_to" else: - scm_require='setuptools_scm>=8' + scm_require = 'setuptools_scm>=8' scm_arg_key = "version_file" From ef985d11cca713c80173a6d964124e0900461589 Mon Sep 17 00:00:00 2001 From: Caspar van Leeuwen Date: Fri, 20 Sep 2024 23:33:26 +0200 Subject: [PATCH 42/48] Make sure packaging version isn't too new for Python 3.6 --- setup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.py b/setup.py index 0685a6ac..6096cb35 100644 --- a/setup.py +++ b/setup.py @@ -3,7 +3,7 @@ python_version = sys.version_info if python_version < (3, 8): - scm_require = 'setuptools_scm<7' + scm_require = 'packaging<=21.3, setuptools_scm<7' scm_arg_key = "write_to" else: scm_require = 'setuptools_scm>=8' From a6ea23c112f651990ee6f35fabb260bdd8dc8de5 Mon Sep 17 00:00:00 2001 From: Caspar van Leeuwen Date: Fri, 20 Sep 2024 23:34:43 +0200 Subject: [PATCH 43/48] Pass list of strings --- setup.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/setup.py b/setup.py index 6096cb35..741deaa4 100644 --- a/setup.py +++ b/setup.py @@ -3,14 +3,14 @@ python_version = sys.version_info if python_version < (3, 8): - scm_require = 'packaging<=21.3, setuptools_scm<7' + scm_require = ['packaging<=21.3, setuptools_scm<7'] scm_arg_key = "write_to" else: - scm_require = 'setuptools_scm>=8' + scm_require = ['setuptools_scm>=8'] scm_arg_key = "version_file" setuptools.setup( use_scm_version={scm_arg_key: 'eessi/testsuite/_version.py'}, - setup_requires=[scm_require], + setup_requires=scm_require, ) From 1f1ee2df1d22751f1d04f6694aa1ef7c38c726cd Mon Sep 17 00:00:00 2001 From: Caspar van Leeuwen Date: Fri, 20 Sep 2024 23:35:48 +0200 Subject: [PATCH 44/48] Now really fix it --- setup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.py b/setup.py index 741deaa4..ea501b76 100644 --- a/setup.py +++ b/setup.py @@ -3,7 +3,7 @@ python_version = sys.version_info if python_version < (3, 8): - scm_require = ['packaging<=21.3, setuptools_scm<7'] + scm_require = ['packaging<=21.3', 'setuptools_scm<7'] scm_arg_key = "write_to" else: scm_require = ['setuptools_scm>=8'] From 38ffee1dd59db661cdc1f1ae223f5cd63191113f Mon Sep 17 00:00:00 2001 From: Caspar van Leeuwen Date: Sat, 21 Sep 2024 11:11:24 +0200 Subject: [PATCH 45/48] Split off the setuptools_scm versioning into a different pr https://github.com/EESSI/test-suite/pull/185 . The current PR now depends on this, as it relies on the eessi/testsuite/__init__.py file to provide a version --- eessi/testsuite/__init__.py | 16 ---------------- pyproject.toml | 10 ++-------- setup.cfg | 5 +---- setup.py | 16 +--------------- 4 files changed, 4 insertions(+), 43 deletions(-) diff --git a/eessi/testsuite/__init__.py b/eessi/testsuite/__init__.py index 8bb9e2a5..e69de29b 100644 --- a/eessi/testsuite/__init__.py +++ b/eessi/testsuite/__init__.py @@ -1,16 +0,0 @@ -# If this is an installed package, setuptools_scm should have written the file _version.py in the current directory -try: - from ._version import __version__ -except ImportError: - # Fallback for when the package is not installed, but git cloned. Note that this requires setuptools_scm to be - # available as a runtime dependency - try: - from setuptools_scm import get_version - # Using a relative path for relative_to doesn't work, because it will be relative to the current working - # directory (which could be anywhere) - # __file__ is the location of this init file (a full path), and this gives us a predictable path to the root - # (namely: two levels up) - # Note that if we ever move this __init__ file relative to the root of the git tree, we'll need to adjust this - __version__ = get_version(root='../..', relative_to=__file__) - except ImportError: - __version__ = "0.0.0" # fallback version if setuptools_scm is not available diff --git a/pyproject.toml b/pyproject.toml index 10b7c7e6..445232af 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,10 +1,10 @@ [build-system] -# Setuptools_scm<7 to be compatible with Python 3.6 -requires = ["setuptools>=42", "wheel"] +requires = ["setuptools"] build-backend = "setuptools.build_meta" [project] name = "eessi-testsuite" +version = "0.3.2" description = "Test suite for the EESSI software stack" readme = "README.md" license = {file = "LICENSE"} @@ -12,7 +12,6 @@ classifiers = [ "Programming Language :: Python" ] requires-python = ">=3.6" -dynamic = ["version"] [project.urls] "Homepage" = "https://eessi.io/docs/test-suite" @@ -20,8 +19,3 @@ dynamic = ["version"] [tool.setuptools.packages.find] include = ["eessi*"] - -[tool.setuptools_scm] -version_scheme = "guess-next-dev" -local_scheme = "node-and-date" -write_to = "eessi/testsuite/_version.py" diff --git a/setup.cfg b/setup.cfg index 3ef11c6e..af7a3b6a 100644 --- a/setup.cfg +++ b/setup.cfg @@ -1,5 +1,6 @@ [metadata] name = eessi-testsuite +version = 0.3.2 description = Test suite for the EESSI software stack long_description = file: README.md long_description_content_type = text/markdown @@ -20,10 +21,6 @@ namespace_packages = eessi [options.packages.find] include = eessi* -[setuptools_scm] -version_scheme = guess-next-dev -local_scheme = node-and-date - [flake8] max-line-length = 120 # ignore star imports (F403, F405) diff --git a/setup.py b/setup.py index ea501b76..a4f49f92 100644 --- a/setup.py +++ b/setup.py @@ -1,16 +1,2 @@ import setuptools -import sys - -python_version = sys.version_info -if python_version < (3, 8): - scm_require = ['packaging<=21.3', 'setuptools_scm<7'] - scm_arg_key = "write_to" -else: - scm_require = ['setuptools_scm>=8'] - scm_arg_key = "version_file" - - -setuptools.setup( - use_scm_version={scm_arg_key: 'eessi/testsuite/_version.py'}, - setup_requires=scm_require, -) +setuptools.setup() From 94d16e77ea53e0e14a199f6d3199e08aa6d7166f Mon Sep 17 00:00:00 2001 From: Samuel Moors Date: Sun, 6 Oct 2024 15:54:21 +0200 Subject: [PATCH 46/48] update mixin class --- eessi/testsuite/eessi_mixin.py | 33 ++++++++++++--------- eessi/testsuite/tests/apps/lammps/lammps.py | 3 -- 2 files changed, 19 insertions(+), 17 deletions(-) diff --git a/eessi/testsuite/eessi_mixin.py b/eessi/testsuite/eessi_mixin.py index deffcd0d..c885533b 100644 --- a/eessi/testsuite/eessi_mixin.py +++ b/eessi/testsuite/eessi_mixin.py @@ -1,13 +1,11 @@ -from reframe.core.builtins import variable, parameter, run_after -from reframe.core.exceptions import ReframeSyntaxError +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 -from eessi.testsuite import __version__ as EESSI_TESTSUITE_VERSION - # 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 @@ -34,12 +32,8 @@ class EESSI_Mixin(RegressionMixin): """ # Current version of the EESSI test suite - eessi_testsuite_version = variable(str, value=EESSI_TESTSUITE_VERSION) measure_memory_usage = False -# valid_prog_environs = ['default'] -# valid_systems = ['*'] -# time_limit = '30m' 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 @@ -47,6 +41,13 @@ class EESSI_Mixin(RegressionMixin): # 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 = ['*'] + 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): @@ -62,9 +63,11 @@ def validate_item_in_dict(self, item, item_dict, check_keys=False): value = getattr(self, item) if value not in valid_items: - valid_items_str = (', '.join("'" + item + "'" for item in valid_items)) - msg = "The variable '%s' had value '%s', but the only valid values are %s" % (item, value, valid_items_str) - raise ReframeSyntaxError(msg) + 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) # We have to make sure that these gets set in any test that inherits # device_type = variable(str) @@ -80,12 +83,14 @@ def validate_init(self): 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 ReframeSyntaxError(msg) + 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): @@ -117,7 +122,7 @@ def validate_setup(self): 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 ReframeSyntaxError(msg) + 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 @@ -126,7 +131,7 @@ def validate_setup(self): 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 ReframeSyntaxError(msg) + raise ReframeFatalError(msg) # Check that the value for these variables is valid # i.e. exists in their respective dict from eessi.testsuite.constants diff --git a/eessi/testsuite/tests/apps/lammps/lammps.py b/eessi/testsuite/tests/apps/lammps/lammps.py index 607ed361..22e70d3e 100644 --- a/eessi/testsuite/tests/apps/lammps/lammps.py +++ b/eessi/testsuite/tests/apps/lammps/lammps.py @@ -12,10 +12,7 @@ class EESSI_LAMMPS_base(rfm.RunOnlyRegressionTest, EESSI_Mixin): - valid_prog_environs = ['default'] - valid_systems = ['*'] time_limit = '30m' - device_type = parameter([DEVICE_TYPES[CPU], DEVICE_TYPES[GPU]]) # Parameterize over all modules that start with LAMMPS From b69d1ee849efab347d3938d084bedc3b1f0001fd Mon Sep 17 00:00:00 2001 From: Samuel Moors Date: Mon, 7 Oct 2024 08:53:48 +0200 Subject: [PATCH 47/48] do not override time_limit --- eessi/testsuite/eessi_mixin.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/eessi/testsuite/eessi_mixin.py b/eessi/testsuite/eessi_mixin.py index c885533b..961312e6 100644 --- a/eessi/testsuite/eessi_mixin.py +++ b/eessi/testsuite/eessi_mixin.py @@ -46,7 +46,8 @@ def __init_subclass__(cls, **kwargs): super().__init_subclass__(**kwargs) cls.valid_prog_environs = ['default'] cls.valid_systems = ['*'] - cls.time_limit = '1h' + 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 From 24951c8ec625d3706752110d57a8f1957bc46c26 Mon Sep 17 00:00:00 2001 From: Caspar van Leeuwen Date: Wed, 9 Oct 2024 11:44:43 +0200 Subject: [PATCH 48/48] Some cleanup --- eessi/testsuite/eessi_mixin.py | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/eessi/testsuite/eessi_mixin.py b/eessi/testsuite/eessi_mixin.py index 961312e6..e87d5311 100644 --- a/eessi/testsuite/eessi_mixin.py +++ b/eessi/testsuite/eessi_mixin.py @@ -25,14 +25,17 @@ 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, and by (the end of) what phase: + 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 - """ - # Current version of the EESSI test suite + The child class may also overwrite the following attributes: + + - Init phase: time_limit, measure_memory_usage + """ + # Set defaults for these class variables, can be overwritten by child class if desired measure_memory_usage = False scale = parameter(SCALES.keys()) @@ -70,11 +73,6 @@ def validate_item_in_dict(self, item, item_dict, check_keys=False): msg = f"The variable '{item}' has value {value}, but the only valid values are {valid_items}" raise ReframeFatalError(msg) - # We have to make sure that these gets set in any test that inherits - # device_type = variable(str) - # scale = variable(str) - # module_name = variable(str) - @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""" @@ -140,7 +138,7 @@ def validate_setup(self): @run_after('setup') def assign_tasks_per_compute_unit(self): - """hooks to run after the setup phase""" + """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 @@ -151,4 +149,5 @@ def assign_tasks_per_compute_unit(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())