From ccc3350e289ba22039d7d9a49bbc847fd5c05e6e Mon Sep 17 00:00:00 2001 From: Javier Otero Date: Thu, 5 Nov 2020 11:37:35 +0100 Subject: [PATCH 01/43] Add support for directive-based test parameters. --- reframe/core/.decorators.py.swp | Bin 0 -> 24576 bytes reframe/core/attributes.py | 161 ++++++++++++++++++++++++++++++++ reframe/core/decorators.py | 52 ++++++++++- reframe/core/meta.py | 23 ++++- reframe/core/pipeline.py | 40 ++++++++ 5 files changed, 271 insertions(+), 5 deletions(-) create mode 100644 reframe/core/.decorators.py.swp create mode 100644 reframe/core/attributes.py diff --git a/reframe/core/.decorators.py.swp b/reframe/core/.decorators.py.swp new file mode 100644 index 0000000000000000000000000000000000000000..0f3c1cde587bc5af5ffeed404ff0c52f0168cbf2 GIT binary patch literal 24576 zcmeI3Ym6kxa3L#op2?f*e)`$=^1z!AJF*MmIT*41|}%k^D-%~w=rc@YlB&rhr= z(MX_?zzdYXt-+y{Wn=1d`(LB(eCbKqG-h0{@pK zP!4Ylevl5l$aTV>_P@#f_TM+U-#5GO;mrGQcE1bveP!nT4es|n?)&pI@7>SltC2t> zfkpz21R4o65@;mQNT88GBY{Q&jRYDAG!pnF8?_kuQPftQ2N+!_SG3dZ0lI0D`P zUIqU1B|-4};FrNRI0+7cSAti7FWwRa&w*#buYfUl44eb62e*Km!GGYW_(SkPP=E#S zuQyQ#yd4||$G|~w7q|`l3mheX4t^Va1pEwm0$c@mgKq<0z}fQG;IF__;CsLw;G;NE zejU6IoChIT0{_m~eFl6Q$T&U&ehK_Acqe!oWZ=8O0r20>siXLu+S1BpUg*B+B~ekR z?NL$kFpqb%8syn_5J<^Zt98u0t`arWAj?&*H{*gdkvQE{!${IP_>pMHl)5MjbvZ7# z)WiCIc@qYGJy1owJxp|`6r%F!B}LfD2iqNeWf-M>-R~GWonjdEv^uO}2)5a^c>%;<84uvD(zovXTjUN}yR z@?3mAkRtvgvWgdL5m89#bhNF>dC|g_m4TsC%&!K%o_tr$K>O;Gb^i=a+7eo|fQEV2 zAN9)i!nql$;ifLz7w65eU6eXA@6S&(drh&x!nwL* zQLa_gXFye%sfg~n@tqJtq06x9>YO^GRWH{FL`A4cx|wLBMTX-jNz^baig<&kQ|-`` z5;d-dKpD*mYdMJwb6t*d)8(o?d3-(fhzBao=+pGr)I&3;P7LI&iVIUP@+{!b>UR_ml;oO`h z9qmMM5{b0>ajtu1GFBtZD>uSg-Ol=}U6+W}Rs0$y*r$kO)yx9b%TThj!eKnrNu28N zVZE7S-L!a${&ho{Lg;@zDexmoip-Guy3Q`wnolabUX}Gxda&nb&{EMxQRY#vG(!`% z>WLta7%_D^N=AAu&$GNeXdNdVO~%BIqiSC@YS6hfijtJ6tkZdNUdBC2?LeR_#)#vh z<6E(#Ige2VdoncnuvLsU?Btsbm@Owoyo=3;ZT8&yI4z?qmfe=<|DMzMhL}>tqNsCR z__l=M#hKRN)~MZXl@#fEmQk0mW2OiZzEjYAEOUb;8s(eCyqzZWm7X4!>S$T!@y4jM z0!d1gs)mEUOLoCvdv9at@#k^+{g;u7QNFOQSV~N zq`e)LEOxy%Yn89#zbETnj`EaAu)SSu?k>nf_1eQ~zf$*~cyMi19Vk{06#LZy)i$}6 z;UG@5DC#0hxGcS5ylHlAIwK6AyR_3XbL}=J&jMjt#nc~5_wV9V7@+;)Vmutm;xFP? z{Sh;ErnDGQVVHu$19l4SSi9Lp$nXa#ILJHZM|+_d7a>K z85OTTlghA5mwfqYo$^nOZW$GkJ{oH-V3Q@QJuODZ(5!~<>ZlOzWhiDk(z{-wpk^sa zWiEEcRku6cN5ijBlaFiyj3JB7$*Nx0pEAyx1md9|ZWTAyK4UTM**VK;JIzdVo%LYA z8ZV6&>SUJcx&`5DqRzsCnGXVaEFV)dW`P$w?di6>T|_iVg2^$WMHp@L!Z))4L$RD$ z+M3g3r>Vlt^U_?`mqwZ<9nsm&PiTmn2Y2lr7fqKBOMgX5fozcJt2MGw87FZ$7OR&P zQp|$QZkCtow0*)7BP)HDPuH7TVttkAWnhmAGRaIdQB56t6tyZUqOGYLmM2z`#R&@x z9NVsAoCR$+s$HgkP|a)p0ha6kWPD`~^*;xrv}d;P{=vk6BS^biv~os>?MkPwheeAy zIv=H-4ULj(|0^1B7O+cNOEg@bS9`zsan29^sXuABLXLF5LNA@@vk8s|vCdc38`qC7 z9a`eZ1RX=#9fH(Dd3FJ@(A17jvLT8Qs&&o0$r@f}!=*%?s|QaUTRXYFreq#BT=aFx zey#{<^H??<=kexNsa6gy-?OxGaOI#{zs$m}PMWQEl&JL)gxMtzQAxD$7<@6Mw~wtK zTfb}V)C20#Q6BfU<^wtZzk@UFi1V+U|9yP^Z#m!p4R{|o3(f#J|KAPn1TO{O3jT}p z{BMEZ1U~`JgU7&8Am{$?051dI4!+3w{=?uy;39Z6_!rLfzX&?uZQvMqBiILC41Slh z{1f0R2*E#aj{ke`Q{X4TWv~NMa4&czcm?O5o2u?t$!7tBrKXwih=?B6Y-5 z!Jc_hmDa&)oWf?GYB$#As53|%c|+Tt*N)WWF6Ys(Xv^<;%ar_S{nGgSvSi0@%i-Zv z?vAjg1s=G)zHl)ZC5g~y?{8^^^6T94s@vh?x-`Ao?YmqFSGYzhJ>J`=SUWOn-{$kW zGz~p-A}HkSJ{qbaULgD;IHS~pJz6Z_#~Z7R9eA$x<76&7rsRI*-JWVI%Pv~?=+S=P zoC7#GjFI-V^Ye0yml_XK)Y~%qc_UjnG~g8BB%>D0;blSI*^VL_VgM%lX$mJ+) z&H+5}(`=ZN z_HdlSg>;TaDsCniHnPKCDMbeO0lgV*0oD8Soma#pG zarslpkUK@?ngKaU@}PFyR&-4W4q1rHj6szXuc2Xx;QT+1FOMim#ZXN`R7Mem+ro`$ z+J`UGig_{F9UG0CJa1cb^^RUYL!q}<+AU)hE%jQ3&t_BGdFk3w2rQXvrpQmN4#%tF za-A7H$FSV?u21+~uhCcm|K#|Z5KcsGqBO$Wk&TR`MVPf9+UI|ue-;t`gR%d`>D+i%F%2Fml!YP$HluVj z~k60SIVWzBsAuZFIrz}##XCObVl<55SC@EuPSKqG-h0*wS3 z2{aODB+y9U|G5Oj%OQT9+6QAZ`MyDE)cAW$ZrZHtVm5KK^6`1sGub?WIscQ#5Zppu zxSap}{r!(}zJC%t0V2=^4}c>;?gK19CSJHXq)3OE413)}?ufoE}K zTmomo8E^o6me~J$z!C5TavnYfejfZBI1ToJkCV&))8NO!y_CExk22X)YU@w@7?%_cAkns1*iK~-6Y}Ex|@x=%Db=%a=}ejDBdld3?}_uldengm|u{~ z2-Zt)8kDetWZ3j)d|A-%lLW7dt!$LI){Ud|YxWP?#`ks*il}xO)>KSKq$)ycn;bYI zI|~sP?P0e-lv&Cdto{5s#za7Ind+X=gfaapzknGyVG*TQslIrnv6Q z?&Qpxb<2;$#0&>Ez$9#F`cTVHp3GMA38JQA9cCkm(B}wC4WA@z8}cc_GGa3F-d}D; z(74Ghwb`k-&g0yrD`pZF{`{xT-mGhdB=_6=V_#~|@0Qr7P2MLRxHXe^TlY~a6U?b{ zlueRtOPVu@ym2ZnWFjGQb-+apNPg3On{y|%k&?0OblmQRK2^N!vmj5eJ-mM6EhlXp zvS*meQX4drn=OG*@SnZ9%Xm&F*}CTZz)(lVWhOP(4~|f##B@xO2UQNY6fh%J3)nB9 z@``Yk<5l!(HgB`NW4pB@-Ew(^#re8VyKWM+umfL?iz#mMa!LKJWB$F>SRYkUzIboq zJ`LOrl9=FAx@ltsl+uzmi={zrfuO*Gi(l2vVp*iU$AwIri;DC@t_#`FlW#eYo-1Mr zM`n{G+pU#w`H9?3>#1+l#z*HH-Hs=XXG9YS^C zx`>U4rE(~Qt<%e4czUJVT~+H&goH6w9;KUFdg}>#kx*08NYPBfpHct{Rw?~bV`y)T z{mnE**Is9qkS+2mb#5bh-R|MTRlVuaaDiIKBh{28kI2^HxpkwnHM`mBB5r`dC) zRz!AJ-}2CtRWBsY%vCM-7}~SJr2A~U_qq1(l`sK<+Rpz4dMk8M=;)Mb)GQZ>_T(W+|mGB?6i0d?}y!++OeoV^Y|q>ps{! zacb~`SiRh}R#i9Cs*7tjJHB3OgbT*V+)I#je?R_Qyz|RORoB)sGxG!&ZfX`D{lv&H zS=(U)!NoSYq~&=%g-p2MMXmu;6x&vF3rO-Ww#;o#q)Fbhlbu+e{B723r&UHD{46q) zgwoGQ0WdeHD#|tpYX(iJ6KEb*j417mSWz>*ZHU3K#Oqh>;&(kYlYaVLvf7TuGDT>v z&=UVAS;O64kZ^ItTeDu928)e_bbZ_tlUK^Yd zE~e;j4ayigrDult8qd;I2H1UzpuHgVX01d9-0sH$8L#Kn%j*sJ MS1IkU*DcNe0iPA|bN~PV literal 0 HcmV?d00001 diff --git a/reframe/core/attributes.py b/reframe/core/attributes.py new file mode 100644 index 0000000000..689bc462be --- /dev/null +++ b/reframe/core/attributes.py @@ -0,0 +1,161 @@ +# Copyright 2016-2020 Swiss National Supercomputing Centre (CSCS/ETH Zurich) +# ReFrame Project Developers. See the top-level LICENSE file for details. +# +# SPDX-License-Identifier: BSD-3-Clause + +# +# attribute extension to the RegressionTest classes. +# + +import copy + +class InputParameter: + ''' + Input paramter and (name + values) and other attributes to deal with the + test parameter inheritance/chaining. + + TODO: Type checking. + ''' + def __init__(self, name, values=None, inherit_params=False, filt_params=None): + ''' + name: parameter name + values: parameter values + inherit_params: If false, it overrides all previous values set for the parameter. + filt_params: Function to filter/modify the inherited values for the parameter. + It only has an effect if used with inherit_params=True. + ''' + # If the values are None (or an empty list), the parameter is considered as + # declared but not defined (i.e. an abstract parameter). + if values is None: + values = [] + + # The values arg must be a list. + if not isinstance(values, list): + raise ValueError(f'Parameter values must be defined in a list.') from None + + # Default filter is no filter. + if filt_params is None: + filt_params = lambda x: x + + self.name = name + self.values = values + self.inherit_params = inherit_params + self.filt_params = filt_params + + def is_undef(self): + ''' + Handy function to check if the parameter is undefined (empty). + ''' + if self.values == []: + return True + + return False + + +class ParameterPack: + ''' + Bundle containing the test parameters inserted in each test (class). + The parameters are store in a dictionary for a more efficient lookup. + The add method is the interface used to add new parameters to the reframe test. + ''' + def __init__(self): + self.parameter_map = {} + + def add(self, name, defaults=None, inherit_params=False, filt_params=None): + ''' + Insert a new parameter in the dictionary. + If the parameter is already present in it, raise an error. + ''' + if name not in self.parameter_map: + self.parameter_map[name] = InputParameter(name, defaults, inherit_params, filt_params) + else: + raise ValueError('Cannot double-define a parameter in the same class.') + + +class RegressionTestAttributes: + ''' + Storage class hosting all the reframe class attributes. + ''' + def __init__(self): + self._rfm_parameter_stage = ParameterPack() + + def get_parameter_stage(self): + return self._rfm_parameter_stage.parameter_map + + def _inherit_parameter_space(self, bases): + ''' + Build the parameter space from the base clases. + ''' + # Temporary dict where we build the parameter space from the base clases + temp_parameter_space = {} + + # Iterate over the base classes and inherit the parameter space + for b in bases: + if hasattr(b, '_rfm_params'): + base_params = copy.deepcopy(b._rfm_params) + for key in base_params: + if key in self.get_parameter_stage() and not self.get_parameter_stage().get(key).inherit_params: + + # Do not inherit a given parameter if the current class wants to override it. + pass + + else: + + # With multiple inheritance, a single parameter could be doubly defined + # and lead to repeated values. + # TODO: add type checking here too. + if key in temp_parameter_space: + if not (temp_parameter_space[key] == [] or base_params[key] == []): + raise KeyError(f'Parameter space conflict (on {key}) ' + f'due to multiple inheritance.') from None + + temp_parameter_space[key] = base_params.get(key, []) + temp_parameter_space.get(key, []) + + else: + # The base class does not have the attribute cls._rfm_params + pass + + return temp_parameter_space + + def _extend_parameter_space(self, parameter_space): + ''' + Add the parameters from the parameter stage into the existing parameter space. + Do the inherit+filter operations as defined in the for each input parameter in the + parameter stage. + ''' + # Loop over the parameter stage. Each element is an instance of InputParameter. + for name, p in self.get_parameter_stage().items(): + parameter_space[name] = p.filt_params(parameter_space.get(name, [])) + p.values if ( + p.inherit_params) else p.values + + def build_parameter_space(self, bases): + ''' + Compiles the full test parameter space by joining the parameter spaces from the base clases, + and extending that with the parameters present in the parameter stage. + ''' + # Inherit from the bases + param_space = self._inherit_parameter_space(bases) + + # Extend with what was added in the current class + self._extend_parameter_space(param_space) + + return param_space + + @staticmethod + def check_namespace_clashing(dict_a, dict_b, name=None): + ''' + Check that these two dictionaries do not have any overlapping keys. + ''' + if len(dict_a) > len(dict_b): + long_dict = dict_a + short_dict = dict_b + else: + long_dict = dict_b + short_dict = dict_a + + name = '__unknown__' if name is None else name + for key in short_dict: + if key in long_dict: + raise AttributeError(f'Attribute {key} clashes with other variables' + f'present in the namespace of class {name}') + diff --git a/reframe/core/decorators.py b/reframe/core/decorators.py index 1ca60588fd..115908586b 100644 --- a/reframe/core/decorators.py +++ b/reframe/core/decorators.py @@ -8,7 +8,7 @@ # __all__ = [ - 'parameterized_test', 'simple_test', 'required_version', + 'test', 'parameterized_test', 'simple_test', 'required_version', 'require_deps', 'run_before', 'run_after' ] @@ -18,6 +18,7 @@ import inspect import sys import traceback +import itertools import reframe from reframe.core.exceptions import ReframeSyntaxError, user_frame @@ -72,6 +73,47 @@ def _validate_test(cls): raise ReframeSyntaxError('the decorated class must be a ' 'subclass of RegressionTest') + if (cls.is_abstract_test()): + raise ValueError(f'Decodated test ({cls.__qualname__}) is an' + f' abstract test.') + + + +def test(cls): + '''Class decorator for registering tests with ReFrame. + + The decorated class must derive from + :class:`reframe.core.pipeline.RegressionTest`. This decorator is also + available directly under the :mod:`reframe` module. + + .. versionadded:: #.## + ''' + def _do_register(cls): + _validate_test(cls) + + # If cls is not a parametrised test, register as is and return + if not cls._rfm_params: + _register_test(cls) + return cls + + # We create a single test for all possible combinations in the parameter space. + # The different combinations are added to an expanded parameter set. + _rfm_expanded_param_space = [] + for inst in itertools.product(*[cls._rfm_params.get(k) for k in cls._rfm_params]): + + param_set = {} + for i, parameter in enumerate(cls._rfm_params): + param_set[parameter] = inst[i] + + _rfm_expanded_param_space.append(param_set) + _register_test(cls) + + cls._rfm_expanded_param_space = iter(itertools.cycle(_rfm_expanded_param_space)) + + return cls + + return _do_register(cls) + def simple_test(cls): '''Class decorator for registering parameterless tests with ReFrame. @@ -81,11 +123,11 @@ def simple_test(cls): available directly under the :mod:`reframe` module. .. versionadded:: 2.13 + + .. to be deprecated ''' - _validate_test(cls) - _register_test(cls) - return cls + return test(cls) def parameterized_test(*inst): @@ -103,6 +145,8 @@ def parameterized_test(*inst): .. note:: This decorator does not instantiate any test. It only registers them. The actual instantiation happens during the loading phase of the test. + + .. to be deprecated ''' def _do_register(cls): _validate_test(cls) diff --git a/reframe/core/meta.py b/reframe/core/meta.py index 538b2393be..a69fec7bf7 100644 --- a/reframe/core/meta.py +++ b/reframe/core/meta.py @@ -8,12 +8,33 @@ # from reframe.core.warnings import user_deprecation_warning - +from reframe.core.attributes import RegressionTestAttributes class RegressionTestMeta(type): + @classmethod + def __prepare__(cls, name, bases, **kwargs): + namespace = super().__prepare__(name, bases, **kwargs) + + # Extend the class attributes to the RegressionTest class + namespace['_rfm_attributes'] = RegressionTestAttributes() + + # Attribute to add a regression test parameter as: + # `rfm_parameter('P0', [0,1,2,3])`. + namespace['rfm_parameter'] = namespace['_rfm_attributes']._rfm_parameter_stage.add + + return namespace + def __init__(cls, name, bases, namespace, **kwargs): super().__init__(name, bases, namespace, **kwargs) + # Set up the regression test parameter space + cls._rfm_params = cls._rfm_attributes.build_parameter_space(bases) + + # Make illegal to have a parameter clashing with any of the RegressionTest + # class variables + cls._rfm_attributes.check_namespace_clashing(cls.__dict__, cls._rfm_params, + cls.__qualname__) + # Set up the hooks for the pipeline stages based on the _rfm_attach # attribute; all dependencies will be resolved first in the post-setup # phase if not assigned elsewhere diff --git a/reframe/core/pipeline.py b/reframe/core/pipeline.py index 7addd3e9d9..97e88e7952 100644 --- a/reframe/core/pipeline.py +++ b/reframe/core/pipeline.py @@ -710,9 +710,19 @@ def disable_hook(cls, hook_name): def __new__(cls, *args, **kwargs): obj = super().__new__(cls) + # Abort the class instantiation if the test is an abstract test. + if cls.is_abstract_test(): + raise ValueError('Cannot instantiate an abstract test.') from None + + # Set the test parameters in the object + cls._set_parameter_space(obj) + # Create a test name from the class name and the constructor's # arguments name = cls.__qualname__ + name += obj._append_parameters_to_name() + + # or alternatively, if the parameterized test was defined the old way. if args or kwargs: arg_names = map(lambda x: util.toalphanum(str(x)), itertools.chain(args, kwargs.values())) @@ -737,6 +747,36 @@ def __new__(cls, *args, **kwargs): def __init__(self): pass + @classmethod + def _set_parameter_space(cls, obj): + ''' + Pick the parameter sets from the queue cls._rfm_param_queue and loop over the dict + inserting the parameters into obj. + ''' + + # Don't do anything if the test is not a parametrised test + if not hasattr(cls, '_rfm_expanded_param_space'): + return + + # Set the parameter values for the test case + for key, value in next(cls._rfm_expanded_param_space).items(): + obj.__dict__[key] = value + + @classmethod + def is_abstract_test(cls): + ''' Checks if the test is an abstract test ''' + for key in cls._rfm_params: + if (cls._rfm_params[key] == []): + return True + + return False + + def _append_parameters_to_name(self): + if self._rfm_params: + return '_' + '_'.join([str(self.__dict__[key]) for key in self._rfm_params]) + else: + return '' + @classmethod def __init_subclass__(cls, *, special=False, base_test=False, **kwargs): super().__init_subclass__(**kwargs) From f2be8c60405d90bf169f58d440ed8e4214461a21 Mon Sep 17 00:00:00 2001 From: Javier Otero Date: Thu, 5 Nov 2020 15:05:10 +0100 Subject: [PATCH 02/43] Cleanup parameterized test implementation --- .gitignore | 3 +++ reframe/core/.decorators.py.swp | Bin 24576 -> 0 bytes reframe/core/decorators.py | 18 ++++++------------ reframe/core/pipeline.py | 29 ++++++++++++++++++++++------- 4 files changed, 31 insertions(+), 19 deletions(-) delete mode 100644 reframe/core/.decorators.py.swp diff --git a/.gitignore b/.gitignore index 2b01f11282..4eaed2111c 100644 --- a/.gitignore +++ b/.gitignore @@ -96,3 +96,6 @@ ENV/ # TextMATE files ._* + +# Vim temp files +*.swp diff --git a/reframe/core/.decorators.py.swp b/reframe/core/.decorators.py.swp deleted file mode 100644 index 0f3c1cde587bc5af5ffeed404ff0c52f0168cbf2..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 24576 zcmeI3Ym6kxa3L#op2?f*e)`$=^1z!AJF*MmIT*41|}%k^D-%~w=rc@YlB&rhr= z(MX_?zzdYXt-+y{Wn=1d`(LB(eCbKqG-h0{@pK zP!4Ylevl5l$aTV>_P@#f_TM+U-#5GO;mrGQcE1bveP!nT4es|n?)&pI@7>SltC2t> zfkpz21R4o65@;mQNT88GBY{Q&jRYDAG!pnF8?_kuQPftQ2N+!_SG3dZ0lI0D`P zUIqU1B|-4};FrNRI0+7cSAti7FWwRa&w*#buYfUl44eb62e*Km!GGYW_(SkPP=E#S zuQyQ#yd4||$G|~w7q|`l3mheX4t^Va1pEwm0$c@mgKq<0z}fQG;IF__;CsLw;G;NE zejU6IoChIT0{_m~eFl6Q$T&U&ehK_Acqe!oWZ=8O0r20>siXLu+S1BpUg*B+B~ekR z?NL$kFpqb%8syn_5J<^Zt98u0t`arWAj?&*H{*gdkvQE{!${IP_>pMHl)5MjbvZ7# z)WiCIc@qYGJy1owJxp|`6r%F!B}LfD2iqNeWf-M>-R~GWonjdEv^uO}2)5a^c>%;<84uvD(zovXTjUN}yR z@?3mAkRtvgvWgdL5m89#bhNF>dC|g_m4TsC%&!K%o_tr$K>O;Gb^i=a+7eo|fQEV2 zAN9)i!nql$;ifLz7w65eU6eXA@6S&(drh&x!nwL* zQLa_gXFye%sfg~n@tqJtq06x9>YO^GRWH{FL`A4cx|wLBMTX-jNz^baig<&kQ|-`` z5;d-dKpD*mYdMJwb6t*d)8(o?d3-(fhzBao=+pGr)I&3;P7LI&iVIUP@+{!b>UR_ml;oO`h z9qmMM5{b0>ajtu1GFBtZD>uSg-Ol=}U6+W}Rs0$y*r$kO)yx9b%TThj!eKnrNu28N zVZE7S-L!a${&ho{Lg;@zDexmoip-Guy3Q`wnolabUX}Gxda&nb&{EMxQRY#vG(!`% z>WLta7%_D^N=AAu&$GNeXdNdVO~%BIqiSC@YS6hfijtJ6tkZdNUdBC2?LeR_#)#vh z<6E(#Ige2VdoncnuvLsU?Btsbm@Owoyo=3;ZT8&yI4z?qmfe=<|DMzMhL}>tqNsCR z__l=M#hKRN)~MZXl@#fEmQk0mW2OiZzEjYAEOUb;8s(eCyqzZWm7X4!>S$T!@y4jM z0!d1gs)mEUOLoCvdv9at@#k^+{g;u7QNFOQSV~N zq`e)LEOxy%Yn89#zbETnj`EaAu)SSu?k>nf_1eQ~zf$*~cyMi19Vk{06#LZy)i$}6 z;UG@5DC#0hxGcS5ylHlAIwK6AyR_3XbL}=J&jMjt#nc~5_wV9V7@+;)Vmutm;xFP? z{Sh;ErnDGQVVHu$19l4SSi9Lp$nXa#ILJHZM|+_d7a>K z85OTTlghA5mwfqYo$^nOZW$GkJ{oH-V3Q@QJuODZ(5!~<>ZlOzWhiDk(z{-wpk^sa zWiEEcRku6cN5ijBlaFiyj3JB7$*Nx0pEAyx1md9|ZWTAyK4UTM**VK;JIzdVo%LYA z8ZV6&>SUJcx&`5DqRzsCnGXVaEFV)dW`P$w?di6>T|_iVg2^$WMHp@L!Z))4L$RD$ z+M3g3r>Vlt^U_?`mqwZ<9nsm&PiTmn2Y2lr7fqKBOMgX5fozcJt2MGw87FZ$7OR&P zQp|$QZkCtow0*)7BP)HDPuH7TVttkAWnhmAGRaIdQB56t6tyZUqOGYLmM2z`#R&@x z9NVsAoCR$+s$HgkP|a)p0ha6kWPD`~^*;xrv}d;P{=vk6BS^biv~os>?MkPwheeAy zIv=H-4ULj(|0^1B7O+cNOEg@bS9`zsan29^sXuABLXLF5LNA@@vk8s|vCdc38`qC7 z9a`eZ1RX=#9fH(Dd3FJ@(A17jvLT8Qs&&o0$r@f}!=*%?s|QaUTRXYFreq#BT=aFx zey#{<^H??<=kexNsa6gy-?OxGaOI#{zs$m}PMWQEl&JL)gxMtzQAxD$7<@6Mw~wtK zTfb}V)C20#Q6BfU<^wtZzk@UFi1V+U|9yP^Z#m!p4R{|o3(f#J|KAPn1TO{O3jT}p z{BMEZ1U~`JgU7&8Am{$?051dI4!+3w{=?uy;39Z6_!rLfzX&?uZQvMqBiILC41Slh z{1f0R2*E#aj{ke`Q{X4TWv~NMa4&czcm?O5o2u?t$!7tBrKXwih=?B6Y-5 z!Jc_hmDa&)oWf?GYB$#As53|%c|+Tt*N)WWF6Ys(Xv^<;%ar_S{nGgSvSi0@%i-Zv z?vAjg1s=G)zHl)ZC5g~y?{8^^^6T94s@vh?x-`Ao?YmqFSGYzhJ>J`=SUWOn-{$kW zGz~p-A}HkSJ{qbaULgD;IHS~pJz6Z_#~Z7R9eA$x<76&7rsRI*-JWVI%Pv~?=+S=P zoC7#GjFI-V^Ye0yml_XK)Y~%qc_UjnG~g8BB%>D0;blSI*^VL_VgM%lX$mJ+) z&H+5}(`=ZN z_HdlSg>;TaDsCniHnPKCDMbeO0lgV*0oD8Soma#pG zarslpkUK@?ngKaU@}PFyR&-4W4q1rHj6szXuc2Xx;QT+1FOMim#ZXN`R7Mem+ro`$ z+J`UGig_{F9UG0CJa1cb^^RUYL!q}<+AU)hE%jQ3&t_BGdFk3w2rQXvrpQmN4#%tF za-A7H$FSV?u21+~uhCcm|K#|Z5KcsGqBO$Wk&TR`MVPf9+UI|ue-;t`gR%d`>D+i%F%2Fml!YP$HluVj z~k60SIVWzBsAuZFIrz}##XCObVl<55SC@EuPSKqG-h0*wS3 z2{aODB+y9U|G5Oj%OQT9+6QAZ`MyDE)cAW$ZrZHtVm5KK^6`1sGub?WIscQ#5Zppu zxSap}{r!(}zJC%t0V2=^4}c>;?gK19CSJHXq)3OE413)}?ufoE}K zTmomo8E^o6me~J$z!C5TavnYfejfZBI1ToJkCV&))8NO!y_CExk22X)YU@w@7?%_cAkns1*iK~-6Y}Ex|@x=%Db=%a=}ejDBdld3?}_uldengm|u{~ z2-Zt)8kDetWZ3j)d|A-%lLW7dt!$LI){Ud|YxWP?#`ks*il}xO)>KSKq$)ycn;bYI zI|~sP?P0e-lv&Cdto{5s#za7Ind+X=gfaapzknGyVG*TQslIrnv6Q z?&Qpxb<2;$#0&>Ez$9#F`cTVHp3GMA38JQA9cCkm(B}wC4WA@z8}cc_GGa3F-d}D; z(74Ghwb`k-&g0yrD`pZF{`{xT-mGhdB=_6=V_#~|@0Qr7P2MLRxHXe^TlY~a6U?b{ zlueRtOPVu@ym2ZnWFjGQb-+apNPg3On{y|%k&?0OblmQRK2^N!vmj5eJ-mM6EhlXp zvS*meQX4drn=OG*@SnZ9%Xm&F*}CTZz)(lVWhOP(4~|f##B@xO2UQNY6fh%J3)nB9 z@``Yk<5l!(HgB`NW4pB@-Ew(^#re8VyKWM+umfL?iz#mMa!LKJWB$F>SRYkUzIboq zJ`LOrl9=FAx@ltsl+uzmi={zrfuO*Gi(l2vVp*iU$AwIri;DC@t_#`FlW#eYo-1Mr zM`n{G+pU#w`H9?3>#1+l#z*HH-Hs=XXG9YS^C zx`>U4rE(~Qt<%e4czUJVT~+H&goH6w9;KUFdg}>#kx*08NYPBfpHct{Rw?~bV`y)T z{mnE**Is9qkS+2mb#5bh-R|MTRlVuaaDiIKBh{28kI2^HxpkwnHM`mBB5r`dC) zRz!AJ-}2CtRWBsY%vCM-7}~SJr2A~U_qq1(l`sK<+Rpz4dMk8M=;)Mb)GQZ>_T(W+|mGB?6i0d?}y!++OeoV^Y|q>ps{! zacb~`SiRh}R#i9Cs*7tjJHB3OgbT*V+)I#je?R_Qyz|RORoB)sGxG!&ZfX`D{lv&H zS=(U)!NoSYq~&=%g-p2MMXmu;6x&vF3rO-Ww#;o#q)Fbhlbu+e{B723r&UHD{46q) zgwoGQ0WdeHD#|tpYX(iJ6KEb*j417mSWz>*ZHU3K#Oqh>;&(kYlYaVLvf7TuGDT>v z&=UVAS;O64kZ^ItTeDu928)e_bbZ_tlUK^Yd zE~e;j4ayigrDult8qd;I2H1UzpuHgVX01d9-0sH$8L#Kn%j*sJ MS1IkU*DcNe0iPA|bN~PV diff --git a/reframe/core/decorators.py b/reframe/core/decorators.py index 115908586b..b74f103dde 100644 --- a/reframe/core/decorators.py +++ b/reframe/core/decorators.py @@ -74,7 +74,7 @@ def _validate_test(cls): 'subclass of RegressionTest') if (cls.is_abstract_test()): - raise ValueError(f'Decodated test ({cls.__qualname__}) is an' + raise ValueError(f'Decorated test ({cls.__qualname__}) is an' f' abstract test.') @@ -96,20 +96,14 @@ def _do_register(cls): _register_test(cls) return cls - # We create a single test for all possible combinations in the parameter space. - # The different combinations are added to an expanded parameter set. - _rfm_expanded_param_space = [] - for inst in itertools.product(*[cls._rfm_params.get(k) for k in cls._rfm_params]): + # Create iterator to cycle through all possible combinations in the parameter space. + def _param_space_iterator(cls): + return itertools.product(*[cls._rfm_params.get(k) for k in cls._rfm_params]) - param_set = {} - for i, parameter in enumerate(cls._rfm_params): - param_set[parameter] = inst[i] - - _rfm_expanded_param_space.append(param_set) + cls._rfm_param_space_iter = _param_space_iterator(cls) + for inst in _param_space_iterator(cls): _register_test(cls) - cls._rfm_expanded_param_space = iter(itertools.cycle(_rfm_expanded_param_space)) - return cls return _do_register(cls) diff --git a/reframe/core/pipeline.py b/reframe/core/pipeline.py index 97e88e7952..1364369bb1 100644 --- a/reframe/core/pipeline.py +++ b/reframe/core/pipeline.py @@ -750,17 +750,32 @@ def __init__(self): @classmethod def _set_parameter_space(cls, obj): ''' - Pick the parameter sets from the queue cls._rfm_param_queue and loop over the dict - inserting the parameters into obj. + Adds to obj.__dict__ the keys corresponding to the test parameters. ''' - # Don't do anything if the test is not a parametrised test - if not hasattr(cls, '_rfm_expanded_param_space'): + if not cls._rfm_params: return - # Set the parameter values for the test case - for key, value in next(cls._rfm_expanded_param_space).items(): - obj.__dict__[key] = value + # Test if the we have the parameter space iterator and set + # the values of the test parameters. + if hasattr(cls, '_rfm_param_space_iter'): + try: + tmp = next(cls._rfm_param_space_iter) + for index, key in enumerate(cls._rfm_params): + obj.__dict__[key] = tmp[index] + + return + + # Delete the iterator if we have exhausted the parameter + # space + except StopIteration: + del cls._rfm_param_space_iter + + # If the param space is not present anymore, it's because we + # have already instantiated the full parameter space. However + # this will get called if the instance is copied. + for key in cls._rfm_params: + obj.__dict__[key] = None @classmethod def is_abstract_test(cls): From 4231f962ca8377e6c9d00ff76d1ea1847b4b76a5 Mon Sep 17 00:00:00 2001 From: Javier Otero Date: Thu, 5 Nov 2020 15:10:41 +0100 Subject: [PATCH 03/43] Fix PEP8. --- reframe/core/attributes.py | 19 +++++++++++++------ reframe/core/decorators.py | 1 - reframe/core/meta.py | 1 + 3 files changed, 14 insertions(+), 7 deletions(-) diff --git a/reframe/core/attributes.py b/reframe/core/attributes.py index 689bc462be..0c3dd5a331 100644 --- a/reframe/core/attributes.py +++ b/reframe/core/attributes.py @@ -9,6 +9,7 @@ import copy + class InputParameter: ''' Input paramter and (name + values) and other attributes to deal with the @@ -16,6 +17,7 @@ class InputParameter: TODO: Type checking. ''' + def __init__(self, name, values=None, inherit_params=False, filt_params=None): ''' name: parameter name @@ -31,11 +33,12 @@ def __init__(self, name, values=None, inherit_params=False, filt_params=None): # The values arg must be a list. if not isinstance(values, list): - raise ValueError(f'Parameter values must be defined in a list.') from None + raise ValueError( + f'Parameter values must be defined in a list.') from None # Default filter is no filter. if filt_params is None: - filt_params = lambda x: x + def filt_params(x): return x self.name = name self.values = values @@ -58,6 +61,7 @@ class ParameterPack: The parameters are store in a dictionary for a more efficient lookup. The add method is the interface used to add new parameters to the reframe test. ''' + def __init__(self): self.parameter_map = {} @@ -67,15 +71,18 @@ def add(self, name, defaults=None, inherit_params=False, filt_params=None): If the parameter is already present in it, raise an error. ''' if name not in self.parameter_map: - self.parameter_map[name] = InputParameter(name, defaults, inherit_params, filt_params) + self.parameter_map[name] = InputParameter( + name, defaults, inherit_params, filt_params) else: - raise ValueError('Cannot double-define a parameter in the same class.') + raise ValueError( + 'Cannot double-define a parameter in the same class.') class RegressionTestAttributes: ''' Storage class hosting all the reframe class attributes. ''' + def __init__(self): self._rfm_parameter_stage = ParameterPack() @@ -109,7 +116,8 @@ def _inherit_parameter_space(self, bases): raise KeyError(f'Parameter space conflict (on {key}) ' f'due to multiple inheritance.') from None - temp_parameter_space[key] = base_params.get(key, []) + temp_parameter_space.get(key, []) + temp_parameter_space[key] = base_params.get( + key, []) + temp_parameter_space.get(key, []) else: # The base class does not have the attribute cls._rfm_params @@ -158,4 +166,3 @@ def check_namespace_clashing(dict_a, dict_b, name=None): if key in long_dict: raise AttributeError(f'Attribute {key} clashes with other variables' f'present in the namespace of class {name}') - diff --git a/reframe/core/decorators.py b/reframe/core/decorators.py index b74f103dde..b9f4e364a4 100644 --- a/reframe/core/decorators.py +++ b/reframe/core/decorators.py @@ -78,7 +78,6 @@ def _validate_test(cls): f' abstract test.') - def test(cls): '''Class decorator for registering tests with ReFrame. diff --git a/reframe/core/meta.py b/reframe/core/meta.py index a69fec7bf7..74460339a7 100644 --- a/reframe/core/meta.py +++ b/reframe/core/meta.py @@ -10,6 +10,7 @@ from reframe.core.warnings import user_deprecation_warning from reframe.core.attributes import RegressionTestAttributes + class RegressionTestMeta(type): @classmethod def __prepare__(cls, name, bases, **kwargs): From 408c3bf162bb04d07e27c655720b1b354fd85be8 Mon Sep 17 00:00:00 2001 From: Javier Otero Date: Thu, 5 Nov 2020 15:41:15 +0100 Subject: [PATCH 04/43] Remove unnecessary copy. --- reframe/core/attributes.py | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/reframe/core/attributes.py b/reframe/core/attributes.py index 0c3dd5a331..bc55a95e0e 100644 --- a/reframe/core/attributes.py +++ b/reframe/core/attributes.py @@ -7,15 +7,11 @@ # attribute extension to the RegressionTest classes. # -import copy - class InputParameter: ''' Input paramter and (name + values) and other attributes to deal with the test parameter inheritance/chaining. - - TODO: Type checking. ''' def __init__(self, name, values=None, inherit_params=False, filt_params=None): @@ -99,7 +95,7 @@ def _inherit_parameter_space(self, bases): # Iterate over the base classes and inherit the parameter space for b in bases: if hasattr(b, '_rfm_params'): - base_params = copy.deepcopy(b._rfm_params) + base_params = b._rfm_params for key in base_params: if key in self.get_parameter_stage() and not self.get_parameter_stage().get(key).inherit_params: @@ -110,7 +106,6 @@ def _inherit_parameter_space(self, bases): # With multiple inheritance, a single parameter could be doubly defined # and lead to repeated values. - # TODO: add type checking here too. if key in temp_parameter_space: if not (temp_parameter_space[key] == [] or base_params[key] == []): raise KeyError(f'Parameter space conflict (on {key}) ' From 574eba557cf69f209b554d852f8750e11fa3cd3e Mon Sep 17 00:00:00 2001 From: Javier Otero Date: Thu, 5 Nov 2020 17:21:43 +0100 Subject: [PATCH 05/43] Add attributest to purge the test parameter space --- reframe/core/attributes.py | 76 +++++++++++++++++++++++++------------- reframe/core/meta.py | 22 ++++++++--- 2 files changed, 68 insertions(+), 30 deletions(-) diff --git a/reframe/core/attributes.py b/reframe/core/attributes.py index bc55a95e0e..e8e4bdfb2d 100644 --- a/reframe/core/attributes.py +++ b/reframe/core/attributes.py @@ -60,6 +60,8 @@ class ParameterPack: def __init__(self): self.parameter_map = {} + self.purge_parameter_space = False + self.parameter_blacklist = set() def add(self, name, defaults=None, inherit_params=False, filt_params=None): ''' @@ -73,6 +75,20 @@ def add(self, name, defaults=None, inherit_params=False, filt_params=None): raise ValueError( 'Cannot double-define a parameter in the same class.') + def purge_all_parameters(self): + ''' + Set the purge_parameter_space flag to true, which blocks the inheritance + of the full parameter space. + ''' + self.purge_parameter_space = True + + def purge_parameters(self, params_to_purge=None): + ''' + Override the inheritance of a given set of parameters. + ''' + for param in params_to_purge: + self.parameter_blacklist.add(param) + class RegressionTestAttributes: ''' @@ -85,6 +101,12 @@ def __init__(self): def get_parameter_stage(self): return self._rfm_parameter_stage.parameter_map + def _purge_all_parameters(self): + return self._rfm_parameter_stage.purge_parameter_space + + def _parameter_blacklist(self): + return self._rfm_parameter_stage.parameter_blacklist + def _inherit_parameter_space(self, bases): ''' Build the parameter space from the base clases. @@ -93,30 +115,34 @@ def _inherit_parameter_space(self, bases): temp_parameter_space = {} # Iterate over the base classes and inherit the parameter space - for b in bases: - if hasattr(b, '_rfm_params'): - base_params = b._rfm_params - for key in base_params: - if key in self.get_parameter_stage() and not self.get_parameter_stage().get(key).inherit_params: - - # Do not inherit a given parameter if the current class wants to override it. - pass - - else: - - # With multiple inheritance, a single parameter could be doubly defined - # and lead to repeated values. - if key in temp_parameter_space: - if not (temp_parameter_space[key] == [] or base_params[key] == []): - raise KeyError(f'Parameter space conflict (on {key}) ' - f'due to multiple inheritance.') from None - - temp_parameter_space[key] = base_params.get( - key, []) + temp_parameter_space.get(key, []) - - else: - # The base class does not have the attribute cls._rfm_params - pass + if not self._purge_all_parameters(): + for b in bases: + if hasattr(b, '_rfm_params'): + base_params = b._rfm_params + for key in base_params: + if ((key in self.get_parameter_stage() and + not self.get_parameter_stage().get(key).inherit_params) or + key in self._parameter_blacklist()): + + # Do not inherit a given parameter if the current class wants to override it. + pass + + else: + + # With multiple inheritance, a single parameter could be doubly defined + # and lead to repeated values. + if key in temp_parameter_space: + if not (temp_parameter_space[key] == [] or + base_params[key] == []): + raise KeyError(f'Parameter space conflict (on {key}) ' + f'due to multiple inheritance.') from None + + temp_parameter_space[key] = base_params.get( + key, []) + temp_parameter_space.get(key, []) + + else: + # The base class does not have the attribute cls._rfm_params + pass return temp_parameter_space @@ -145,7 +171,7 @@ def build_parameter_space(self, bases): return param_space @staticmethod - def check_namespace_clashing(dict_a, dict_b, name=None): + def namespace_clash_check(dict_a, dict_b, name=None): ''' Check that these two dictionaries do not have any overlapping keys. ''' diff --git a/reframe/core/meta.py b/reframe/core/meta.py index 74460339a7..dfeceda88a 100644 --- a/reframe/core/meta.py +++ b/reframe/core/meta.py @@ -17,11 +17,23 @@ def __prepare__(cls, name, bases, **kwargs): namespace = super().__prepare__(name, bases, **kwargs) # Extend the class attributes to the RegressionTest class - namespace['_rfm_attributes'] = RegressionTestAttributes() + namespace['__rfm_attributes'] = RegressionTestAttributes() # Attribute to add a regression test parameter as: # `rfm_parameter('P0', [0,1,2,3])`. - namespace['rfm_parameter'] = namespace['_rfm_attributes']._rfm_parameter_stage.add + namespace['rfm_parameter'] = namespace['__rfm_attributes']._rfm_parameter_stage.add + + # Attribute to purge a list of test parameters. + namespace['rfm_purge_parameters'] = namespace['__rfm_attributes']._rfm_parameter_stage.purge_parameters + + # Attribute to purge the entire parameter space. + namespace['rfm_purge_all_parameters'] = namespace['__rfm_attributes']._rfm_parameter_stage.purge_all_parameters + + # Method to build the parameter space + namespace['_rfm_build_parameter_space'] = namespace['__rfm_attributes'].build_parameter_space + + # Method to check that the test parameter space does not clash with the RegressionTest namespace + namespace['_rfm_namespace_clash_check'] = namespace['__rfm_attributes'].namespace_clash_check return namespace @@ -29,12 +41,12 @@ def __init__(cls, name, bases, namespace, **kwargs): super().__init__(name, bases, namespace, **kwargs) # Set up the regression test parameter space - cls._rfm_params = cls._rfm_attributes.build_parameter_space(bases) + cls._rfm_params = cls._rfm_build_parameter_space(bases) # Make illegal to have a parameter clashing with any of the RegressionTest # class variables - cls._rfm_attributes.check_namespace_clashing(cls.__dict__, cls._rfm_params, - cls.__qualname__) + cls._rfm_namespace_clash_check(cls.__dict__, cls._rfm_params, + cls.__qualname__) # Set up the hooks for the pipeline stages based on the _rfm_attach # attribute; all dependencies will be resolved first in the post-setup From f329d83c89332ceb5331b4148dfb6d5f972f6966 Mon Sep 17 00:00:00 2001 From: Javier Otero Date: Thu, 5 Nov 2020 23:45:47 +0100 Subject: [PATCH 06/43] Make if syntax more compact. --- reframe/core/pipeline.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/reframe/core/pipeline.py b/reframe/core/pipeline.py index 1364369bb1..fe83e9ad05 100644 --- a/reframe/core/pipeline.py +++ b/reframe/core/pipeline.py @@ -780,8 +780,7 @@ def _set_parameter_space(cls, obj): @classmethod def is_abstract_test(cls): ''' Checks if the test is an abstract test ''' - for key in cls._rfm_params: - if (cls._rfm_params[key] == []): + if len(filter(lambda x: x==[], cls._rfm_params.values())) == 0: return True return False From ad2022caaa3a162ea1438b3b4fb5c861ffd77097 Mon Sep 17 00:00:00 2001 From: Javier Otero Date: Thu, 5 Nov 2020 23:50:24 +0100 Subject: [PATCH 07/43] Change if order. --- reframe/core/pipeline.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/reframe/core/pipeline.py b/reframe/core/pipeline.py index fe83e9ad05..91e998ff21 100644 --- a/reframe/core/pipeline.py +++ b/reframe/core/pipeline.py @@ -780,10 +780,10 @@ def _set_parameter_space(cls, obj): @classmethod def is_abstract_test(cls): ''' Checks if the test is an abstract test ''' - if len(filter(lambda x: x==[], cls._rfm_params.values())) == 0: - return True + if list(filter(lambda x: x==[], cls._rfm_params.values())) == []: + return False - return False + return True def _append_parameters_to_name(self): if self._rfm_params: From f9fb896ed898a520075e656d9835a98578739150 Mon Sep 17 00:00:00 2001 From: Javier Otero Date: Fri, 6 Nov 2020 11:41:03 +0100 Subject: [PATCH 08/43] Change function signature to purge test parameters. --- reframe/core/attributes.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/reframe/core/attributes.py b/reframe/core/attributes.py index e8e4bdfb2d..dcfd8cf2f4 100644 --- a/reframe/core/attributes.py +++ b/reframe/core/attributes.py @@ -63,14 +63,14 @@ def __init__(self): self.purge_parameter_space = False self.parameter_blacklist = set() - def add(self, name, defaults=None, inherit_params=False, filt_params=None): + def add(self, name, values=None, inherit_params=False, filt_params=None): ''' Insert a new parameter in the dictionary. If the parameter is already present in it, raise an error. ''' if name not in self.parameter_map: self.parameter_map[name] = InputParameter( - name, defaults, inherit_params, filt_params) + name, values, inherit_params, filt_params) else: raise ValueError( 'Cannot double-define a parameter in the same class.') @@ -82,7 +82,7 @@ def purge_all_parameters(self): ''' self.purge_parameter_space = True - def purge_parameters(self, params_to_purge=None): + def purge_parameters(self, *params_to_purge): ''' Override the inheritance of a given set of parameters. ''' From f3c6b7821bc008111a00fd6b943b3e7ffb1b8f5c Mon Sep 17 00:00:00 2001 From: Javier Otero Date: Mon, 23 Nov 2020 12:13:29 +0100 Subject: [PATCH 09/43] Fix PEP8. --- reframe/core/attributes.py | 15 ++++++++++----- reframe/core/decorators.py | 6 ++++-- reframe/core/pipeline.py | 7 ++++--- 3 files changed, 18 insertions(+), 10 deletions(-) diff --git a/reframe/core/attributes.py b/reframe/core/attributes.py index dcfd8cf2f4..56547f8d43 100644 --- a/reframe/core/attributes.py +++ b/reframe/core/attributes.py @@ -14,7 +14,8 @@ class InputParameter: test parameter inheritance/chaining. ''' - def __init__(self, name, values=None, inherit_params=False, filt_params=None): + def __init__(self, name, values=None, + inherit_params=False, filt_params=None): ''' name: parameter name values: parameter values @@ -111,7 +112,8 @@ def _inherit_parameter_space(self, bases): ''' Build the parameter space from the base clases. ''' - # Temporary dict where we build the parameter space from the base clases + # Temporary dict where we build the parameter space from the base + # clases temp_parameter_space = {} # Iterate over the base classes and inherit the parameter space @@ -124,7 +126,8 @@ def _inherit_parameter_space(self, bases): not self.get_parameter_stage().get(key).inherit_params) or key in self._parameter_blacklist()): - # Do not inherit a given parameter if the current class wants to override it. + # Do not inherit a given parameter if the current + # class wants to override it. pass else: @@ -141,7 +144,8 @@ def _inherit_parameter_space(self, bases): key, []) + temp_parameter_space.get(key, []) else: - # The base class does not have the attribute cls._rfm_params + # The base class does not have the attribute + # cls._rfm_params pass return temp_parameter_space @@ -152,7 +156,8 @@ def _extend_parameter_space(self, parameter_space): Do the inherit+filter operations as defined in the for each input parameter in the parameter stage. ''' - # Loop over the parameter stage. Each element is an instance of InputParameter. + # Loop over the parameter stage. Each element is an instance of + # InputParameter. for name, p in self.get_parameter_stage().items(): parameter_space[name] = p.filt_params(parameter_space.get(name, [])) + p.values if ( p.inherit_params) else p.values diff --git a/reframe/core/decorators.py b/reframe/core/decorators.py index b9f4e364a4..5318dafeec 100644 --- a/reframe/core/decorators.py +++ b/reframe/core/decorators.py @@ -95,9 +95,11 @@ def _do_register(cls): _register_test(cls) return cls - # Create iterator to cycle through all possible combinations in the parameter space. + # Create iterator to cycle through all possible combinations in the + # parameter space. def _param_space_iterator(cls): - return itertools.product(*[cls._rfm_params.get(k) for k in cls._rfm_params]) + return itertools.product(*[cls._rfm_params.get(k) + for k in cls._rfm_params]) cls._rfm_param_space_iter = _param_space_iterator(cls) for inst in _param_space_iterator(cls): diff --git a/reframe/core/pipeline.py b/reframe/core/pipeline.py index 34e5d415d2..10b3daaf78 100644 --- a/reframe/core/pipeline.py +++ b/reframe/core/pipeline.py @@ -783,14 +783,15 @@ def _set_parameter_space(cls, obj): @classmethod def is_abstract_test(cls): ''' Checks if the test is an abstract test ''' - if list(filter(lambda x: x==[], cls._rfm_params.values())) == []: - return False + if list(filter(lambda x: x == [], cls._rfm_params.values())) == []: + return False return True def _append_parameters_to_name(self): if self._rfm_params: - return '_' + '_'.join([str(self.__dict__[key]) for key in self._rfm_params]) + return '_' + '_'.join([str(self.__dict__[key]) + for key in self._rfm_params]) else: return '' From 30f260e5c7e391d5a5f21cae27f32fb7f9be37f3 Mon Sep 17 00:00:00 2001 From: "Javier J. Otero Perez" Date: Mon, 30 Nov 2020 12:09:50 +0100 Subject: [PATCH 10/43] Drop rfm_ prefix from the test attributes. --- reframe/core/decorators.py | 4 ++++ reframe/core/meta.py | 23 +++++++++++++++-------- 2 files changed, 19 insertions(+), 8 deletions(-) diff --git a/reframe/core/decorators.py b/reframe/core/decorators.py index 5318dafeec..47286c8a3d 100644 --- a/reframe/core/decorators.py +++ b/reframe/core/decorators.py @@ -102,6 +102,10 @@ def _param_space_iterator(cls): for k in cls._rfm_params]) cls._rfm_param_space_iter = _param_space_iterator(cls) + + # Register the test for each of the parameter space combinations. This + # does **not** feed the params to the test. That is done during the + # object creation. for inst in _param_space_iterator(cls): _register_test(cls) diff --git a/reframe/core/meta.py b/reframe/core/meta.py index dfeceda88a..bef5128fe8 100644 --- a/reframe/core/meta.py +++ b/reframe/core/meta.py @@ -16,24 +16,31 @@ class RegressionTestMeta(type): def __prepare__(cls, name, bases, **kwargs): namespace = super().__prepare__(name, bases, **kwargs) - # Extend the class attributes to the RegressionTest class - namespace['__rfm_attributes'] = RegressionTestAttributes() + # Extend the RegressionTest class with the directives defined in the + # RegressionTestAttributes class. + rfm_attr = RegressionTestAttributes() + namespace['__rfm_attributes'] = rfm_attr # Attribute to add a regression test parameter as: # `rfm_parameter('P0', [0,1,2,3])`. - namespace['rfm_parameter'] = namespace['__rfm_attributes']._rfm_parameter_stage.add + namespace['parameter'] = rfm_attr._rfm_parameter_stage.add # Attribute to purge a list of test parameters. - namespace['rfm_purge_parameters'] = namespace['__rfm_attributes']._rfm_parameter_stage.purge_parameters + namespace['purge_parameters'] = (rfm_attr + )._rfm_parameter_stage.purge_parameters # Attribute to purge the entire parameter space. - namespace['rfm_purge_all_parameters'] = namespace['__rfm_attributes']._rfm_parameter_stage.purge_all_parameters + namespace['purge_all_parameters'] = (rfm_attr._rfm_parameter_stage + ).purge_all_parameters # Method to build the parameter space - namespace['_rfm_build_parameter_space'] = namespace['__rfm_attributes'].build_parameter_space + namespace['_rfm_build_parameter_space'] = (rfm_attr + ).build_parameter_space - # Method to check that the test parameter space does not clash with the RegressionTest namespace - namespace['_rfm_namespace_clash_check'] = namespace['__rfm_attributes'].namespace_clash_check + # Method to check that the test parameter space does not clash with the + # RegressionTest namespace + namespace['_rfm_namespace_clash_check'] = (rfm_attr + ).namespace_clash_check return namespace From 528eae4b879b78cea86d8c1a105fef7c23bf6a31 Mon Sep 17 00:00:00 2001 From: "Javier J. Otero Perez" Date: Wed, 2 Dec 2020 10:26:25 +0100 Subject: [PATCH 11/43] Address initial PR comments. --- reframe/core/attributes.py | 90 ++++++++++++++------------------------ reframe/core/decorators.py | 19 +------- reframe/core/meta.py | 12 +---- 3 files changed, 38 insertions(+), 83 deletions(-) diff --git a/reframe/core/attributes.py b/reframe/core/attributes.py index 56547f8d43..a522cee577 100644 --- a/reframe/core/attributes.py +++ b/reframe/core/attributes.py @@ -14,18 +14,19 @@ class InputParameter: test parameter inheritance/chaining. ''' - def __init__(self, name, values=None, + def __init__(self, name, *values, inherit_params=False, filt_params=None): ''' name: parameter name values: parameter values - inherit_params: If false, it overrides all previous values set for the parameter. - filt_params: Function to filter/modify the inherited values for the parameter. - It only has an effect if used with inherit_params=True. - ''' - # If the values are None (or an empty list), the parameter is considered as - # declared but not defined (i.e. an abstract parameter). - if values is None: + inherit_params: If false, it overrides all previous values set for the + parameter. + filt_params: Function to filter/modify the inherited values for the + parameter. It only has an effect if used with inherit_params=True. + ''' + # If no values are passed, the parameter is considered as declared + # but not defined (i.e. an abstract parameter). + if values == (): values = [] # The values arg must be a list. @@ -38,31 +39,21 @@ def __init__(self, name, values=None, def filt_params(x): return x self.name = name - self.values = values + self.values = list(values) self.inherit_params = inherit_params self.filt_params = filt_params - def is_undef(self): - ''' - Handy function to check if the parameter is undefined (empty). - ''' - if self.values == []: - return True - - return False - class ParameterPack: ''' Bundle containing the test parameters inserted in each test (class). The parameters are store in a dictionary for a more efficient lookup. - The add method is the interface used to add new parameters to the reframe test. + The add method is the interface used to add new parameters to the reframe + test. ''' def __init__(self): self.parameter_map = {} - self.purge_parameter_space = False - self.parameter_blacklist = set() def add(self, name, values=None, inherit_params=False, filt_params=None): ''' @@ -76,20 +67,6 @@ def add(self, name, values=None, inherit_params=False, filt_params=None): raise ValueError( 'Cannot double-define a parameter in the same class.') - def purge_all_parameters(self): - ''' - Set the purge_parameter_space flag to true, which blocks the inheritance - of the full parameter space. - ''' - self.purge_parameter_space = True - - def purge_parameters(self, *params_to_purge): - ''' - Override the inheritance of a given set of parameters. - ''' - for param in params_to_purge: - self.parameter_blacklist.add(param) - class RegressionTestAttributes: ''' @@ -102,12 +79,6 @@ def __init__(self): def get_parameter_stage(self): return self._rfm_parameter_stage.parameter_map - def _purge_all_parameters(self): - return self._rfm_parameter_stage.purge_parameter_space - - def _parameter_blacklist(self): - return self._rfm_parameter_stage.parameter_blacklist - def _inherit_parameter_space(self, bases): ''' Build the parameter space from the base clases. @@ -123,8 +94,8 @@ def _inherit_parameter_space(self, bases): base_params = b._rfm_params for key in base_params: if ((key in self.get_parameter_stage() and - not self.get_parameter_stage().get(key).inherit_params) or - key in self._parameter_blacklist()): + not (self.get_parameter_stage().get(key) + ).inherit_params)): # Do not inherit a given parameter if the current # class wants to override it. @@ -132,13 +103,16 @@ def _inherit_parameter_space(self, bases): else: - # With multiple inheritance, a single parameter could be doubly defined - # and lead to repeated values. + # With multiple inheritance, a single parameter + # could be doubly defined and lead to repeated + # values. if key in temp_parameter_space: if not (temp_parameter_space[key] == [] or base_params[key] == []): - raise KeyError(f'Parameter space conflict (on {key}) ' - f'due to multiple inheritance.') from None + raise KeyError(f'Parameter space conflict ' + f'(on {key}) due to ' + f'multiple inheritance.' + ) from None temp_parameter_space[key] = base_params.get( key, []) + temp_parameter_space.get(key, []) @@ -152,20 +126,23 @@ def _inherit_parameter_space(self, bases): def _extend_parameter_space(self, parameter_space): ''' - Add the parameters from the parameter stage into the existing parameter space. - Do the inherit+filter operations as defined in the for each input parameter in the - parameter stage. + Add the parameters from the parameter stage into the existing parameter + space. + Do the inherit+filter operations as defined in the for each input + parameter in the parameter stage. ''' # Loop over the parameter stage. Each element is an instance of # InputParameter. for name, p in self.get_parameter_stage().items(): - parameter_space[name] = p.filt_params(parameter_space.get(name, [])) + p.values if ( - p.inherit_params) else p.values + parameter_space[name] = p.filt_params( + parameter_space.get(name, [])) + p.values if ( + p.inherit_params) else p.values def build_parameter_space(self, bases): ''' - Compiles the full test parameter space by joining the parameter spaces from the base clases, - and extending that with the parameters present in the parameter stage. + Compiles the full test parameter space by joining the parameter spaces + from the base clases, and extending that with the parameters present + in the parameter stage. ''' # Inherit from the bases param_space = self._inherit_parameter_space(bases) @@ -190,5 +167,6 @@ def namespace_clash_check(dict_a, dict_b, name=None): name = '__unknown__' if name is None else name for key in short_dict: if key in long_dict: - raise AttributeError(f'Attribute {key} clashes with other variables' - f'present in the namespace of class {name}') + raise AttributeError(f'Attribute {key} clashes with other ' + f'variables present in the namespace ' + f'of class {name}') diff --git a/reframe/core/decorators.py b/reframe/core/decorators.py index 47286c8a3d..f072ce408d 100644 --- a/reframe/core/decorators.py +++ b/reframe/core/decorators.py @@ -78,14 +78,14 @@ def _validate_test(cls): f' abstract test.') -def test(cls): +def simple_test(cls): '''Class decorator for registering tests with ReFrame. The decorated class must derive from :class:`reframe.core.pipeline.RegressionTest`. This decorator is also available directly under the :mod:`reframe` module. - .. versionadded:: #.## + .. versionadded:: 2.13 ''' def _do_register(cls): _validate_test(cls) @@ -114,21 +114,6 @@ def _param_space_iterator(cls): return _do_register(cls) -def simple_test(cls): - '''Class decorator for registering parameterless tests with ReFrame. - - The decorated class must derive from - :class:`reframe.core.pipeline.RegressionTest`. This decorator is also - available directly under the :mod:`reframe` module. - - .. versionadded:: 2.13 - - .. to be deprecated - ''' - - return test(cls) - - def parameterized_test(*inst): '''Class decorator for registering multiple instantiations of a test class. diff --git a/reframe/core/meta.py b/reframe/core/meta.py index bef5128fe8..4d8aa9bb48 100644 --- a/reframe/core/meta.py +++ b/reframe/core/meta.py @@ -25,14 +25,6 @@ def __prepare__(cls, name, bases, **kwargs): # `rfm_parameter('P0', [0,1,2,3])`. namespace['parameter'] = rfm_attr._rfm_parameter_stage.add - # Attribute to purge a list of test parameters. - namespace['purge_parameters'] = (rfm_attr - )._rfm_parameter_stage.purge_parameters - - # Attribute to purge the entire parameter space. - namespace['purge_all_parameters'] = (rfm_attr._rfm_parameter_stage - ).purge_all_parameters - # Method to build the parameter space namespace['_rfm_build_parameter_space'] = (rfm_attr ).build_parameter_space @@ -50,8 +42,8 @@ def __init__(cls, name, bases, namespace, **kwargs): # Set up the regression test parameter space cls._rfm_params = cls._rfm_build_parameter_space(bases) - # Make illegal to have a parameter clashing with any of the RegressionTest - # class variables + # Make illegal to have a parameter clashing with any of the + # RegressionTest class variables cls._rfm_namespace_clash_check(cls.__dict__, cls._rfm_params, cls.__qualname__) From 01f9bb4a69c4ef2c3bf9d0bbd8c196d7f8633c87 Mon Sep 17 00:00:00 2001 From: "Javier J. Otero Perez" Date: Fri, 4 Dec 2020 10:15:53 +0100 Subject: [PATCH 12/43] Bugfix RegressionTest directives. --- reframe/core/attributes.py | 65 +++++++++++++++++++------------------- reframe/core/meta.py | 2 +- 2 files changed, 33 insertions(+), 34 deletions(-) diff --git a/reframe/core/attributes.py b/reframe/core/attributes.py index a522cee577..33f6e9e74d 100644 --- a/reframe/core/attributes.py +++ b/reframe/core/attributes.py @@ -88,39 +88,38 @@ def _inherit_parameter_space(self, bases): temp_parameter_space = {} # Iterate over the base classes and inherit the parameter space - if not self._purge_all_parameters(): - for b in bases: - if hasattr(b, '_rfm_params'): - base_params = b._rfm_params - for key in base_params: - if ((key in self.get_parameter_stage() and - not (self.get_parameter_stage().get(key) - ).inherit_params)): - - # Do not inherit a given parameter if the current - # class wants to override it. - pass - - else: - - # With multiple inheritance, a single parameter - # could be doubly defined and lead to repeated - # values. - if key in temp_parameter_space: - if not (temp_parameter_space[key] == [] or - base_params[key] == []): - raise KeyError(f'Parameter space conflict ' - f'(on {key}) due to ' - f'multiple inheritance.' - ) from None - - temp_parameter_space[key] = base_params.get( - key, []) + temp_parameter_space.get(key, []) - - else: - # The base class does not have the attribute - # cls._rfm_params - pass + for b in bases: + if hasattr(b, '_rfm_params'): + base_params = b._rfm_params + for key in base_params: + if ((key in self.get_parameter_stage() and + not (self.get_parameter_stage().get(key) + ).inherit_params)): + + # Do not inherit a given parameter if the current + # class wants to override it. + pass + + else: + + # With multiple inheritance, a single parameter + # could be doubly defined and lead to repeated + # values. + if key in temp_parameter_space: + if not (temp_parameter_space[key] == [] or + base_params[key] == []): + raise KeyError(f'Parameter space conflict ' + f'(on {key}) due to ' + f'multiple inheritance.' + ) from None + + temp_parameter_space[key] = base_params.get( + key, []) + temp_parameter_space.get(key, []) + + else: + # The base class does not have the attribute + # cls._rfm_params + pass return temp_parameter_space diff --git a/reframe/core/meta.py b/reframe/core/meta.py index 4d8aa9bb48..e1df4b73f6 100644 --- a/reframe/core/meta.py +++ b/reframe/core/meta.py @@ -22,7 +22,7 @@ def __prepare__(cls, name, bases, **kwargs): namespace['__rfm_attributes'] = rfm_attr # Attribute to add a regression test parameter as: - # `rfm_parameter('P0', [0,1,2,3])`. + # `rfm_parameter('P0', 0,1,2,3)`. namespace['parameter'] = rfm_attr._rfm_parameter_stage.add # Method to build the parameter space From ae91526f8b4d26ff70570f5971e10f662867b420 Mon Sep 17 00:00:00 2001 From: Javier Otero Date: Fri, 4 Dec 2020 11:09:15 +0100 Subject: [PATCH 13/43] Bugfix the parameter directive. --- reframe/core/attributes.py | 29 ++++++++++------------------- reframe/core/decorators.py | 2 +- 2 files changed, 11 insertions(+), 20 deletions(-) diff --git a/reframe/core/attributes.py b/reframe/core/attributes.py index 33f6e9e74d..8138c19bdc 100644 --- a/reframe/core/attributes.py +++ b/reframe/core/attributes.py @@ -18,28 +18,19 @@ def __init__(self, name, *values, inherit_params=False, filt_params=None): ''' name: parameter name - values: parameter values + values: parameter values. If no values are passed, the parameter is + considered as declared but not defined (i.e. an abstract param). inherit_params: If false, it overrides all previous values set for the parameter. filt_params: Function to filter/modify the inherited values for the parameter. It only has an effect if used with inherit_params=True. ''' - # If no values are passed, the parameter is considered as declared - # but not defined (i.e. an abstract parameter). - if values == (): - values = [] - - # The values arg must be a list. - if not isinstance(values, list): - raise ValueError( - f'Parameter values must be defined in a list.') from None - # Default filter is no filter. if filt_params is None: def filt_params(x): return x self.name = name - self.values = list(values) + self.values = values self.inherit_params = inherit_params self.filt_params = filt_params @@ -55,14 +46,14 @@ class ParameterPack: def __init__(self): self.parameter_map = {} - def add(self, name, values=None, inherit_params=False, filt_params=None): + def add(self, name, *values, inherit_params=False, filt_params=None): ''' Insert a new parameter in the dictionary. If the parameter is already present in it, raise an error. ''' if name not in self.parameter_map: - self.parameter_map[name] = InputParameter( - name, values, inherit_params, filt_params) + self.parameter_map[name] = InputParameter(name, *values, + inherit_params=inherit_params, filt_params=filt_params) else: raise ValueError( 'Cannot double-define a parameter in the same class.') @@ -106,15 +97,15 @@ def _inherit_parameter_space(self, bases): # could be doubly defined and lead to repeated # values. if key in temp_parameter_space: - if not (temp_parameter_space[key] == [] or - base_params[key] == []): + if not (temp_parameter_space[key] == () or + base_params[key] == ()): raise KeyError(f'Parameter space conflict ' f'(on {key}) due to ' f'multiple inheritance.' ) from None temp_parameter_space[key] = base_params.get( - key, []) + temp_parameter_space.get(key, []) + key, ()) + temp_parameter_space.get(key, ()) else: # The base class does not have the attribute @@ -134,7 +125,7 @@ def _extend_parameter_space(self, parameter_space): # InputParameter. for name, p in self.get_parameter_stage().items(): parameter_space[name] = p.filt_params( - parameter_space.get(name, [])) + p.values if ( + parameter_space.get(name, ())) + p.values if ( p.inherit_params) else p.values def build_parameter_space(self, bases): diff --git a/reframe/core/decorators.py b/reframe/core/decorators.py index f072ce408d..0261b61c07 100644 --- a/reframe/core/decorators.py +++ b/reframe/core/decorators.py @@ -8,7 +8,7 @@ # __all__ = [ - 'test', 'parameterized_test', 'simple_test', 'required_version', + 'parameterized_test', 'simple_test', 'required_version', 'require_deps', 'run_before', 'run_after' ] From 26f2ee0f406037626bcf654bab2c061151be7f2d Mon Sep 17 00:00:00 2001 From: Javier Otero Date: Fri, 4 Dec 2020 11:25:15 +0100 Subject: [PATCH 14/43] Fix PEP8 style. --- reframe/core/attributes.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/reframe/core/attributes.py b/reframe/core/attributes.py index 8138c19bdc..b1f99b191b 100644 --- a/reframe/core/attributes.py +++ b/reframe/core/attributes.py @@ -27,7 +27,8 @@ def __init__(self, name, *values, ''' # Default filter is no filter. if filt_params is None: - def filt_params(x): return x + def filt_params(x): + return x self.name = name self.values = values @@ -52,8 +53,11 @@ def add(self, name, *values, inherit_params=False, filt_params=None): If the parameter is already present in it, raise an error. ''' if name not in self.parameter_map: - self.parameter_map[name] = InputParameter(name, *values, - inherit_params=inherit_params, filt_params=filt_params) + self.parameter_map[name] = InputParameter( + name, *values, + inherit_params=inherit_params, + filt_params=filt_params + ) else: raise ValueError( 'Cannot double-define a parameter in the same class.') From c23a549d601041b35010d214e89f6f7907877109 Mon Sep 17 00:00:00 2001 From: Javier Otero Date: Fri, 4 Dec 2020 20:03:10 +0100 Subject: [PATCH 15/43] Rename directives and cleanup. --- reframe/core/attributes.py | 166 ---------------------------------- reframe/core/directives.py | 180 +++++++++++++++++++++++++++++++++++++ reframe/core/meta.py | 30 +++---- 3 files changed, 192 insertions(+), 184 deletions(-) delete mode 100644 reframe/core/attributes.py create mode 100644 reframe/core/directives.py diff --git a/reframe/core/attributes.py b/reframe/core/attributes.py deleted file mode 100644 index b1f99b191b..0000000000 --- a/reframe/core/attributes.py +++ /dev/null @@ -1,166 +0,0 @@ -# Copyright 2016-2020 Swiss National Supercomputing Centre (CSCS/ETH Zurich) -# ReFrame Project Developers. See the top-level LICENSE file for details. -# -# SPDX-License-Identifier: BSD-3-Clause - -# -# attribute extension to the RegressionTest classes. -# - - -class InputParameter: - ''' - Input paramter and (name + values) and other attributes to deal with the - test parameter inheritance/chaining. - ''' - - def __init__(self, name, *values, - inherit_params=False, filt_params=None): - ''' - name: parameter name - values: parameter values. If no values are passed, the parameter is - considered as declared but not defined (i.e. an abstract param). - inherit_params: If false, it overrides all previous values set for the - parameter. - filt_params: Function to filter/modify the inherited values for the - parameter. It only has an effect if used with inherit_params=True. - ''' - # Default filter is no filter. - if filt_params is None: - def filt_params(x): - return x - - self.name = name - self.values = values - self.inherit_params = inherit_params - self.filt_params = filt_params - - -class ParameterPack: - ''' - Bundle containing the test parameters inserted in each test (class). - The parameters are store in a dictionary for a more efficient lookup. - The add method is the interface used to add new parameters to the reframe - test. - ''' - - def __init__(self): - self.parameter_map = {} - - def add(self, name, *values, inherit_params=False, filt_params=None): - ''' - Insert a new parameter in the dictionary. - If the parameter is already present in it, raise an error. - ''' - if name not in self.parameter_map: - self.parameter_map[name] = InputParameter( - name, *values, - inherit_params=inherit_params, - filt_params=filt_params - ) - else: - raise ValueError( - 'Cannot double-define a parameter in the same class.') - - -class RegressionTestAttributes: - ''' - Storage class hosting all the reframe class attributes. - ''' - - def __init__(self): - self._rfm_parameter_stage = ParameterPack() - - def get_parameter_stage(self): - return self._rfm_parameter_stage.parameter_map - - def _inherit_parameter_space(self, bases): - ''' - Build the parameter space from the base clases. - ''' - # Temporary dict where we build the parameter space from the base - # clases - temp_parameter_space = {} - - # Iterate over the base classes and inherit the parameter space - for b in bases: - if hasattr(b, '_rfm_params'): - base_params = b._rfm_params - for key in base_params: - if ((key in self.get_parameter_stage() and - not (self.get_parameter_stage().get(key) - ).inherit_params)): - - # Do not inherit a given parameter if the current - # class wants to override it. - pass - - else: - - # With multiple inheritance, a single parameter - # could be doubly defined and lead to repeated - # values. - if key in temp_parameter_space: - if not (temp_parameter_space[key] == () or - base_params[key] == ()): - raise KeyError(f'Parameter space conflict ' - f'(on {key}) due to ' - f'multiple inheritance.' - ) from None - - temp_parameter_space[key] = base_params.get( - key, ()) + temp_parameter_space.get(key, ()) - - else: - # The base class does not have the attribute - # cls._rfm_params - pass - - return temp_parameter_space - - def _extend_parameter_space(self, parameter_space): - ''' - Add the parameters from the parameter stage into the existing parameter - space. - Do the inherit+filter operations as defined in the for each input - parameter in the parameter stage. - ''' - # Loop over the parameter stage. Each element is an instance of - # InputParameter. - for name, p in self.get_parameter_stage().items(): - parameter_space[name] = p.filt_params( - parameter_space.get(name, ())) + p.values if ( - p.inherit_params) else p.values - - def build_parameter_space(self, bases): - ''' - Compiles the full test parameter space by joining the parameter spaces - from the base clases, and extending that with the parameters present - in the parameter stage. - ''' - # Inherit from the bases - param_space = self._inherit_parameter_space(bases) - - # Extend with what was added in the current class - self._extend_parameter_space(param_space) - - return param_space - - @staticmethod - def namespace_clash_check(dict_a, dict_b, name=None): - ''' - Check that these two dictionaries do not have any overlapping keys. - ''' - if len(dict_a) > len(dict_b): - long_dict = dict_a - short_dict = dict_b - else: - long_dict = dict_b - short_dict = dict_a - - name = '__unknown__' if name is None else name - for key in short_dict: - if key in long_dict: - raise AttributeError(f'Attribute {key} clashes with other ' - f'variables present in the namespace ' - f'of class {name}') diff --git a/reframe/core/directives.py b/reframe/core/directives.py new file mode 100644 index 0000000000..b1242161c7 --- /dev/null +++ b/reframe/core/directives.py @@ -0,0 +1,180 @@ +# Copyright 2016-2020 Swiss National Supercomputing Centre (CSCS/ETH Zurich) +# ReFrame Project Developers. See the top-level LICENSE file for details. +# +# SPDX-License-Identifier: BSD-3-Clause + +# +# attribute extension to the RegressionTest classes. +# + + +class RegressionTestParameter: + ''' + Regression test paramter class. + Stores the attributes of a regression test parameter needed to build + the full parameter space of a regression test. These are the parameter's + name, values, and inheritance behaviour. + + This is used by the ParameterStagingArea class below. + ''' + + def __init__(self, name, *values, + inherit_params=False, filter_params=None): + ''' + name: parameter name + values: parameter values. If no values are passed, the parameter is + considered as declared but not defined (i.e. an abstract param). + inherit_params: If false, this parameter is marked to not inherit any + values that might have been defined in a parent class. + filter_params: Function to filter/modify the inherited parameter values + from the parent classes. This only has an effect if used with + inherit_params=True. + ''' + # By default, do not filter any of the inherited parameter values. + if filter_params is None: + def filter_params(x): + return x + + self.name = name + self.values = values + self.inherit_params = inherit_params + self.filt_params = filter_params + + +class ParameterStagingArea: + ''' + Staging area of the regression test parameters used to build the + regression test parameter space. This staging area is simply a set of + regression test parameters declared in a given class derived from the + RegressionTest class. This set does not include any other parameters + that might have been declared/defined in any of the parent classes. The + parameter staging area must not be confused with the regression test's + parameter space. + + Example: In the pseudo-code below, the parameter staging area of A is {P0}, + and the staging area of B is {P1}. However, the parameter space of A is + still {P0}, and the parameter space of B is {P0, P1}. + + class A(RegressionTest): + -> define parameter P0 with value X. + + class B(A): + -> define parameter P1 with value Y. + + + The parameter staging area is populated during the class body execution. + This is done using class directives that call the member function + add_regression_test_parameter. + + The regression test's parameter space is assembled during the class object + initialization (i.e. the __init__ method of the metaclass), where a call + to the member function build_parameter_space is made. This member function + handles the parameter inheritance from the parent classes. + ''' + + def __init__(self): + self.staging_area = {} + + def add_regression_test_parameter(self, name, *values, **kwargs): + ''' + Insert a new regression test parameter in the staging area. + If the parameter is already present in the dictionary, raise an error. + See the RegressionTestParameter class for further information on the + function arguments. + ''' + if name not in self.staging_area: + self.staging_area[name] = RegressionTestParameter( + name, *values, **kwargs + ) + else: + raise ValueError( + 'Cannot double-define a parameter in the same class.' + ) from None + + + def _inherit_parameter_space(self, bases, param_space_key): + ''' + Inherit the parameter space from the base clases. Note that the + parameter space is simply a dictionary where the keys are the + parameter names and the values of this dictionary are tuples with the + values of the associated parameter. + + bases: iterable containing the parent classes. + param_space_key: class attribute name under which the parameter space is + stored. + ''' + # Temporary dict where we build the parameter space from the base + # clases + temp_parameter_space = {} + + # Iterate over the base classes and inherit the parameter space + for b in bases: + base_params = b.__dict__.get(param_space_key, ()) + for key in base_params: + # With multiple inheritance, a single parameter + # could be doubly defined and lead to repeated + # values. + if key in temp_parameter_space: + if not (temp_parameter_space[key] == () or + base_params[key] == ()): + raise KeyError(f'Parameter space conflict ' + f'(on {key}) due to ' + f'multiple inheritance.' + ) from None + + temp_parameter_space[key] = base_params.get( + key, ()) + temp_parameter_space.get(key, ()) + + return temp_parameter_space + + def _extend_parameter_space(self, parameter_space): + ''' + Add the parameters from the staging area into the parameter space + inherited from the parent classes. + + Each parameter is dealt with independently, following the inheritance + behaviour set in the RegressionTestParameter class (i.e. inherit+filter + operations as defined for each parameter). + ''' + # Loop over the staging area. + for name, p in self.staging_area.items(): + parameter_space[name] = p.filt_params( + parameter_space.get(name, ())) + p.values if ( + p.inherit_params) else p.values + + def build_parameter_space(self, bases, param_space_key): + ''' + Compiles the full test parameter space by joining the parameter spaces + from the base clases, and extending that with the parameters present + in the staging area. + + bases: iterable containing the parent classes. + param_space_key: class attribute name under which the parameter space is + stored. + ''' + # Inherit from the bases + param_space = self._inherit_parameter_space(bases, param_space_key) + + # Extend with what was added in the current class + self._extend_parameter_space(param_space) + + return param_space + + +def namespace_clash_check(dict_a, dict_b, name=None): + ''' + Check that these two dictionaries do not have any overlapping keys. + ''' + if len(dict_a) > len(dict_b): + long_dict = dict_a + short_dict = dict_b + else: + long_dict = dict_b + short_dict = dict_a + + name = '__unknown__' if name is None else name + for key in short_dict: + if key in long_dict: + raise AttributeError(f'Attribute {key} clashes with other ' + f'variables present in the namespace ' + f'from class {name}') diff --git a/reframe/core/meta.py b/reframe/core/meta.py index e1df4b73f6..7512242ab4 100644 --- a/reframe/core/meta.py +++ b/reframe/core/meta.py @@ -8,7 +8,7 @@ # from reframe.core.warnings import user_deprecation_warning -from reframe.core.attributes import RegressionTestAttributes +import reframe.core.directives as directives class RegressionTestMeta(type): @@ -16,36 +16,30 @@ class RegressionTestMeta(type): def __prepare__(cls, name, bases, **kwargs): namespace = super().__prepare__(name, bases, **kwargs) - # Extend the RegressionTest class with the directives defined in the - # RegressionTestAttributes class. - rfm_attr = RegressionTestAttributes() - namespace['__rfm_attributes'] = rfm_attr + # Staging area to build the regression test parameter space + # using directives + param_stage = directives.ParameterStagingArea() - # Attribute to add a regression test parameter as: - # `rfm_parameter('P0', 0,1,2,3)`. - namespace['parameter'] = rfm_attr._rfm_parameter_stage.add + # Directive to add a regression test parameter directly in the + # class body as: `parameter('P0', 0,1,2,3)`. + namespace['parameter'] = param_stage.add_regression_test_parameter - # Method to build the parameter space - namespace['_rfm_build_parameter_space'] = (rfm_attr + # Export the method to build the final parameter space + namespace['_rfm_build_parameter_space'] = (param_stage ).build_parameter_space - # Method to check that the test parameter space does not clash with the - # RegressionTest namespace - namespace['_rfm_namespace_clash_check'] = (rfm_attr - ).namespace_clash_check - return namespace def __init__(cls, name, bases, namespace, **kwargs): super().__init__(name, bases, namespace, **kwargs) # Set up the regression test parameter space - cls._rfm_params = cls._rfm_build_parameter_space(bases) + cls._rfm_params = cls._rfm_build_parameter_space(bases, '_rfm_params') # Make illegal to have a parameter clashing with any of the # RegressionTest class variables - cls._rfm_namespace_clash_check(cls.__dict__, cls._rfm_params, - cls.__qualname__) + directives.namespace_clash_check(cls.__dict__, cls._rfm_params, + cls.__qualname__) # Set up the hooks for the pipeline stages based on the _rfm_attach # attribute; all dependencies will be resolved first in the post-setup From e5bea30b500b32fc2a19b8b362e8aeb43de472bb Mon Sep 17 00:00:00 2001 From: Javier Otero Date: Fri, 4 Dec 2020 20:17:10 +0100 Subject: [PATCH 16/43] Fix PEP8 complaints. --- reframe/core/directives.py | 35 +++++++++++++++++------------------ 1 file changed, 17 insertions(+), 18 deletions(-) diff --git a/reframe/core/directives.py b/reframe/core/directives.py index b1242161c7..7ca6820515 100644 --- a/reframe/core/directives.py +++ b/reframe/core/directives.py @@ -91,7 +91,6 @@ def add_regression_test_parameter(self, name, *values, **kwargs): 'Cannot double-define a parameter in the same class.' ) from None - def _inherit_parameter_space(self, bases, param_space_key): ''' Inherit the parameter space from the base clases. Note that the @@ -100,8 +99,8 @@ def _inherit_parameter_space(self, bases, param_space_key): values of the associated parameter. bases: iterable containing the parent classes. - param_space_key: class attribute name under which the parameter space is - stored. + param_space_key: class attribute name under which the parameter space + is stored. ''' # Temporary dict where we build the parameter space from the base # clases @@ -111,19 +110,19 @@ def _inherit_parameter_space(self, bases, param_space_key): for b in bases: base_params = b.__dict__.get(param_space_key, ()) for key in base_params: - # With multiple inheritance, a single parameter - # could be doubly defined and lead to repeated - # values. - if key in temp_parameter_space: - if not (temp_parameter_space[key] == () or - base_params[key] == ()): - raise KeyError(f'Parameter space conflict ' - f'(on {key}) due to ' - f'multiple inheritance.' - ) from None - - temp_parameter_space[key] = base_params.get( - key, ()) + temp_parameter_space.get(key, ()) + # With multiple inheritance, a single parameter + # could be doubly defined and lead to repeated + # values. + if key in temp_parameter_space: + if not (temp_parameter_space[key] == () or + base_params[key] == ()): + raise KeyError(f'Parameter space conflict ' + f'(on {key}) due to ' + f'multiple inheritance.' + ) from None + + temp_parameter_space[key] = base_params.get( + key, ()) + temp_parameter_space.get(key, ()) return temp_parameter_space @@ -149,8 +148,8 @@ def build_parameter_space(self, bases, param_space_key): in the staging area. bases: iterable containing the parent classes. - param_space_key: class attribute name under which the parameter space is - stored. + param_space_key: class attribute name under which the parameter space + is stored. ''' # Inherit from the bases param_space = self._inherit_parameter_space(bases, param_space_key) From 351274d34dc03938125d1955e7101e808e48c084 Mon Sep 17 00:00:00 2001 From: Javier Otero Date: Mon, 7 Dec 2020 17:06:40 +0100 Subject: [PATCH 17/43] Add prepare parameter space method. --- reframe/core/decorators.py | 19 ++----------------- reframe/core/meta.py | 2 +- reframe/core/pipeline.py | 32 +++++++++++++++++++++++++++++++- 3 files changed, 34 insertions(+), 19 deletions(-) diff --git a/reframe/core/decorators.py b/reframe/core/decorators.py index 0261b61c07..aab9231d13 100644 --- a/reframe/core/decorators.py +++ b/reframe/core/decorators.py @@ -90,23 +90,8 @@ def simple_test(cls): def _do_register(cls): _validate_test(cls) - # If cls is not a parametrised test, register as is and return - if not cls._rfm_params: - _register_test(cls) - return cls - - # Create iterator to cycle through all possible combinations in the - # parameter space. - def _param_space_iterator(cls): - return itertools.product(*[cls._rfm_params.get(k) - for k in cls._rfm_params]) - - cls._rfm_param_space_iter = _param_space_iterator(cls) - - # Register the test for each of the parameter space combinations. This - # does **not** feed the params to the test. That is done during the - # object creation. - for inst in _param_space_iterator(cls): + # Register the test as many times as its parameter space length + for i in range(cls.prepare_param_space()): _register_test(cls) return cls diff --git a/reframe/core/meta.py b/reframe/core/meta.py index 7512242ab4..a3194f41da 100644 --- a/reframe/core/meta.py +++ b/reframe/core/meta.py @@ -24,7 +24,7 @@ def __prepare__(cls, name, bases, **kwargs): # class body as: `parameter('P0', 0,1,2,3)`. namespace['parameter'] = param_stage.add_regression_test_parameter - # Export the method to build the final parameter space + # Method to build the final parameter space namespace['_rfm_build_parameter_space'] = (param_stage ).build_parameter_space diff --git a/reframe/core/pipeline.py b/reframe/core/pipeline.py index 10b3daaf78..faeedebabf 100644 --- a/reframe/core/pipeline.py +++ b/reframe/core/pipeline.py @@ -750,6 +750,36 @@ def __new__(cls, *args, **kwargs): def __init__(self): pass + @classmethod + def param_space_len(cls): + ''' + Returns the total number of points in the parameter space. + If the RegressionTest has no parameters, the length is 1. + Otherwise, the length is the number of all-to-all combinations + for each of the values in cls._rfm_params. + ''' + if not cls._rfm_params: + return 1 + + return functools.reduce( + lambda x, y: x*y, + (len(p) for p in cls._rfm_params.values()) + ) + + @classmethod + def prepare_param_space(cls): + ''' + Creates an iterator to traverse the full parameter space during the + class instantiation, and it also returns the lenght of the parameter + space. This is the number of instances of this class to be created. + ''' + if cls._rfm_params: + cls._rfm_param_space_iter = itertools.product( + *(p for p in cls._rfm_params.values()) + ) + + return cls.param_space_len() + @classmethod def _set_parameter_space(cls, obj): ''' @@ -783,7 +813,7 @@ def _set_parameter_space(cls, obj): @classmethod def is_abstract_test(cls): ''' Checks if the test is an abstract test ''' - if list(filter(lambda x: x == [], cls._rfm_params.values())) == []: + if len(list(filter(lambda x: x == (), cls._rfm_params.values()))) == 0: return False return True From 55703c44ffd3999808c348817e50d226b7a2e16a Mon Sep 17 00:00:00 2001 From: Javier Otero Date: Mon, 7 Dec 2020 18:55:02 +0100 Subject: [PATCH 18/43] Simplify the namespace clashing check. --- reframe/core/directives.py | 29 ++++++++++------------------- reframe/core/meta.py | 14 ++++++-------- 2 files changed, 16 insertions(+), 27 deletions(-) diff --git a/reframe/core/directives.py b/reframe/core/directives.py index 7ca6820515..235d69b0da 100644 --- a/reframe/core/directives.py +++ b/reframe/core/directives.py @@ -141,12 +141,14 @@ def _extend_parameter_space(self, parameter_space): parameter_space.get(name, ())) + p.values if ( p.inherit_params) else p.values - def build_parameter_space(self, bases, param_space_key): + def build_parameter_space(self, target_cls, bases, param_space_key): ''' Compiles the full test parameter space by joining the parameter spaces from the base clases, and extending that with the parameters present in the staging area. + target_cls: class to test if the parameter space overlaps with its own + namespace. bases: iterable containing the parent classes. param_space_key: class attribute name under which the parameter space is stored. @@ -157,23 +159,12 @@ def build_parameter_space(self, bases, param_space_key): # Extend with what was added in the current class self._extend_parameter_space(param_space) - return param_space + trgt_namespace = set(dir(target_cls)) + for key in param_space: + if key in trgt_namespace: + raise AttributeError(f'Attribute {key} clashes with other ' + f'variables present in the namespace ' + f'from class {target_cls.__qualname__}') + return param_space -def namespace_clash_check(dict_a, dict_b, name=None): - ''' - Check that these two dictionaries do not have any overlapping keys. - ''' - if len(dict_a) > len(dict_b): - long_dict = dict_a - short_dict = dict_b - else: - long_dict = dict_b - short_dict = dict_a - - name = '__unknown__' if name is None else name - for key in short_dict: - if key in long_dict: - raise AttributeError(f'Attribute {key} clashes with other ' - f'variables present in the namespace ' - f'from class {name}') diff --git a/reframe/core/meta.py b/reframe/core/meta.py index d907b7bf17..509fd21fda 100644 --- a/reframe/core/meta.py +++ b/reframe/core/meta.py @@ -24,9 +24,12 @@ def __prepare__(cls, name, bases, **kwargs): # class body as: `parameter('P0', 0,1,2,3)`. namespace['parameter'] = param_stage.add_regression_test_parameter + @classmethod + def build_param_space(cls, *args, **kwargs): + return param_stage.build_parameter_space(cls, *args, **kwargs) + # Method to build the final parameter space - namespace['_rfm_build_parameter_space'] = (param_stage - ).build_parameter_space + namespace['_rfm_build_param_space'] = build_param_space return namespace @@ -34,12 +37,7 @@ def __init__(cls, name, bases, namespace, **kwargs): super().__init__(name, bases, namespace, **kwargs) # Set up the regression test parameter space - cls._rfm_params = cls._rfm_build_parameter_space(bases, '_rfm_params') - - # Make illegal to have a parameter clashing with any of the - # RegressionTest class variables - ReframeDirectives.namespace_clash_check(cls.__dict__, cls._rfm_params, - cls.__qualname__) + cls._rfm_params = cls._rfm_build_param_space(bases, '_rfm_params') # Set up the hooks for the pipeline stages based on the _rfm_attach # attribute; all dependencies will be resolved first in the post-setup From 98d65c87771f001ab7f1c7ce34c210dda47105d6 Mon Sep 17 00:00:00 2001 From: Javier Otero Date: Mon, 7 Dec 2020 18:58:27 +0100 Subject: [PATCH 19/43] Fix PEP8 issues. --- reframe/core/directives.py | 1 - 1 file changed, 1 deletion(-) diff --git a/reframe/core/directives.py b/reframe/core/directives.py index 235d69b0da..84acd66290 100644 --- a/reframe/core/directives.py +++ b/reframe/core/directives.py @@ -167,4 +167,3 @@ def build_parameter_space(self, target_cls, bases, param_space_key): f'from class {target_cls.__qualname__}') return param_space - From 702d8553456e645ccfee98b84f60921196771a47 Mon Sep 17 00:00:00 2001 From: Javier Otero Date: Mon, 7 Dec 2020 19:03:51 +0100 Subject: [PATCH 20/43] Remove unused itertools import. --- reframe/core/decorators.py | 1 - 1 file changed, 1 deletion(-) diff --git a/reframe/core/decorators.py b/reframe/core/decorators.py index aab9231d13..6b54287248 100644 --- a/reframe/core/decorators.py +++ b/reframe/core/decorators.py @@ -18,7 +18,6 @@ import inspect import sys import traceback -import itertools import reframe from reframe.core.exceptions import ReframeSyntaxError, user_frame From 63c292772346dccd43f39b38e6ff9bcbc722821c Mon Sep 17 00:00:00 2001 From: Javier Otero Date: Wed, 9 Dec 2020 18:25:55 +0100 Subject: [PATCH 21/43] Simplify simple_test decorator. --- reframe/core/decorators.py | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/reframe/core/decorators.py b/reframe/core/decorators.py index 6b54287248..dbe36b3c37 100644 --- a/reframe/core/decorators.py +++ b/reframe/core/decorators.py @@ -86,16 +86,14 @@ def simple_test(cls): .. versionadded:: 2.13 ''' - def _do_register(cls): - _validate_test(cls) + _validate_test(cls) - # Register the test as many times as its parameter space length - for i in range(cls.prepare_param_space()): - _register_test(cls) + # Register the test as many times as its parameter space length + for _ in range(cls.prepare_param_space()): + _register_test(cls) - return cls + return cls - return _do_register(cls) def parameterized_test(*inst): From 5909d94a4df89aee20888edf81073ed7c112f7ca Mon Sep 17 00:00:00 2001 From: Javier Otero Date: Tue, 15 Dec 2020 12:38:53 +0100 Subject: [PATCH 22/43] Move parameter functionality to parameters.py. --- reframe/core/directives.py | 169 ------------------------------- reframe/core/meta.py | 19 ++-- reframe/core/parameters.py | 198 +++++++++++++++++++++++++++++++++++++ 3 files changed, 204 insertions(+), 182 deletions(-) delete mode 100644 reframe/core/directives.py create mode 100644 reframe/core/parameters.py diff --git a/reframe/core/directives.py b/reframe/core/directives.py deleted file mode 100644 index 84acd66290..0000000000 --- a/reframe/core/directives.py +++ /dev/null @@ -1,169 +0,0 @@ -# Copyright 2016-2020 Swiss National Supercomputing Centre (CSCS/ETH Zurich) -# ReFrame Project Developers. See the top-level LICENSE file for details. -# -# SPDX-License-Identifier: BSD-3-Clause - -# -# attribute extension to the RegressionTest classes. -# - - -class RegressionTestParameter: - ''' - Regression test paramter class. - Stores the attributes of a regression test parameter needed to build - the full parameter space of a regression test. These are the parameter's - name, values, and inheritance behaviour. - - This is used by the ParameterStagingArea class below. - ''' - - def __init__(self, name, *values, - inherit_params=False, filter_params=None): - ''' - name: parameter name - values: parameter values. If no values are passed, the parameter is - considered as declared but not defined (i.e. an abstract param). - inherit_params: If false, this parameter is marked to not inherit any - values that might have been defined in a parent class. - filter_params: Function to filter/modify the inherited parameter values - from the parent classes. This only has an effect if used with - inherit_params=True. - ''' - # By default, do not filter any of the inherited parameter values. - if filter_params is None: - def filter_params(x): - return x - - self.name = name - self.values = values - self.inherit_params = inherit_params - self.filt_params = filter_params - - -class ParameterStagingArea: - ''' - Staging area of the regression test parameters used to build the - regression test parameter space. This staging area is simply a set of - regression test parameters declared in a given class derived from the - RegressionTest class. This set does not include any other parameters - that might have been declared/defined in any of the parent classes. The - parameter staging area must not be confused with the regression test's - parameter space. - - Example: In the pseudo-code below, the parameter staging area of A is {P0}, - and the staging area of B is {P1}. However, the parameter space of A is - still {P0}, and the parameter space of B is {P0, P1}. - - class A(RegressionTest): - -> define parameter P0 with value X. - - class B(A): - -> define parameter P1 with value Y. - - - The parameter staging area is populated during the class body execution. - This is done using class directives that call the member function - add_regression_test_parameter. - - The regression test's parameter space is assembled during the class object - initialization (i.e. the __init__ method of the metaclass), where a call - to the member function build_parameter_space is made. This member function - handles the parameter inheritance from the parent classes. - ''' - - def __init__(self): - self.staging_area = {} - - def add_regression_test_parameter(self, name, *values, **kwargs): - ''' - Insert a new regression test parameter in the staging area. - If the parameter is already present in the dictionary, raise an error. - See the RegressionTestParameter class for further information on the - function arguments. - ''' - if name not in self.staging_area: - self.staging_area[name] = RegressionTestParameter( - name, *values, **kwargs - ) - else: - raise ValueError( - 'Cannot double-define a parameter in the same class.' - ) from None - - def _inherit_parameter_space(self, bases, param_space_key): - ''' - Inherit the parameter space from the base clases. Note that the - parameter space is simply a dictionary where the keys are the - parameter names and the values of this dictionary are tuples with the - values of the associated parameter. - - bases: iterable containing the parent classes. - param_space_key: class attribute name under which the parameter space - is stored. - ''' - # Temporary dict where we build the parameter space from the base - # clases - temp_parameter_space = {} - - # Iterate over the base classes and inherit the parameter space - for b in bases: - base_params = b.__dict__.get(param_space_key, ()) - for key in base_params: - # With multiple inheritance, a single parameter - # could be doubly defined and lead to repeated - # values. - if key in temp_parameter_space: - if not (temp_parameter_space[key] == () or - base_params[key] == ()): - raise KeyError(f'Parameter space conflict ' - f'(on {key}) due to ' - f'multiple inheritance.' - ) from None - - temp_parameter_space[key] = base_params.get( - key, ()) + temp_parameter_space.get(key, ()) - - return temp_parameter_space - - def _extend_parameter_space(self, parameter_space): - ''' - Add the parameters from the staging area into the parameter space - inherited from the parent classes. - - Each parameter is dealt with independently, following the inheritance - behaviour set in the RegressionTestParameter class (i.e. inherit+filter - operations as defined for each parameter). - ''' - # Loop over the staging area. - for name, p in self.staging_area.items(): - parameter_space[name] = p.filt_params( - parameter_space.get(name, ())) + p.values if ( - p.inherit_params) else p.values - - def build_parameter_space(self, target_cls, bases, param_space_key): - ''' - Compiles the full test parameter space by joining the parameter spaces - from the base clases, and extending that with the parameters present - in the staging area. - - target_cls: class to test if the parameter space overlaps with its own - namespace. - bases: iterable containing the parent classes. - param_space_key: class attribute name under which the parameter space - is stored. - ''' - # Inherit from the bases - param_space = self._inherit_parameter_space(bases, param_space_key) - - # Extend with what was added in the current class - self._extend_parameter_space(param_space) - - trgt_namespace = set(dir(target_cls)) - for key in param_space: - if key in trgt_namespace: - raise AttributeError(f'Attribute {key} clashes with other ' - f'variables present in the namespace ' - f'from class {target_cls.__qualname__}') - - return param_space diff --git a/reframe/core/meta.py b/reframe/core/meta.py index 509fd21fda..33384d3583 100644 --- a/reframe/core/meta.py +++ b/reframe/core/meta.py @@ -8,7 +8,7 @@ # from reframe.core.exceptions import ReframeSyntaxError -import reframe.core.directives as ReframeDirectives +import reframe.core.parameters as parameters class RegressionTestMeta(type): @@ -16,20 +16,13 @@ class RegressionTestMeta(type): def __prepare__(cls, name, bases, **kwargs): namespace = super().__prepare__(name, bases, **kwargs) - # Staging area to build the regression test parameter space - # using directives - param_stage = ReframeDirectives.ParameterStagingArea() + # Regression test parameter space defined at the class level + local_param_space = parameters.LocalParamSpace() + namespace['_rfm_local_param_space'] = local_param_space # Directive to add a regression test parameter directly in the # class body as: `parameter('P0', 0,1,2,3)`. - namespace['parameter'] = param_stage.add_regression_test_parameter - - @classmethod - def build_param_space(cls, *args, **kwargs): - return param_stage.build_parameter_space(cls, *args, **kwargs) - - # Method to build the final parameter space - namespace['_rfm_build_param_space'] = build_param_space + namespace['parameter'] = local_param_space.add_param return namespace @@ -37,7 +30,7 @@ def __init__(cls, name, bases, namespace, **kwargs): super().__init__(name, bases, namespace, **kwargs) # Set up the regression test parameter space - cls._rfm_params = cls._rfm_build_param_space(bases, '_rfm_params') + cls._rfm_params = parameters.build_parameter_space(cls) # Set up the hooks for the pipeline stages based on the _rfm_attach # attribute; all dependencies will be resolved first in the post-setup diff --git a/reframe/core/parameters.py b/reframe/core/parameters.py new file mode 100644 index 0000000000..41a01e1602 --- /dev/null +++ b/reframe/core/parameters.py @@ -0,0 +1,198 @@ +# Copyright 2016-2020 Swiss National Supercomputing Centre (CSCS/ETH Zurich) +# ReFrame Project Developers. See the top-level LICENSE file for details. +# +# SPDX-License-Identifier: BSD-3-Clause + +# +# Functionality to build extensible parameterized tests. +# + +from reframe.core.exceptions import ReframeSyntaxError, NameConflictError + +class _TestParameter: + '''Regression test paramter class. + + Stores the attributes of a regression test parameter as defined directly + in the test definition. These attributes are the parameter's name, + values, and inheritance behaviour. This class should be thought of as a + temporary storage for these parameter attributes, before the full final + parameter space is built. + + :param name: parameter name + :param values: parameter values. If no values are passed, the parameter is + considered as declared but not defined (i.e. an abstract parameter). + :param inherit_params: If false, this parameter is marked to not inherit + any values for the same parameter that might have been defined in a + parent class. + :param filter_params: Function to filter/modify the inherited parameter + values from the parent classes. This only has an effect if used with + inherit_params=True. + ''' + + def __init__(self, name, *values, + inherit_params=False, filter_params=None): + # By default, filter out all the parameter values defined in the + # base classes. + if not inherit_params: + def filter_params(x): + return () + + # If inherit_params==True, inherit all the parameter values from the + # base classes as default behaviour. + elif filter_params is None: + def filter_params(x): + return x + + self.name = name + self.values = values + self.filter_params = filter_params + + +class LocalParamSpace: + '''Local parameter space of a regression test. + + Stores all the regression test parameters defined in the test class body. + In the context of this class, a regression test parameter is an instance + of the class _TestParameter. This local parameter space is populated + during the test class body execution through the add_param method, and the + different parameters are stored under the _params attribute. This class + should be thought of as a temporary storage for this local parameter space, + before the full final parameter space is built. + + Example: In the pseudo-code below, the local parameter space of A is {P0}, + and the local parameter space of B is {P1}. However, the final parameter + space of A is still {P0}, and the final parameter space of B is {P0, P1}. + + class A(RegressionTest): + -> define parameter P0 with value X. + + class B(A): + -> define parameter P1 with value Y. + ''' + + def __init__(self): + self._params = {} + + def __getattr__(self, name): + # Delegate any unknown attribute access to the actual parameter space + return getattr(self._params, name) + + def __setitem__(self, name, value): + if name not in self._params: + self._params[name] = value + else: + raise ValueError( + 'parameter {name} already defined in this class' + ) + + def add_param(self, name, *values, **kwargs): + ''' + Insert a new regression test parameter in the staging area. + If the parameter is already present in the dictionary, raise an error. + See the _TestParameter class for further information on the + function arguments. + ''' + self[name] = _TestParameter(name, *values, **kwargs) + + @property + def params(self): + return self._params.items() + + +def _merge_parameter_spaces(bases): + '''Merges the parameter space from multiple classes. + + Joins the parameter space of multiple classess into a single parameter + space. This method allows multiple inheritance, as long as a parameter is + not doubly defined in two or more different parameter spaces. + + :param bases: iterable containing the parent classes. + + :returns: merged parameter space. + ''' + # Temporary dict where we build the parameter space from the base + # classes + param_space = {} + + # Iterate over the base classes and inherit the parameter space + for b in bases: + base_params = getattr(b, '_rfm_params', ()) + for key in base_params: + # With multiple inheritance, a single parameter + # could be doubly defined and lead to repeated + # values. + if key in param_space: + if not (param_space[key] == () or + base_params[key] == ()): + raise NameConflictError(f'Parameter space conflict ' + f'(on {key}) due to ' + f'multiple inheritance.' + ) from None + + param_space[key] = base_params.get( + key, ()) + param_space.get(key, ()) + + return param_space + + +def _extend_parameter_space(local_param_space, param_space): + '''Extend a given parameter space with a local parameter space. + + Each parameter is dealt with independently, given that each parameter + has its own inheritance behaviour defined in the local parameter space + (see the + :class:`reframe.core.parameters_TestParameter` class). + + :param local_param_space: a local parameter space from a regression test. + This must be an instance of the class + :class:`reframe.core.parameters.LocalParamSpace`. + :param param_space: an existing parameter space. This **must** have been + generated with + :meth:`reframe.core.parameters._merge_parameter_spaces`. + + ''' + # Ensure that the local parameter space is an instance of LocalParamSpace + if not isinstance(local_param_space, LocalParamSpace): + raise ReframeSyntaxError(f'the local_param_space must be an ' + f'instance of the LocalParamSpace class') + + # Loop over the staging area. + for name, p in local_param_space.params: + param_space[name] = p.filter_params( + param_space.get(name, ())) + p.values + + +def build_parameter_space(cls): + ''' Builder of the full parameter space of a regression test. + + Handles the full parameter space build, inheriting the parameter spaces + form the base classes, and extending these with the local parameter space + of the class (stored in cls._rfm_local_param_space). This method is called + during the class object initialization (i.e. the __init__ method of the + regression test metaclass). This method has three main steps, which are + (in order of execution) the inheritance of the parameter spaces from the + base classes, the extension of the inherited parameter space with the + local parameter space, and lastly, a check to ensure that none of the + parameter names clashes with any of the class attributes existing in the + regression test class. + + :param cls: the class where the full parameter space is to be built. + + :returns: dictionary containing the full parameter space. The keys are the + parameter names and the values are tuples containing all the values + for each of the parameters. + ''' + # Inherit the parameter space from the base classes + param_space = _merge_parameter_spaces(cls.__bases__) + + # Extend the parameter space with the local parameter space + _extend_parameter_space(cls._rfm_local_param_space, param_space) + + trgt_namespace = set(dir(cls)) + for key in param_space: + if key in trgt_namespace: + raise NameConflictError(f'Attribute {key} clashes with other ' + f'variables present in the namespace ' + f'from class {cls.__qualname__}') + + return param_space From 2f290086f430d6d0b93b8fc03ca6f18799f37761 Mon Sep 17 00:00:00 2001 From: Javier Otero Date: Tue, 15 Dec 2020 12:44:03 +0100 Subject: [PATCH 23/43] Address more PR comments. --- reframe/core/pipeline.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/reframe/core/pipeline.py b/reframe/core/pipeline.py index 31249613d0..70283f4912 100644 --- a/reframe/core/pipeline.py +++ b/reframe/core/pipeline.py @@ -708,7 +708,7 @@ def __new__(cls, *args, **kwargs): # Abort the class instantiation if the test is an abstract test. if cls.is_abstract_test(): - raise ValueError('Cannot instantiate an abstract test.') from None + raise ValueError('Cannot instantiate an abstract test.') # Set the test parameters in the object cls._set_parameter_space(obj) @@ -806,7 +806,13 @@ def _set_parameter_space(cls, obj): @classmethod def is_abstract_test(cls): - ''' Checks if the test is an abstract test ''' + '''Checks if the test is an abstract test. + + If any of the parameters declared in the test parameter space + (cls._rfm_paramms) has no defined values, the parameter is considered + an abstract parameter. Therefore, a regression test with at least one + abstract parameter is considered an abstract test. + ''' if len(list(filter(lambda x: x == (), cls._rfm_params.values()))) == 0: return False From 2e876e4b5573ca95a5a2c2fefb30fa993088da4c Mon Sep 17 00:00:00 2001 From: Javier Otero Date: Tue, 15 Dec 2020 17:56:01 +0100 Subject: [PATCH 24/43] Improve docstring. --- reframe/core/decorators.py | 3 +- reframe/core/parameters.py | 17 +++++------ reframe/core/pipeline.py | 59 +++++++++++++++++++++++++------------- 3 files changed, 50 insertions(+), 29 deletions(-) diff --git a/reframe/core/decorators.py b/reframe/core/decorators.py index dbe36b3c37..61f8e17dd3 100644 --- a/reframe/core/decorators.py +++ b/reframe/core/decorators.py @@ -88,7 +88,8 @@ def simple_test(cls): ''' _validate_test(cls) - # Register the test as many times as its parameter space length + # Prepare the test's parameter space iterator and register the test as many + # times as the lenght of this iterator. for _ in range(cls.prepare_param_space()): _register_test(cls) diff --git a/reframe/core/parameters.py b/reframe/core/parameters.py index 41a01e1602..0b860da843 100644 --- a/reframe/core/parameters.py +++ b/reframe/core/parameters.py @@ -62,7 +62,7 @@ class LocalParamSpace: Example: In the pseudo-code below, the local parameter space of A is {P0}, and the local parameter space of B is {P1}. However, the final parameter space of A is still {P0}, and the final parameter space of B is {P0, P1}. - + .. code:: python class A(RegressionTest): -> define parameter P0 with value X. @@ -87,7 +87,7 @@ def __setitem__(self, name, value): def add_param(self, name, *values, **kwargs): ''' - Insert a new regression test parameter in the staging area. + Insert a new regression test parameter in the local parameter space. If the parameter is already present in the dictionary, raise an error. See the _TestParameter class for further information on the function arguments. @@ -96,7 +96,7 @@ def add_param(self, name, *values, **kwargs): @property def params(self): - return self._params.items() + return self._params def _merge_parameter_spaces(bases): @@ -106,7 +106,8 @@ def _merge_parameter_spaces(bases): space. This method allows multiple inheritance, as long as a parameter is not doubly defined in two or more different parameter spaces. - :param bases: iterable containing the parent classes. + :param bases: iterable containing the classes from which to merge the + parameter space. :returns: merged parameter space. ''' @@ -124,7 +125,7 @@ def _merge_parameter_spaces(bases): if key in param_space: if not (param_space[key] == () or base_params[key] == ()): - raise NameConflictError(f'Parameter space conflict ' + raise NameConflictError(f'parameter space conflict ' f'(on {key}) due to ' f'multiple inheritance.' ) from None @@ -156,8 +157,8 @@ def _extend_parameter_space(local_param_space, param_space): raise ReframeSyntaxError(f'the local_param_space must be an ' f'instance of the LocalParamSpace class') - # Loop over the staging area. - for name, p in local_param_space.params: + # Loop over the local parameter space. + for name, p in local_param_space.params.items(): param_space[name] = p.filter_params( param_space.get(name, ())) + p.values @@ -191,7 +192,7 @@ def build_parameter_space(cls): trgt_namespace = set(dir(cls)) for key in param_space: if key in trgt_namespace: - raise NameConflictError(f'Attribute {key} clashes with other ' + raise NameConflictError(f'parameter {key} clashes with other ' f'variables present in the namespace ' f'from class {cls.__qualname__}') diff --git a/reframe/core/pipeline.py b/reframe/core/pipeline.py index 70283f4912..bc25a104a8 100644 --- a/reframe/core/pipeline.py +++ b/reframe/core/pipeline.py @@ -711,7 +711,7 @@ def __new__(cls, *args, **kwargs): raise ValueError('Cannot instantiate an abstract test.') # Set the test parameters in the object - cls._set_parameter_space(obj) + cls._set_param_space(obj) # Create a test name from the class name and the constructor's # arguments @@ -746,11 +746,16 @@ def __init__(self): @classmethod def param_space_len(cls): - ''' - Returns the total number of points in the parameter space. - If the RegressionTest has no parameters, the length is 1. - Otherwise, the length is the number of all-to-all combinations - for each of the values in cls._rfm_params. + '''Returns the length of the parameter space iterator. + + Method to efficiently calculate the length of the parameter space + iterator without having to iterate through it. If the RegressionTest + has no parameters, the length is 1. Otherwise, the length is the + number of all-to-all combinations for each of the values in + cls._rfm_params. This length might be used by the reframe decorators to + register the test as many times as points in the parameter space. + + :return: length of the parameter space iterator ''' if not cls._rfm_params: return 1 @@ -762,10 +767,15 @@ def param_space_len(cls): @classmethod def prepare_param_space(cls): - ''' - Creates an iterator to traverse the full parameter space during the - class instantiation, and it also returns the lenght of the parameter - space. This is the number of instances of this class to be created. + '''Creates the parameter space iterator and returns its lenght + + Creates an iterator to traverse the full parameter space later on + during the class instantiation. This iterator covers all possible + combinations of the parameter space. This function might be used by the + reframe decorator to register the tests and obtain the number of times + this class is to be instantiated. + + :return: length of the parameter space iterator ''' if cls._rfm_params: cls._rfm_param_space_iter = itertools.product( @@ -775,21 +785,28 @@ class instantiation, and it also returns the lenght of the parameter return cls.param_space_len() @classmethod - def _set_parameter_space(cls, obj): - ''' - Adds to obj.__dict__ the keys corresponding to the test parameters. + def _set_param_space(cls, obj): + '''Sets the test parameters as class attributes. + + During the object creation, this method inserts the regression test + parameters as object attributes. The values assigned to these test + parameters is obtained from the iterator created by the + :meth `reframe.core.pipeline.prepare_param_space` function above. If + this class were instantiated a number of times greater than the length + of the iterator, the regression test parameters would still be added as + attributes, but equalling None instead. ''' # Don't do anything if the test is not a parametrised test if not cls._rfm_params: return - # Test if the we have the parameter space iterator and set - # the values of the test parameters. + # Test if the parameter space iterator exists and, if so, set the + # values of the test parameters. if hasattr(cls, '_rfm_param_space_iter'): try: tmp = next(cls._rfm_param_space_iter) for index, key in enumerate(cls._rfm_params): - obj.__dict__[key] = tmp[index] + setattr(obj, key, tmp[index]) return @@ -798,11 +815,11 @@ def _set_parameter_space(cls, obj): except StopIteration: del cls._rfm_param_space_iter - # If the param space is not present anymore, it's because we - # have already instantiated the full parameter space. However - # this will get called if the instance is copied. + # If the param space iterator is not present anymore, it's because we + # have already instantiated the full parameter space. However this + # method will get called if the instance is copied. for key in cls._rfm_params: - obj.__dict__[key] = None + setattr(obj, key, None) @classmethod def is_abstract_test(cls): @@ -812,6 +829,8 @@ def is_abstract_test(cls): (cls._rfm_paramms) has no defined values, the parameter is considered an abstract parameter. Therefore, a regression test with at least one abstract parameter is considered an abstract test. + + :return: bool indicating wheteher the test is abstract or not ''' if len(list(filter(lambda x: x == (), cls._rfm_params.values()))) == 0: return False From 26417ef4083e967d4b2335a73ec65cbc89f30393 Mon Sep 17 00:00:00 2001 From: Javier Otero Date: Tue, 15 Dec 2020 18:12:38 +0100 Subject: [PATCH 25/43] Move the assignment of _rfm_params. --- reframe/core/meta.py | 2 +- reframe/core/parameters.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/reframe/core/meta.py b/reframe/core/meta.py index 33384d3583..e99ceb7b04 100644 --- a/reframe/core/meta.py +++ b/reframe/core/meta.py @@ -30,7 +30,7 @@ def __init__(cls, name, bases, namespace, **kwargs): super().__init__(name, bases, namespace, **kwargs) # Set up the regression test parameter space - cls._rfm_params = parameters.build_parameter_space(cls) + parameters.build_parameter_space(cls) # Set up the hooks for the pipeline stages based on the _rfm_attach # attribute; all dependencies will be resolved first in the post-setup diff --git a/reframe/core/parameters.py b/reframe/core/parameters.py index 0b860da843..3bc377bdab 100644 --- a/reframe/core/parameters.py +++ b/reframe/core/parameters.py @@ -196,4 +196,4 @@ def build_parameter_space(cls): f'variables present in the namespace ' f'from class {cls.__qualname__}') - return param_space + setattr(cls, '_rfm_params', param_space) From 9afd2a2b54a0f606a7aaf067551aefc177b1b57d Mon Sep 17 00:00:00 2001 From: Javier Otero Date: Wed, 16 Dec 2020 11:00:56 +0100 Subject: [PATCH 26/43] Improve simple_test implementation. --- reframe/core/decorators.py | 9 ++++---- reframe/core/pipeline.py | 46 +++++++++++++++++++------------------- 2 files changed, 28 insertions(+), 27 deletions(-) diff --git a/reframe/core/decorators.py b/reframe/core/decorators.py index 61f8e17dd3..bf401e499d 100644 --- a/reframe/core/decorators.py +++ b/reframe/core/decorators.py @@ -88,15 +88,16 @@ def simple_test(cls): ''' _validate_test(cls) - # Prepare the test's parameter space iterator and register the test as many - # times as the lenght of this iterator. - for _ in range(cls.prepare_param_space()): + # Prepare the test's parameter space iterator + cls.prepare_param_space() + + # Register the test + for _ in range(cls.param_space_len()): _register_test(cls) return cls - def parameterized_test(*inst): '''Class decorator for registering multiple instantiations of a test class. diff --git a/reframe/core/pipeline.py b/reframe/core/pipeline.py index bc25a104a8..59e72eb1dc 100644 --- a/reframe/core/pipeline.py +++ b/reframe/core/pipeline.py @@ -746,16 +746,15 @@ def __init__(self): @classmethod def param_space_len(cls): - '''Returns the length of the parameter space iterator. + '''Returns the number of all possible parameter combinations. - Method to efficiently calculate the length of the parameter space - iterator without having to iterate through it. If the RegressionTest - has no parameters, the length is 1. Otherwise, the length is the - number of all-to-all combinations for each of the values in - cls._rfm_params. This length might be used by the reframe decorators to - register the test as many times as points in the parameter space. + Method to calculate the test's parameter space length (i.e. the number + of all possible parameter combinations). If the RegressionTest + has no parameters, the length is 1. This method might be used by the + reframe decorators to query the number of times a test should be + registered. - :return: length of the parameter space iterator + :return: length of the parameter space ''' if not cls._rfm_params: return 1 @@ -767,13 +766,17 @@ def param_space_len(cls): @classmethod def prepare_param_space(cls): - '''Creates the parameter space iterator and returns its lenght + '''Creates the parameter space iterator Creates an iterator to traverse the full parameter space later on during the class instantiation. This iterator covers all possible - combinations of the parameter space. This function might be used by the - reframe decorator to register the tests and obtain the number of times - this class is to be instantiated. + parameter combinations. This function might be used by a reframe + decorator to prepare the test for instantiation. + + .. note:: + If this method is not called before the class is instantiated, all + the test parameters will be set to None by + :meth: `reframe.core.pipeline._set_param_space`. :return: length of the parameter space iterator ''' @@ -782,7 +785,7 @@ def prepare_param_space(cls): *(p for p in cls._rfm_params.values()) ) - return cls.param_space_len() + return @classmethod def _set_param_space(cls, obj): @@ -790,11 +793,11 @@ def _set_param_space(cls, obj): During the object creation, this method inserts the regression test parameters as object attributes. The values assigned to these test - parameters is obtained from the iterator created by the - :meth `reframe.core.pipeline.prepare_param_space` function above. If - this class were instantiated a number of times greater than the length - of the iterator, the regression test parameters would still be added as - attributes, but equalling None instead. + parameters are obtained from the iterator created by the + :meth `reframe.core.pipeline.prepare_param_space` method. This iterator + is deleted once it gets exhausted. Instantiating this class without the + iterator being present is allowed. In that case, the test parameters + would simply be initialized as None. ''' # Don't do anything if the test is not a parametrised test if not cls._rfm_params: @@ -810,14 +813,11 @@ def _set_param_space(cls, obj): return - # Delete the iterator if we have exhausted the parameter - # space + # Delete the iterator if exhausted except StopIteration: del cls._rfm_param_space_iter - # If the param space iterator is not present anymore, it's because we - # have already instantiated the full parameter space. However this - # method will get called if the instance is copied. + # If the iterator is not pressent, assign the paramters a default value for key in cls._rfm_params: setattr(obj, key, None) From abc97def792407d482f93bfdd0129cb00aa0858b Mon Sep 17 00:00:00 2001 From: Javier Otero Date: Wed, 16 Dec 2020 11:43:18 +0100 Subject: [PATCH 27/43] Improve pipeline's docstring. --- reframe/core/decorators.py | 2 +- reframe/core/parameters.py | 1 + reframe/core/pipeline.py | 10 +++------- 3 files changed, 5 insertions(+), 8 deletions(-) diff --git a/reframe/core/decorators.py b/reframe/core/decorators.py index bf401e499d..e4ffe4d12c 100644 --- a/reframe/core/decorators.py +++ b/reframe/core/decorators.py @@ -88,7 +88,7 @@ def simple_test(cls): ''' _validate_test(cls) - # Prepare the test's parameter space iterator + # Prepare the test's parameter space cls.prepare_param_space() # Register the test diff --git a/reframe/core/parameters.py b/reframe/core/parameters.py index 3bc377bdab..05d238b28c 100644 --- a/reframe/core/parameters.py +++ b/reframe/core/parameters.py @@ -9,6 +9,7 @@ from reframe.core.exceptions import ReframeSyntaxError, NameConflictError + class _TestParameter: '''Regression test paramter class. diff --git a/reframe/core/pipeline.py b/reframe/core/pipeline.py index 59e72eb1dc..994479025a 100644 --- a/reframe/core/pipeline.py +++ b/reframe/core/pipeline.py @@ -777,23 +777,19 @@ def prepare_param_space(cls): If this method is not called before the class is instantiated, all the test parameters will be set to None by :meth: `reframe.core.pipeline._set_param_space`. - - :return: length of the parameter space iterator ''' if cls._rfm_params: cls._rfm_param_space_iter = itertools.product( *(p for p in cls._rfm_params.values()) ) - return - @classmethod def _set_param_space(cls, obj): '''Sets the test parameters as class attributes. - During the object creation, this method inserts the regression test - parameters as object attributes. The values assigned to these test - parameters are obtained from the iterator created by the + Inserts the regression test parameters as object attributes during + object creation. The values assigned to these test parameters are + obtained from the iterator created by the :meth `reframe.core.pipeline.prepare_param_space` method. This iterator is deleted once it gets exhausted. Instantiating this class without the iterator being present is allowed. In that case, the test parameters From dc80f4f6c8c95fefeaf0c3a78fb55aae92ef74ad Mon Sep 17 00:00:00 2001 From: Javier Otero Date: Wed, 16 Dec 2020 13:35:11 +0100 Subject: [PATCH 28/43] Simplify is_abstract method. --- reframe/core/decorators.py | 6 ++---- reframe/core/pipeline.py | 26 ++++++++++++-------------- 2 files changed, 14 insertions(+), 18 deletions(-) diff --git a/reframe/core/decorators.py b/reframe/core/decorators.py index e4ffe4d12c..6b06b7dc26 100644 --- a/reframe/core/decorators.py +++ b/reframe/core/decorators.py @@ -73,8 +73,8 @@ def _validate_test(cls): 'subclass of RegressionTest') if (cls.is_abstract_test()): - raise ValueError(f'Decorated test ({cls.__qualname__}) is an' - f' abstract test.') + raise ValueError(f'decorated test ({cls.__qualname__}) is an' + f' abstract test') def simple_test(cls): @@ -113,8 +113,6 @@ def parameterized_test(*inst): .. note:: This decorator does not instantiate any test. It only registers them. The actual instantiation happens during the loading phase of the test. - - .. to be deprecated ''' def _do_register(cls): _validate_test(cls) diff --git a/reframe/core/pipeline.py b/reframe/core/pipeline.py index 994479025a..96851d31e9 100644 --- a/reframe/core/pipeline.py +++ b/reframe/core/pipeline.py @@ -706,10 +706,6 @@ def pipeline_hooks(cls): def __new__(cls, *args, **kwargs): obj = super().__new__(cls) - # Abort the class instantiation if the test is an abstract test. - if cls.is_abstract_test(): - raise ValueError('Cannot instantiate an abstract test.') - # Set the test parameters in the object cls._set_param_space(obj) @@ -754,6 +750,10 @@ def param_space_len(cls): reframe decorators to query the number of times a test should be registered. + .. note:: + If the test is an abstract test (i.e. has undefined parameters in + the parameter space), the returned parameter space length is 0. + :return: length of the parameter space ''' if not cls._rfm_params: @@ -774,9 +774,8 @@ def prepare_param_space(cls): decorator to prepare the test for instantiation. .. note:: - If this method is not called before the class is instantiated, all - the test parameters will be set to None by - :meth: `reframe.core.pipeline._set_param_space`. + If the test is an abstract test (i.e. has undefined parameters in + the parameter space), the resulting iterator will be empty. ''' if cls._rfm_params: cls._rfm_param_space_iter = itertools.product( @@ -791,9 +790,8 @@ def _set_param_space(cls, obj): object creation. The values assigned to these test parameters are obtained from the iterator created by the :meth `reframe.core.pipeline.prepare_param_space` method. This iterator - is deleted once it gets exhausted. Instantiating this class without the - iterator being present is allowed. In that case, the test parameters - would simply be initialized as None. + is exhausted or not present, the parameters would simply be initialized + to None. ''' # Don't do anything if the test is not a parametrised test if not cls._rfm_params: @@ -822,16 +820,16 @@ def is_abstract_test(cls): '''Checks if the test is an abstract test. If any of the parameters declared in the test parameter space - (cls._rfm_paramms) has no defined values, the parameter is considered + (cls._rfm_params) has no defined values, the parameter is considered an abstract parameter. Therefore, a regression test with at least one abstract parameter is considered an abstract test. :return: bool indicating wheteher the test is abstract or not ''' - if len(list(filter(lambda x: x == (), cls._rfm_params.values()))) == 0: - return False + if cls.param_space_len() == 0: + return True - return True + return False def _append_parameters_to_name(self): if self._rfm_params: From 98b81d7e405a00296326d04e4f4803dc9a8f01c5 Mon Sep 17 00:00:00 2001 From: Javier Otero Date: Wed, 16 Dec 2020 17:02:55 +0100 Subject: [PATCH 29/43] Add unit tests --- unittests/resources/checks/paramcheck.py | 35 ++++++ unittests/test_parameters.py | 150 +++++++++++++++++++++++ 2 files changed, 185 insertions(+) create mode 100644 unittests/resources/checks/paramcheck.py create mode 100644 unittests/test_parameters.py diff --git a/unittests/resources/checks/paramcheck.py b/unittests/resources/checks/paramcheck.py new file mode 100644 index 0000000000..768dfbae08 --- /dev/null +++ b/unittests/resources/checks/paramcheck.py @@ -0,0 +1,35 @@ +# Copyright 2016-2020 Swiss National Supercomputing Centre (CSCS/ETH Zurich) +# ReFrame Project Developers. See the top-level LICENSE file for details. +# +# SPDX-License-Identifier: BSD-3-Clause + +import reframe as rfm +import reframe.utility.sanity as sn + + +class NoParams(rfm.RunOnlyRegressionTest): + def __init__(self): + self.name = 'noParams' + self.descr = 'Hello World test' + + # All available systems are supported + self.valid_systems = ['*'] + self.valid_prog_environs = ['*'] + self.executable = 'echo "Hello, World!"' + self.tags = {'foo', 'bar'} + self.sanity_patterns = sn.assert_found(r'Hello, World', self.stdout) + self.maintainers = ['JO'] + + +class TwoParams(NoParams): + parameter('P0', 'a') + parameter('P1', 'b') + + +class Abstract(TwoParams): + parameter('P0') + + +class ExtendParams(TwoParams): + parameter('P1', 'c', 'd', 'e', inherit_params=True) + parameter('P2', 'f', 'g') diff --git a/unittests/test_parameters.py b/unittests/test_parameters.py new file mode 100644 index 0000000000..5fa27755de --- /dev/null +++ b/unittests/test_parameters.py @@ -0,0 +1,150 @@ +# Copyright 2016-2020 Swiss National Supercomputing Centre (CSCS/ETH Zurich) +# ReFrame Project Developers. See the top-level LICENSE file for details. +# +# SPDX-License-Identifier: BSD-3-Clause + +from unittests.resources.checks.paramcheck import (NoParams, TwoParams, + Abstract, ExtendParams) + + +def test_param_space_is_empty(): + class MyTest(NoParams): + pass + + assert MyTest._rfm_params == {} + + +def test_params_are_present(): + class MyTest(TwoParams): + pass + + assert MyTest._rfm_params['P0'] == ('a',) + assert MyTest._rfm_params['P1'] == ('b',) + + +def test_param_override(): + class MyTest(TwoParams): + parameter('P1', '-') + + assert MyTest._rfm_params['P0'] == ('a',) + assert MyTest._rfm_params['P1'] == ('-',) + + +def test_param_inheritance(): + class MyTest(TwoParams): + parameter('P1', 'c', inherit_params=True) + + assert MyTest._rfm_params['P0'] == ('a',) + assert MyTest._rfm_params['P1'] == ('b', 'c',) + + +def test_filter_params(): + class MyTest(ExtendParams): + parameter('P1', inherit_params=True, filter_params=lambda x: x[2:]) + + assert MyTest._rfm_params['P0'] == ('a',) + assert MyTest._rfm_params['P1'] == ('d', 'e',) + assert MyTest._rfm_params['P2'] == ('f', 'g',) + + +def test_is_abstract_test(): + class MyTest(Abstract): + pass + + assert MyTest.is_abstract_test() + + +def test_param_len_is_zero(): + class MyTest(Abstract): + pass + + assert MyTest.param_space_len() == 0; + + +def test_extended_param_len(): + class MyTest(ExtendParams): + pass + + assert MyTest.param_space_len() == 8 + + +def test_param_iterator_is_empty(): + class MyTest(Abstract): + @classmethod + def check_param_iterator(cls): + try: + tmp = next(cls._rfm_param_space_iter) + return False + + except StopIteration: + return True + + except: + return False + + MyTest.prepare_param_space() + assert MyTest.check_param_iterator() + + +def test_params_are_none(): + class MyTest(Abstract): + pass + + test = MyTest() + assert test.P0 == None + assert test.P1 == None + + class MyTest(TwoParams): + pass + + test = MyTest() + assert test.P0 == None + assert test.P1 == None + + +def test_param_values_are_not_set(): + class MyTest(Abstract): + pass + + MyTest.prepare_param_space() + test = MyTest() + assert test.P0 == None + assert test.P1 == None + + +def test_param_values_are_set(): + class MyTest(TwoParams): + pass + + MyTest.prepare_param_space() + test = MyTest() + assert test.P0 == 'a' + assert test.P1 == 'b' + + +def test_extended_params(): + class MyTest(ExtendParams): + pass + + test = MyTest() + assert hasattr(test, 'P0') + assert hasattr(test, 'P1') + assert hasattr(test, 'P2') + + +def test_extended_params_are_set(): + class MyTest(ExtendParams): + pass + + MyTest.prepare_param_space() + for i in range(MyTest.param_space_len()): + test = MyTest() + assert test.P0 != None + assert test.P1 != None + assert test.P2 != None + + test = MyTest() + assert test.P0 == None + assert test.P1 == None + assert test.P2 == None + From 871f4b8bf9d564d50d1349d68a5c3fa944e15fa3 Mon Sep 17 00:00:00 2001 From: Javier Otero Date: Thu, 17 Dec 2020 10:12:59 +0100 Subject: [PATCH 30/43] Add code readability improvements. --- reframe/core/parameters.py | 14 +++++++------- reframe/core/pipeline.py | 6 +++--- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/reframe/core/parameters.py b/reframe/core/parameters.py index 05d238b28c..86ad67f587 100644 --- a/reframe/core/parameters.py +++ b/reframe/core/parameters.py @@ -103,7 +103,7 @@ def params(self): def _merge_parameter_spaces(bases): '''Merges the parameter space from multiple classes. - Joins the parameter space of multiple classess into a single parameter + Joins the parameter space of multiple classes into a single parameter space. This method allows multiple inheritance, as long as a parameter is not doubly defined in two or more different parameter spaces. @@ -129,10 +129,10 @@ def _merge_parameter_spaces(bases): raise NameConflictError(f'parameter space conflict ' f'(on {key}) due to ' f'multiple inheritance.' - ) from None + ) from None - param_space[key] = base_params.get( - key, ()) + param_space.get(key, ()) + param_space[key] = (base_params.get(key, ()) + + param_space.get(key, ())) return param_space @@ -155,13 +155,13 @@ def _extend_parameter_space(local_param_space, param_space): ''' # Ensure that the local parameter space is an instance of LocalParamSpace if not isinstance(local_param_space, LocalParamSpace): - raise ReframeSyntaxError(f'the local_param_space must be an ' + raise ReframeSyntaxError(f'local_param_space must be an ' f'instance of the LocalParamSpace class') # Loop over the local parameter space. for name, p in local_param_space.params.items(): - param_space[name] = p.filter_params( - param_space.get(name, ())) + p.values + param_space[name] = (p.filter_params(param_space.get(name, ())) + + p.values) def build_parameter_space(cls): diff --git a/reframe/core/pipeline.py b/reframe/core/pipeline.py index 96851d31e9..20454deff2 100644 --- a/reframe/core/pipeline.py +++ b/reframe/core/pipeline.py @@ -789,9 +789,9 @@ def _set_param_space(cls, obj): Inserts the regression test parameters as object attributes during object creation. The values assigned to these test parameters are obtained from the iterator created by the - :meth `reframe.core.pipeline.prepare_param_space` method. This iterator - is exhausted or not present, the parameters would simply be initialized - to None. + :meth `reframe.core.pipeline.prepare_param_space` method. If this + iterator is exhausted or not present, the parameters would simply be + initialized to None. ''' # Don't do anything if the test is not a parametrised test if not cls._rfm_params: From e6b2377b7707716bbb705ef38b8fe0f5612dfc54 Mon Sep 17 00:00:00 2001 From: Javier Otero Date: Fri, 18 Dec 2020 12:14:14 +0100 Subject: [PATCH 31/43] Address PR comments --- reframe/core/decorators.py | 4 +- reframe/core/parameters.py | 70 ++++++++++++------------ reframe/core/pipeline.py | 61 ++++++++++----------- unittests/resources/checks/paramcheck.py | 35 ------------ unittests/test_parameters.py | 19 ++++++- 5 files changed, 83 insertions(+), 106 deletions(-) delete mode 100644 unittests/resources/checks/paramcheck.py diff --git a/reframe/core/decorators.py b/reframe/core/decorators.py index 6b06b7dc26..541a626a17 100644 --- a/reframe/core/decorators.py +++ b/reframe/core/decorators.py @@ -72,8 +72,8 @@ def _validate_test(cls): raise ReframeSyntaxError('the decorated class must be a ' 'subclass of RegressionTest') - if (cls.is_abstract_test()): - raise ValueError(f'decorated test ({cls.__qualname__}) is an' + if (cls.is_abstract()): + raise ValueError(f'decorated test ({cls.__qualname__!r}) is an' f' abstract test') diff --git a/reframe/core/parameters.py b/reframe/core/parameters.py index 86ad67f587..2870feaf5e 100644 --- a/reframe/core/parameters.py +++ b/reframe/core/parameters.py @@ -7,7 +7,7 @@ # Functionality to build extensible parameterized tests. # -from reframe.core.exceptions import ReframeSyntaxError, NameConflictError +from reframe.core.exceptions import ReframeSyntaxError class _TestParameter: @@ -63,7 +63,9 @@ class LocalParamSpace: Example: In the pseudo-code below, the local parameter space of A is {P0}, and the local parameter space of B is {P1}. However, the final parameter space of A is still {P0}, and the final parameter space of B is {P0, P1}. + .. code:: python + class A(RegressionTest): -> define parameter P0 with value X. @@ -83,12 +85,12 @@ def __setitem__(self, name, value): self._params[name] = value else: raise ValueError( - 'parameter {name} already defined in this class' + f'parameter {name} already defined in this class' ) def add_param(self, name, *values, **kwargs): - ''' - Insert a new regression test parameter in the local parameter space. + '''Insert a new regression test parameter in the local parameter space. + If the parameter is already present in the dictionary, raise an error. See the _TestParameter class for further information on the function arguments. @@ -101,7 +103,7 @@ def params(self): def _merge_parameter_spaces(bases): - '''Merges the parameter space from multiple classes. + '''Merge the parameter space from multiple classes. Joins the parameter space of multiple classes into a single parameter space. This method allows multiple inheritance, as long as a parameter is @@ -123,21 +125,23 @@ def _merge_parameter_spaces(bases): # With multiple inheritance, a single parameter # could be doubly defined and lead to repeated # values. - if key in param_space: - if not (param_space[key] == () or - base_params[key] == ()): - raise NameConflictError(f'parameter space conflict ' - f'(on {key}) due to ' - f'multiple inheritance.' - ) from None - - param_space[key] = (base_params.get(key, ()) + - param_space.get(key, ())) + if (key in param_space + and param_space[key] != () + and base_params[key] != () + ): + raise ReframeSyntaxError(f'parameter space conflict: ' + f' parameter {key!r} already defined ' + f'in {b.__qualname__!r}' + ) + + param_space[key] = ( + base_params.get(key, ()) + param_space.get(key, ()) + ) return param_space -def _extend_parameter_space(local_param_space, param_space): +def _extend_parameter_space(param_space, local_param_space): '''Extend a given parameter space with a local parameter space. Each parameter is dealt with independently, given that each parameter @@ -145,23 +149,23 @@ def _extend_parameter_space(local_param_space, param_space): (see the :class:`reframe.core.parameters_TestParameter` class). - :param local_param_space: a local parameter space from a regression test. - This must be an instance of the class - :class:`reframe.core.parameters.LocalParamSpace`. :param param_space: an existing parameter space. This **must** have been generated with :meth:`reframe.core.parameters._merge_parameter_spaces`. - + :param local_param_space: a local parameter space from a regression test. + This must be an instance of the class + :class:`reframe.core.parameters.LocalParamSpace`. ''' - # Ensure that the local parameter space is an instance of LocalParamSpace - if not isinstance(local_param_space, LocalParamSpace): - raise ReframeSyntaxError(f'local_param_space must be an ' - f'instance of the LocalParamSpace class') + # The argument local_param_space must be an instance of LocalParamSpace + assert isinstance(local_param_space, LocalParamSpace) # Loop over the local parameter space. for name, p in local_param_space.params.items(): - param_space[name] = (p.filter_params(param_space.get(name, ())) + - p.values) + param_space[name] = ( + p.filter_params(param_space.get(name, ())) + p.values + ) + + return param_space def build_parameter_space(cls): @@ -184,17 +188,15 @@ def build_parameter_space(cls): parameter names and the values are tuples containing all the values for each of the parameters. ''' - # Inherit the parameter space from the base classes - param_space = _merge_parameter_spaces(cls.__bases__) - - # Extend the parameter space with the local parameter space - _extend_parameter_space(cls._rfm_local_param_space, param_space) + param_space = _extend_parameter_space( + _merge_parameter_spaces(cls.__bases__), cls._rfm_local_param_space + ) trgt_namespace = set(dir(cls)) for key in param_space: if key in trgt_namespace: - raise NameConflictError(f'parameter {key} clashes with other ' - f'variables present in the namespace ' - f'from class {cls.__qualname__}') + raise ReframeSyntaxError(f'parameter {key!r} clashes with other ' + f'variables present in the namespace ' + f'from class {cls.__qualname__!r}') setattr(cls, '_rfm_params', param_space) diff --git a/reframe/core/pipeline.py b/reframe/core/pipeline.py index 20454deff2..111da104f2 100644 --- a/reframe/core/pipeline.py +++ b/reframe/core/pipeline.py @@ -707,7 +707,7 @@ def __new__(cls, *args, **kwargs): obj = super().__new__(cls) # Set the test parameters in the object - cls._set_param_space(obj) + cls._construct_params(obj) # Create a test name from the class name and the constructor's # arguments @@ -746,7 +746,7 @@ def param_space_len(cls): Method to calculate the test's parameter space length (i.e. the number of all possible parameter combinations). If the RegressionTest - has no parameters, the length is 1. This method might be used by the + has no parameters, the length is 1. This method may be used by the reframe decorators to query the number of times a test should be registered. @@ -764,26 +764,31 @@ def param_space_len(cls): (len(p) for p in cls._rfm_params.values()) ) + @classmethod + def walk_param_space(cls): + '''Create a generator object to iterate over the parameter space + + :return: generator object to iterate over the parameter space. + ''' + yield from itertools.product(*(p for p in cls._rfm_params.values())) + @classmethod def prepare_param_space(cls): '''Creates the parameter space iterator Creates an iterator to traverse the full parameter space later on during the class instantiation. This iterator covers all possible - parameter combinations. This function might be used by a reframe + parameter combinations. This function may be used by a reframe decorator to prepare the test for instantiation. .. note:: If the test is an abstract test (i.e. has undefined parameters in the parameter space), the resulting iterator will be empty. ''' - if cls._rfm_params: - cls._rfm_param_space_iter = itertools.product( - *(p for p in cls._rfm_params.values()) - ) + cls._rfm_param_space_iter = cls.walk_param_space() @classmethod - def _set_param_space(cls, obj): + def _construct_params(cls, obj): '''Sets the test parameters as class attributes. Inserts the regression test parameters as object attributes during @@ -792,31 +797,24 @@ def _set_param_space(cls, obj): :meth `reframe.core.pipeline.prepare_param_space` method. If this iterator is exhausted or not present, the parameters would simply be initialized to None. - ''' - # Don't do anything if the test is not a parametrised test - if not cls._rfm_params: - return - - # Test if the parameter space iterator exists and, if so, set the - # values of the test parameters. - if hasattr(cls, '_rfm_param_space_iter'): - try: - tmp = next(cls._rfm_param_space_iter) - for index, key in enumerate(cls._rfm_params): - setattr(obj, key, tmp[index]) - return + :meta private: + ''' + # Try to set the values of the test parameters from the param iterator. + try: + param_values = next(cls._rfm_param_space_iter) + for index, key in enumerate(cls._rfm_params): + setattr(obj, key, param_values[index]) - # Delete the iterator if exhausted - except StopIteration: - del cls._rfm_param_space_iter + return - # If the iterator is not pressent, assign the paramters a default value - for key in cls._rfm_params: - setattr(obj, key, None) + # Initialize the params as None if an exception was raised + except: + for key in cls._rfm_params: + setattr(obj, key, None) @classmethod - def is_abstract_test(cls): + def is_abstract(cls): '''Checks if the test is an abstract test. If any of the parameters declared in the test parameter space @@ -825,11 +823,10 @@ def is_abstract_test(cls): abstract parameter is considered an abstract test. :return: bool indicating wheteher the test is abstract or not - ''' - if cls.param_space_len() == 0: - return True - return False + :meta private: + ''' + return cls.param_space_len() == 0 def _append_parameters_to_name(self): if self._rfm_params: diff --git a/unittests/resources/checks/paramcheck.py b/unittests/resources/checks/paramcheck.py deleted file mode 100644 index 768dfbae08..0000000000 --- a/unittests/resources/checks/paramcheck.py +++ /dev/null @@ -1,35 +0,0 @@ -# Copyright 2016-2020 Swiss National Supercomputing Centre (CSCS/ETH Zurich) -# ReFrame Project Developers. See the top-level LICENSE file for details. -# -# SPDX-License-Identifier: BSD-3-Clause - -import reframe as rfm -import reframe.utility.sanity as sn - - -class NoParams(rfm.RunOnlyRegressionTest): - def __init__(self): - self.name = 'noParams' - self.descr = 'Hello World test' - - # All available systems are supported - self.valid_systems = ['*'] - self.valid_prog_environs = ['*'] - self.executable = 'echo "Hello, World!"' - self.tags = {'foo', 'bar'} - self.sanity_patterns = sn.assert_found(r'Hello, World', self.stdout) - self.maintainers = ['JO'] - - -class TwoParams(NoParams): - parameter('P0', 'a') - parameter('P1', 'b') - - -class Abstract(TwoParams): - parameter('P0') - - -class ExtendParams(TwoParams): - parameter('P1', 'c', 'd', 'e', inherit_params=True) - parameter('P2', 'f', 'g') diff --git a/unittests/test_parameters.py b/unittests/test_parameters.py index 5fa27755de..05e6f000ce 100644 --- a/unittests/test_parameters.py +++ b/unittests/test_parameters.py @@ -3,8 +3,22 @@ # # SPDX-License-Identifier: BSD-3-Clause -from unittests.resources.checks.paramcheck import (NoParams, TwoParams, - Abstract, ExtendParams) + +class NoParams(rfm.RunOnlyRegressionTest): + pass + +class TwoParams(NoParams): + parameter('P0', 'a') + parameter('P1', 'b') + + +class Abstract(TwoParams): + parameter('P0') + + +class ExtendParams(TwoParams): + parameter('P1', 'c', 'd', 'e', inherit_params=True) + parameter('P2', 'f', 'g') def test_param_space_is_empty(): @@ -147,4 +161,3 @@ class MyTest(ExtendParams): assert test.P0 == None assert test.P1 == None assert test.P2 == None - From 3749fda697f920dcf2ee8da897852818bca03257 Mon Sep 17 00:00:00 2001 From: Javier Otero Date: Fri, 18 Dec 2020 12:17:54 +0100 Subject: [PATCH 32/43] Bugfix unit tests --- unittests/test_parameters.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/unittests/test_parameters.py b/unittests/test_parameters.py index 05e6f000ce..b2197f4329 100644 --- a/unittests/test_parameters.py +++ b/unittests/test_parameters.py @@ -4,6 +4,9 @@ # SPDX-License-Identifier: BSD-3-Clause +import reframe as rfm + + class NoParams(rfm.RunOnlyRegressionTest): pass @@ -65,7 +68,7 @@ def test_is_abstract_test(): class MyTest(Abstract): pass - assert MyTest.is_abstract_test() + assert MyTest.is_abstract() def test_param_len_is_zero(): From 0c07a236628e3aeb4d4d22008f525217f36fd24c Mon Sep 17 00:00:00 2001 From: Javier Otero Date: Fri, 18 Dec 2020 14:27:21 +0100 Subject: [PATCH 33/43] Fix PEP8 issues. --- reframe/core/meta.py | 5 ++++ reframe/core/parameters.py | 11 ++++---- reframe/core/pipeline.py | 2 +- unittests/test_parameters.py | 52 +++++++++++++++++++++--------------- 4 files changed, 41 insertions(+), 29 deletions(-) diff --git a/reframe/core/meta.py b/reframe/core/meta.py index e99ceb7b04..7c92a8408a 100644 --- a/reframe/core/meta.py +++ b/reframe/core/meta.py @@ -77,3 +77,8 @@ def __init__(cls, name, bases, namespace, **kwargs): f"'{b.__qualname__}.{v.__name__}'; " f"you should use the pipeline hooks instead") raise ReframeSyntaxError(msg) + + # Make the parameter space available as read-only + @property + def param_space(cls): + return cls._rfm_params diff --git a/reframe/core/parameters.py b/reframe/core/parameters.py index 2870feaf5e..cadaae5c19 100644 --- a/reframe/core/parameters.py +++ b/reframe/core/parameters.py @@ -125,14 +125,13 @@ def _merge_parameter_spaces(bases): # With multiple inheritance, a single parameter # could be doubly defined and lead to repeated # values. - if (key in param_space - and param_space[key] != () - and base_params[key] != () - ): + if (key in param_space and ( + param_space[key] != () and base_params[key] != () + )): + raise ReframeSyntaxError(f'parameter space conflict: ' f' parameter {key!r} already defined ' - f'in {b.__qualname__!r}' - ) + f'in {b.__qualname__!r}') param_space[key] = ( base_params.get(key, ()) + param_space.get(key, ()) diff --git a/reframe/core/pipeline.py b/reframe/core/pipeline.py index 111da104f2..0f2123503d 100644 --- a/reframe/core/pipeline.py +++ b/reframe/core/pipeline.py @@ -809,7 +809,7 @@ def _construct_params(cls, obj): return # Initialize the params as None if an exception was raised - except: + except Exception: for key in cls._rfm_params: setattr(obj, key, None) diff --git a/unittests/test_parameters.py b/unittests/test_parameters.py index b2197f4329..b24d3784aa 100644 --- a/unittests/test_parameters.py +++ b/unittests/test_parameters.py @@ -10,6 +10,7 @@ class NoParams(rfm.RunOnlyRegressionTest): pass + class TwoParams(NoParams): parameter('P0', 'a') parameter('P1', 'b') @@ -28,40 +29,40 @@ def test_param_space_is_empty(): class MyTest(NoParams): pass - assert MyTest._rfm_params == {} + assert MyTest.param_space == {} def test_params_are_present(): class MyTest(TwoParams): pass - assert MyTest._rfm_params['P0'] == ('a',) - assert MyTest._rfm_params['P1'] == ('b',) + assert MyTest.param_space['P0'] == ('a',) + assert MyTest.param_space['P1'] == ('b',) def test_param_override(): class MyTest(TwoParams): parameter('P1', '-') - assert MyTest._rfm_params['P0'] == ('a',) - assert MyTest._rfm_params['P1'] == ('-',) + assert MyTest.param_space['P0'] == ('a',) + assert MyTest.param_space['P1'] == ('-',) def test_param_inheritance(): class MyTest(TwoParams): parameter('P1', 'c', inherit_params=True) - assert MyTest._rfm_params['P0'] == ('a',) - assert MyTest._rfm_params['P1'] == ('b', 'c',) + assert MyTest.param_space['P0'] == ('a',) + assert MyTest.param_space['P1'] == ('b', 'c',) def test_filter_params(): class MyTest(ExtendParams): parameter('P1', inherit_params=True, filter_params=lambda x: x[2:]) - assert MyTest._rfm_params['P0'] == ('a',) - assert MyTest._rfm_params['P1'] == ('d', 'e',) - assert MyTest._rfm_params['P2'] == ('f', 'g',) + assert MyTest.param_space['P0'] == ('a',) + assert MyTest.param_space['P1'] == ('d', 'e',) + assert MyTest.param_space['P2'] == ('f', 'g',) def test_is_abstract_test(): @@ -71,6 +72,13 @@ class MyTest(Abstract): assert MyTest.is_abstract() +def test_is_not_abstract_test(): + class MyTest(TwoParams): + pass + + assert not MyTest.is_abstract() + + def test_param_len_is_zero(): class MyTest(Abstract): pass @@ -108,15 +116,15 @@ class MyTest(Abstract): pass test = MyTest() - assert test.P0 == None - assert test.P1 == None + assert test.P0 is None + assert test.P1 is None class MyTest(TwoParams): pass test = MyTest() - assert test.P0 == None - assert test.P1 == None + assert test.P0 is None + assert test.P1 is None def test_param_values_are_not_set(): @@ -125,8 +133,8 @@ class MyTest(Abstract): MyTest.prepare_param_space() test = MyTest() - assert test.P0 == None - assert test.P1 == None + assert test.P0 is None + assert test.P1 is None def test_param_values_are_set(): @@ -156,11 +164,11 @@ class MyTest(ExtendParams): MyTest.prepare_param_space() for i in range(MyTest.param_space_len()): test = MyTest() - assert test.P0 != None - assert test.P1 != None - assert test.P2 != None + assert test.P0 is not None + assert test.P1 is not None + assert test.P2 is not None test = MyTest() - assert test.P0 == None - assert test.P1 == None - assert test.P2 == None + assert test.P0 is None + assert test.P1 is None + assert test.P2 is None From 7e6c652ee9d3255993a1083cb004bdf1424f2e73 Mon Sep 17 00:00:00 2001 From: Javier Otero Date: Fri, 18 Dec 2020 19:31:01 +0100 Subject: [PATCH 34/43] Create ParamSpace class --- reframe/core/decorators.py | 7 +- reframe/core/meta.py | 23 +++- reframe/core/parameters.py | 201 +++++++++++++++++++++-------------- reframe/core/pipeline.py | 83 ++------------- unittests/test_parameters.py | 49 ++------- 5 files changed, 159 insertions(+), 204 deletions(-) diff --git a/reframe/core/decorators.py b/reframe/core/decorators.py index 541a626a17..12d738a6d1 100644 --- a/reframe/core/decorators.py +++ b/reframe/core/decorators.py @@ -72,7 +72,7 @@ def _validate_test(cls): raise ReframeSyntaxError('the decorated class must be a ' 'subclass of RegressionTest') - if (cls.is_abstract()): + if (cls.is_abstract): raise ValueError(f'decorated test ({cls.__qualname__!r}) is an' f' abstract test') @@ -88,11 +88,8 @@ def simple_test(cls): ''' _validate_test(cls) - # Prepare the test's parameter space - cls.prepare_param_space() - # Register the test - for _ in range(cls.param_space_len()): + for _ in range(len(cls.param_space)): _register_test(cls) return cls diff --git a/reframe/core/meta.py b/reframe/core/meta.py index 7c92a8408a..0a813a9e1c 100644 --- a/reframe/core/meta.py +++ b/reframe/core/meta.py @@ -7,6 +7,7 @@ # Meta-class for creating regression tests. # + from reframe.core.exceptions import ReframeSyntaxError import reframe.core.parameters as parameters @@ -29,8 +30,8 @@ def __prepare__(cls, name, bases, **kwargs): def __init__(cls, name, bases, namespace, **kwargs): super().__init__(name, bases, namespace, **kwargs) - # Set up the regression test parameter space - parameters.build_parameter_space(cls) + # Build the regression test parameter space + cls._rfm_param_space = parameters.ParamSpace(cls) # Set up the hooks for the pipeline stages based on the _rfm_attach # attribute; all dependencies will be resolved first in the post-setup @@ -78,7 +79,21 @@ def __init__(cls, name, bases, namespace, **kwargs): f"you should use the pipeline hooks instead") raise ReframeSyntaxError(msg) - # Make the parameter space available as read-only @property def param_space(cls): - return cls._rfm_params + # Make the parameter space available as read-only + return cls._rfm_param_space + + @property + def is_abstract(cls): + '''Checks if the test is an abstract test. + + If the parameter space has undefined parameters, the test is considered + an abstract test. If that is the case, the length of the parameter + space is just 0. + + :return: bool indicating wheteher the test is abstract or not + + :meta private: + ''' + return len(cls.param_space) == 0 diff --git a/reframe/core/parameters.py b/reframe/core/parameters.py index cadaae5c19..64533e93bb 100644 --- a/reframe/core/parameters.py +++ b/reframe/core/parameters.py @@ -7,6 +7,9 @@ # Functionality to build extensible parameterized tests. # +import functools +import itertools + from reframe.core.exceptions import ReframeSyntaxError @@ -85,7 +88,7 @@ def __setitem__(self, name, value): self._params[name] = value else: raise ValueError( - f'parameter {name} already defined in this class' + f'parameter {name!r} already defined in this class' ) def add_param(self, name, *values, **kwargs): @@ -101,101 +104,141 @@ def add_param(self, name, *values, **kwargs): def params(self): return self._params - -def _merge_parameter_spaces(bases): - '''Merge the parameter space from multiple classes. - - Joins the parameter space of multiple classes into a single parameter - space. This method allows multiple inheritance, as long as a parameter is - not doubly defined in two or more different parameter spaces. - - :param bases: iterable containing the classes from which to merge the - parameter space. - - :returns: merged parameter space. + def items(self): + return self._params.items() + + +class ParamSpace: + ''' Regression test parameter space + + Host class for the parameter space of a regresion test. The parameter + space is stored as a dictionary (self._params), where the keys are the + parameter names and the values are tuples with all the available values + for each parameter. The __init__ method in this class takes an optional + argument (target_class), which is the regression test class where the + parameter space is to be built. If this target class is provided, the + __init__ method performs three main steps. These are (in order of exec) + the inheritance of the parameter spaces from the direct parent classes, + the extension of the inherited parameter space with the local parameter + space (this must be an instance of + :class `reframe.core.parameters.LocalParamSpace`), and lastly, a check to + ensure that none of the parameter names clashes with any of the class + attributes existing in the target class. If no target class is provided, + the parameter space is initialized as empty. After the parameter space is + set, a parameter space iterator is created, which allows traversing the + full parameter space walking though all posible parameter combinations. + Since this class is iterable, this may be used by the RegressionTest + constructor to assing the values to the test parameters. Note that the + length of this iterator matches the value returned by the member function + __len__. + + :param target_cls: the class where the full parameter space is to be built. + + .. note:: + The __init__ method is aware of the implementation details of the + regression test metaclass. This is required to retrieve the parameter + spaces from the base classes, and also the local parameter space from + the target class. ''' - # Temporary dict where we build the parameter space from the base - # classes - param_space = {} - - # Iterate over the base classes and inherit the parameter space - for b in bases: - base_params = getattr(b, '_rfm_params', ()) - for key in base_params: + def __init__(self, target_cls=None): + self._params = {} + + # If a target class is provided, build the param space for it + if target_cls: + + # Inherit the parameter spaces from the direct parent classes + for base in filter(lambda x: hasattr(x, 'param_space'), + target_cls.__bases__): + self.join(base._rfm_param_space) + + # Extend the parameter space with the local parameter space + try: + for name, p in target_cls._rfm_local_param_space.items(): + self._params[name] = ( + p.filter_params(self._params.get(name, ())) + p.values + ) + except AttributeError: + pass + + # Make sure there is none of the parameters clashes with the target + # class namespace + target_namespace = set(dir(target_cls)) + for key in self._params: + if key in target_namespace: + raise ReframeSyntaxError( + f'parameter {key!r} clashes with other variables' + f' present in the namespace from class ' + f'{target_cls.__qualname__!r}' + ) + + # Initialize the parameter space iterator + self._iter = self.param_space_iterator() + + def join(self, other): + '''Join two parameter spaces into one + + Join two different parameter spaces into a single one. Both parameter + spaces must be an instance ot the ParamSpace class. This method will + raise an error if a parameter is defined in the two parameter spaces + to be merged. + + :param other: instance of the ParamSpace class + ''' + for key in other.params: # With multiple inheritance, a single parameter # could be doubly defined and lead to repeated - # values. - if (key in param_space and ( - param_space[key] != () and base_params[key] != () + # values + if (key in self._params and ( + self._params[key] != () and other.params[key] != () )): raise ReframeSyntaxError(f'parameter space conflict: ' - f' parameter {key!r} already defined ' + f'parameter {key!r} already defined ' f'in {b.__qualname__!r}') - param_space[key] = ( - base_params.get(key, ()) + param_space.get(key, ()) + self._params[key] = ( + other.params.get(key, ()) + self._params.get(key, ()) ) - return param_space + def param_space_iterator(self): + '''Create a generator object to iterate over the parameter space + :return: generator object to iterate over the parameter space. + ''' + yield from itertools.product(*(p for p in self._params.values())) -def _extend_parameter_space(param_space, local_param_space): - '''Extend a given parameter space with a local parameter space. - - Each parameter is dealt with independently, given that each parameter - has its own inheritance behaviour defined in the local parameter space - (see the - :class:`reframe.core.parameters_TestParameter` class). + @property + def params(self): + return self._params - :param param_space: an existing parameter space. This **must** have been - generated with - :meth:`reframe.core.parameters._merge_parameter_spaces`. - :param local_param_space: a local parameter space from a regression test. - This must be an instance of the class - :class:`reframe.core.parameters.LocalParamSpace`. - ''' - # The argument local_param_space must be an instance of LocalParamSpace - assert isinstance(local_param_space, LocalParamSpace) + def __len__(self): + '''Returns the number of all possible parameter combinations. - # Loop over the local parameter space. - for name, p in local_param_space.params.items(): - param_space[name] = ( - p.filter_params(param_space.get(name, ())) + p.values - ) + Method to calculate the test's parameter space length (i.e. the number + of all possible parameter combinations). If the RegressionTest + has no parameters, the length is 1. - return param_space + .. note:: + If the test is an abstract test (i.e. has undefined parameters in + the parameter space), the returned parameter space length is 0. + :return: length of the parameter space + ''' + if not self._params: + return 1 -def build_parameter_space(cls): - ''' Builder of the full parameter space of a regression test. + return functools.reduce( + lambda x, y: x*y, + (len(p) for p in self._params.values()) + ) - Handles the full parameter space build, inheriting the parameter spaces - form the base classes, and extending these with the local parameter space - of the class (stored in cls._rfm_local_param_space). This method is called - during the class object initialization (i.e. the __init__ method of the - regression test metaclass). This method has three main steps, which are - (in order of execution) the inheritance of the parameter spaces from the - base classes, the extension of the inherited parameter space with the - local parameter space, and lastly, a check to ensure that none of the - parameter names clashes with any of the class attributes existing in the - regression test class. + def __next__(self): + # Make the class iterable + return next(self._iter) - :param cls: the class where the full parameter space is to be built. + def __getitem__(self, key): + return self._params.get(key, ()) - :returns: dictionary containing the full parameter space. The keys are the - parameter names and the values are tuples containing all the values - for each of the parameters. - ''' - param_space = _extend_parameter_space( - _merge_parameter_spaces(cls.__bases__), cls._rfm_local_param_space - ) - - trgt_namespace = set(dir(cls)) - for key in param_space: - if key in trgt_namespace: - raise ReframeSyntaxError(f'parameter {key!r} clashes with other ' - f'variables present in the namespace ' - f'from class {cls.__qualname__!r}') - - setattr(cls, '_rfm_params', param_space) + @property + def is_empty(self): + return self._params == {} diff --git a/reframe/core/pipeline.py b/reframe/core/pipeline.py index 0f2123503d..c5f245342c 100644 --- a/reframe/core/pipeline.py +++ b/reframe/core/pipeline.py @@ -740,98 +740,35 @@ def __new__(cls, *args, **kwargs): def __init__(self): pass - @classmethod - def param_space_len(cls): - '''Returns the number of all possible parameter combinations. - - Method to calculate the test's parameter space length (i.e. the number - of all possible parameter combinations). If the RegressionTest - has no parameters, the length is 1. This method may be used by the - reframe decorators to query the number of times a test should be - registered. - - .. note:: - If the test is an abstract test (i.e. has undefined parameters in - the parameter space), the returned parameter space length is 0. - - :return: length of the parameter space - ''' - if not cls._rfm_params: - return 1 - - return functools.reduce( - lambda x, y: x*y, - (len(p) for p in cls._rfm_params.values()) - ) - - @classmethod - def walk_param_space(cls): - '''Create a generator object to iterate over the parameter space - - :return: generator object to iterate over the parameter space. - ''' - yield from itertools.product(*(p for p in cls._rfm_params.values())) - - @classmethod - def prepare_param_space(cls): - '''Creates the parameter space iterator - - Creates an iterator to traverse the full parameter space later on - during the class instantiation. This iterator covers all possible - parameter combinations. This function may be used by a reframe - decorator to prepare the test for instantiation. - - .. note:: - If the test is an abstract test (i.e. has undefined parameters in - the parameter space), the resulting iterator will be empty. - ''' - cls._rfm_param_space_iter = cls.walk_param_space() - @classmethod def _construct_params(cls, obj): - '''Sets the test parameters as class attributes. + '''Attaches the test parameters as class attributes. Inserts the regression test parameters as object attributes during object creation. The values assigned to these test parameters are - obtained from the iterator created by the - :meth `reframe.core.pipeline.prepare_param_space` method. If this - iterator is exhausted or not present, the parameters would simply be - initialized to None. + obtained from the iterator in the test's parameter space (see + :class `reframe.core.parameters.ParamSpace`). When this iterator is + exhausted, the parameters will simply be initialized to None. :meta private: ''' # Try to set the values of the test parameters from the param iterator. try: - param_values = next(cls._rfm_param_space_iter) - for index, key in enumerate(cls._rfm_params): + param_values = next(cls._rfm_param_space) + for index, key in enumerate(cls._rfm_param_space.params): setattr(obj, key, param_values[index]) return # Initialize the params as None if an exception was raised - except Exception: - for key in cls._rfm_params: + except StopIteration: + for key in cls._rfm_param_space.params: setattr(obj, key, None) - @classmethod - def is_abstract(cls): - '''Checks if the test is an abstract test. - - If any of the parameters declared in the test parameter space - (cls._rfm_params) has no defined values, the parameter is considered - an abstract parameter. Therefore, a regression test with at least one - abstract parameter is considered an abstract test. - - :return: bool indicating wheteher the test is abstract or not - - :meta private: - ''' - return cls.param_space_len() == 0 - def _append_parameters_to_name(self): - if self._rfm_params: + if self._rfm_param_space.params: return '_' + '_'.join([str(self.__dict__[key]) - for key in self._rfm_params]) + for key in self._rfm_param_space.params]) else: return '' diff --git a/unittests/test_parameters.py b/unittests/test_parameters.py index b24d3784aa..214dbaa88b 100644 --- a/unittests/test_parameters.py +++ b/unittests/test_parameters.py @@ -29,7 +29,7 @@ def test_param_space_is_empty(): class MyTest(NoParams): pass - assert MyTest.param_space == {} + assert MyTest.param_space.is_empty def test_params_are_present(): @@ -69,69 +69,34 @@ def test_is_abstract_test(): class MyTest(Abstract): pass - assert MyTest.is_abstract() + assert MyTest.is_abstract def test_is_not_abstract_test(): class MyTest(TwoParams): pass - assert not MyTest.is_abstract() + assert not MyTest.is_abstract def test_param_len_is_zero(): class MyTest(Abstract): pass - assert MyTest.param_space_len() == 0; + assert len(MyTest.param_space) == 0; def test_extended_param_len(): class MyTest(ExtendParams): pass - assert MyTest.param_space_len() == 8 - - -def test_param_iterator_is_empty(): - class MyTest(Abstract): - @classmethod - def check_param_iterator(cls): - try: - tmp = next(cls._rfm_param_space_iter) - return False - - except StopIteration: - return True - - except: - return False - - MyTest.prepare_param_space() - assert MyTest.check_param_iterator() - - -def test_params_are_none(): - class MyTest(Abstract): - pass - - test = MyTest() - assert test.P0 is None - assert test.P1 is None - - class MyTest(TwoParams): - pass - - test = MyTest() - assert test.P0 is None - assert test.P1 is None + assert len(MyTest.param_space) == 8 def test_param_values_are_not_set(): class MyTest(Abstract): pass - MyTest.prepare_param_space() test = MyTest() assert test.P0 is None assert test.P1 is None @@ -141,7 +106,6 @@ def test_param_values_are_set(): class MyTest(TwoParams): pass - MyTest.prepare_param_space() test = MyTest() assert test.P0 == 'a' assert test.P1 == 'b' @@ -161,8 +125,7 @@ def test_extended_params_are_set(): class MyTest(ExtendParams): pass - MyTest.prepare_param_space() - for i in range(MyTest.param_space_len()): + for i in range(len(MyTest.param_space)): test = MyTest() assert test.P0 is not None assert test.P1 is not None From 9c31fe5108cf6422b9780f9d49ae8c292364bdc3 Mon Sep 17 00:00:00 2001 From: Javier Otero Date: Mon, 21 Dec 2020 16:25:41 +0100 Subject: [PATCH 35/43] Improve parameter space consumption --- reframe/core/decorators.py | 24 +++++++++++----- reframe/core/meta.py | 11 +++++++- reframe/core/parameters.py | 55 ++++++++++++++++++++---------------- reframe/core/pipeline.py | 40 +++++++++++++++----------- unittests/test_parameters.py | 51 ++++++++++++++++++--------------- 5 files changed, 109 insertions(+), 72 deletions(-) diff --git a/reframe/core/decorators.py b/reframe/core/decorators.py index 12d738a6d1..e3fbc16c7d 100644 --- a/reframe/core/decorators.py +++ b/reframe/core/decorators.py @@ -27,13 +27,22 @@ def _register_test(cls, args=None): + '''Register the test + + Register the test with _rfm_use_params=True. This additional argument flags + this case to consume the parameter space. Otherwise, the regression test + parameters would simply be initialized to None. + + :meta private: + ''' def _instantiate(cls, args): if isinstance(args, collections.abc.Sequence): - return cls(*args) + return cls(*args, _rfm_use_params=True) elif isinstance(args, collections.abc.Mapping): + args['_rfm_use_params'] = True return cls(**args) elif args is None: - return cls() + return cls(_rfm_use_params=True) def _instantiate_all(): ret = [] @@ -72,7 +81,7 @@ def _validate_test(cls): raise ReframeSyntaxError('the decorated class must be a ' 'subclass of RegressionTest') - if (cls.is_abstract): + if (cls.is_abstract()): raise ValueError(f'decorated test ({cls.__qualname__!r}) is an' f' abstract test') @@ -88,8 +97,7 @@ def simple_test(cls): ''' _validate_test(cls) - # Register the test - for _ in range(len(cls.param_space)): + for _ in cls.param_space: _register_test(cls) return cls @@ -113,8 +121,10 @@ def parameterized_test(*inst): ''' def _do_register(cls): _validate_test(cls) - for args in inst: - _register_test(cls, args) + + for _ in cls.param_space: + for args in inst: + _register_test(cls, args) return cls diff --git a/reframe/core/meta.py b/reframe/core/meta.py index 0a813a9e1c..7f551aa3bf 100644 --- a/reframe/core/meta.py +++ b/reframe/core/meta.py @@ -79,12 +79,21 @@ def __init__(cls, name, bases, namespace, **kwargs): f"you should use the pipeline hooks instead") raise ReframeSyntaxError(msg) + def __call__(cls, *args, **kwargs): + obj = cls.__new__(cls, *args, **kwargs) + + # Intercept constructor arguments + if '_rfm_use_params' in kwargs: + del kwargs['_rfm_use_params'] + + obj.__init__(*args, **kwargs) + return obj + @property def param_space(cls): # Make the parameter space available as read-only return cls._rfm_param_space - @property def is_abstract(cls): '''Checks if the test is an abstract test. diff --git a/reframe/core/parameters.py b/reframe/core/parameters.py index 64533e93bb..c2e0e0670a 100644 --- a/reframe/core/parameters.py +++ b/reframe/core/parameters.py @@ -125,12 +125,17 @@ class ParamSpace: ensure that none of the parameter names clashes with any of the class attributes existing in the target class. If no target class is provided, the parameter space is initialized as empty. After the parameter space is - set, a parameter space iterator is created, which allows traversing the - full parameter space walking though all posible parameter combinations. - Since this class is iterable, this may be used by the RegressionTest - constructor to assing the values to the test parameters. Note that the - length of this iterator matches the value returned by the member function - __len__. + set, a parameter space iterator is created under self.__unique_iter, which + acts as an internal control variable that tracks the usage of this + parameter space. This iterator walks through all possible parameter + combinations and cannot be restored after reaching exhaustion. This unique + iterator is made available as read-only through cls.unique_iterator and it + may be used by an external class to track the usage of the parameter space + (i.e. the + :class `reframe.core.pipeline.RegressionTest` can use this unique iterator + to ensure that each point of the parameter space has only been instantiated + once). The length of this iterator matches the value returned by the member + function __len__. :param target_cls: the class where the full parameter space is to be built. @@ -145,6 +150,9 @@ def __init__(self, target_cls=None): # If a target class is provided, build the param space for it if target_cls: + assert hasattr(target_cls, '_rfm_local_param_space') + assert isinstance(target_cls._rfm_local_param_space, + LocalParamSpace) # Inherit the parameter spaces from the direct parent classes for base in filter(lambda x: hasattr(x, 'param_space'), @@ -152,15 +160,12 @@ def __init__(self, target_cls=None): self.join(base._rfm_param_space) # Extend the parameter space with the local parameter space - try: - for name, p in target_cls._rfm_local_param_space.items(): - self._params[name] = ( - p.filter_params(self._params.get(name, ())) + p.values - ) - except AttributeError: - pass + for name, p in target_cls._rfm_local_param_space.items(): + self._params[name] = ( + p.filter_params(self._params.get(name, ())) + p.values + ) - # Make sure there is none of the parameters clashes with the target + # Make sure that none of the parameters clashes with the target # class namespace target_namespace = set(dir(target_cls)) for key in self._params: @@ -171,8 +176,8 @@ def __init__(self, target_cls=None): f'{target_cls.__qualname__!r}' ) - # Initialize the parameter space iterator - self._iter = self.param_space_iterator() + # Internal parameter space usage tracker + self.__unique_iter = iter(self) def join(self, other): '''Join two parameter spaces into one @@ -188,9 +193,9 @@ def join(self, other): # With multiple inheritance, a single parameter # could be doubly defined and lead to repeated # values - if (key in self._params and ( - self._params[key] != () and other.params[key] != () - )): + if (key in self._params and + self._params[key] != () and + other.params[key] != ()): raise ReframeSyntaxError(f'parameter space conflict: ' f'parameter {key!r} already defined ' @@ -200,7 +205,7 @@ def join(self, other): other.params.get(key, ()) + self._params.get(key, ()) ) - def param_space_iterator(self): + def __iter__(self): '''Create a generator object to iterate over the parameter space :return: generator object to iterate over the parameter space. @@ -211,6 +216,11 @@ def param_space_iterator(self): def params(self): return self._params + @property + def unique_iter(self): + '''Expose the internal iterator as read-only''' + return self.__unique_iter + def __len__(self): '''Returns the number of all possible parameter combinations. @@ -232,13 +242,8 @@ def __len__(self): (len(p) for p in self._params.values()) ) - def __next__(self): - # Make the class iterable - return next(self._iter) - def __getitem__(self, key): return self._params.get(key, ()) - @property def is_empty(self): return self._params == {} diff --git a/reframe/core/pipeline.py b/reframe/core/pipeline.py index c5f245342c..8d265d9fb1 100644 --- a/reframe/core/pipeline.py +++ b/reframe/core/pipeline.py @@ -703,11 +703,11 @@ def pipeline_hooks(cls): #: :type: boolean : :default: :class:`True` build_locally = fields.TypedField('build_locally', bool) - def __new__(cls, *args, **kwargs): + def __new__(cls, *args, _rfm_use_params=False, **kwargs): obj = super().__new__(cls) # Set the test parameters in the object - cls._construct_params(obj) + cls._init_params(obj, _rfm_use_params) # Create a test name from the class name and the constructor's # arguments @@ -741,29 +741,35 @@ def __init__(self): pass @classmethod - def _construct_params(cls, obj): - '''Attaches the test parameters as class attributes. - - Inserts the regression test parameters as object attributes during - object creation. The values assigned to these test parameters are - obtained from the iterator in the test's parameter space (see - :class `reframe.core.parameters.ParamSpace`). When this iterator is - exhausted, the parameters will simply be initialized to None. + def _init_params(cls, obj, use_params=False): + '''Attach the test parameters as class attributes. + + Create and initialize the regression test parameters as object + attributes. The values assigned to these parameters exclusively depend + on the use_params argument. If this is set to True, the current object + uses the parameter space iterator (see + :class `reframe.core.pipeline.RegressionTest` and consumes a set of + parameter values (i.e. a point in the parameter space). Contrarily, if + use_params is False, the regression test parameters are initialized as + None. + + :param use_param: bool that dictates whether an instance of the + :class `reframe.core.pipeline.RegressionTest` is to use the + parameter values defined in the parameter space. :meta private: ''' - # Try to set the values of the test parameters from the param iterator. - try: - param_values = next(cls._rfm_param_space) + # Set the values of the test parameters (if any) + if use_params and cls._rfm_param_space.params: + param_values = next(cls._rfm_param_space.unique_iter) for index, key in enumerate(cls._rfm_param_space.params): setattr(obj, key, param_values[index]) return - # Initialize the params as None if an exception was raised - except StopIteration: - for key in cls._rfm_param_space.params: - setattr(obj, key, None) + # Initialize the params as None if + for key in cls._rfm_param_space.params: + setattr(obj, key, None) def _append_parameters_to_name(self): if self._rfm_param_space.params: diff --git a/unittests/test_parameters.py b/unittests/test_parameters.py index 214dbaa88b..b5bc4c201c 100644 --- a/unittests/test_parameters.py +++ b/unittests/test_parameters.py @@ -29,7 +29,7 @@ def test_param_space_is_empty(): class MyTest(NoParams): pass - assert MyTest.param_space.is_empty + assert MyTest.param_space.is_empty() def test_params_are_present(): @@ -39,6 +39,13 @@ class MyTest(TwoParams): assert MyTest.param_space['P0'] == ('a',) assert MyTest.param_space['P1'] == ('b',) +def test_abstract_param(): + class MyTest(Abstract): + pass + + assert MyTest.param_space['P0'] == () + assert MyTest.param_space['P1'] == ('b',) + def test_param_override(): class MyTest(TwoParams): @@ -69,14 +76,14 @@ def test_is_abstract_test(): class MyTest(Abstract): pass - assert MyTest.is_abstract + assert MyTest.is_abstract() def test_is_not_abstract_test(): class MyTest(TwoParams): pass - assert not MyTest.is_abstract + assert not MyTest.is_abstract() def test_param_len_is_zero(): @@ -94,7 +101,7 @@ class MyTest(ExtendParams): def test_param_values_are_not_set(): - class MyTest(Abstract): + class MyTest(TwoParams): pass test = MyTest() @@ -102,13 +109,13 @@ class MyTest(Abstract): assert test.P1 is None -def test_param_values_are_set(): - class MyTest(TwoParams): +def test_abstract_param_values_are_not_set(): + class MyTest(Abstract): pass test = MyTest() - assert test.P0 == 'a' - assert test.P1 == 'b' + assert test.P0 is None + assert test.P1 is None def test_extended_params(): @@ -121,17 +128,17 @@ class MyTest(ExtendParams): assert hasattr(test, 'P2') -def test_extended_params_are_set(): - class MyTest(ExtendParams): - pass - - for i in range(len(MyTest.param_space)): - test = MyTest() - assert test.P0 is not None - assert test.P1 is not None - assert test.P2 is not None - - test = MyTest() - assert test.P0 is None - assert test.P1 is None - assert test.P2 is None +#def test_extended_params_are_set(): +# class MyTest(ExtendParams): +# pass +# +# for _ in MyTest.param_space: +# test = MyTest() +# assert test.P0 is not None +# assert test.P1 is not None +# assert test.P2 is not None +# +# test = MyTest() +# assert test.P0 is None +# assert test.P1 is None +# assert test.P2 is None From 2e19b0b98c570f809eeacd68e5152eadd79e6904 Mon Sep 17 00:00:00 2001 From: Javier Otero Date: Tue, 22 Dec 2020 10:34:16 +0100 Subject: [PATCH 36/43] Address PR comments --- reframe/core/decorators.py | 4 +--- reframe/core/meta.py | 17 ++++++++++++++--- reframe/core/pipeline.py | 11 +++++------ 3 files changed, 20 insertions(+), 12 deletions(-) diff --git a/reframe/core/decorators.py b/reframe/core/decorators.py index e3fbc16c7d..f5ecfb7901 100644 --- a/reframe/core/decorators.py +++ b/reframe/core/decorators.py @@ -27,13 +27,11 @@ def _register_test(cls, args=None): - '''Register the test + '''Register the test. Register the test with _rfm_use_params=True. This additional argument flags this case to consume the parameter space. Otherwise, the regression test parameters would simply be initialized to None. - - :meta private: ''' def _instantiate(cls, args): if isinstance(args, collections.abc.Sequence): diff --git a/reframe/core/meta.py b/reframe/core/meta.py index 7f551aa3bf..49376a9875 100644 --- a/reframe/core/meta.py +++ b/reframe/core/meta.py @@ -80,11 +80,22 @@ def __init__(cls, name, bases, namespace, **kwargs): raise ReframeSyntaxError(msg) def __call__(cls, *args, **kwargs): + '''Intercept reframe-specific constructor arguments. + + When registering a regression test using any supported decorator, + this decorator may pass additional arguments to the class constructor + to perform specific reframe-internal actions. This gives extra control + over the class instantiation process, allowing reframe to instantiate + the regression test class differently if this class was registered or + not (e.g. when deep-copying a regression test object). These interal + arguments must be intercepted before the object initialization, since + these would otherwise affect the __init__ method's signature, and these + internal mechanisms must be fully transparent to the user. + ''' obj = cls.__new__(cls, *args, **kwargs) # Intercept constructor arguments - if '_rfm_use_params' in kwargs: - del kwargs['_rfm_use_params'] + kwargs.pop('_rfm_use_params', None) obj.__init__(*args, **kwargs) return obj @@ -95,7 +106,7 @@ def param_space(cls): return cls._rfm_param_space def is_abstract(cls): - '''Checks if the test is an abstract test. + '''Check if the test is an abstract test. If the parameter space has undefined parameters, the test is considered an abstract test. If that is the case, the length of the parameter diff --git a/reframe/core/pipeline.py b/reframe/core/pipeline.py index 8d265d9fb1..2fcfd77e26 100644 --- a/reframe/core/pipeline.py +++ b/reframe/core/pipeline.py @@ -761,15 +761,14 @@ def _init_params(cls, obj, use_params=False): ''' # Set the values of the test parameters (if any) if use_params and cls._rfm_param_space.params: + # Consume the parameter space iterator param_values = next(cls._rfm_param_space.unique_iter) for index, key in enumerate(cls._rfm_param_space.params): setattr(obj, key, param_values[index]) - - return - - # Initialize the params as None if - for key in cls._rfm_param_space.params: - setattr(obj, key, None) + else: + # Otherwise init the params as None + for key in cls._rfm_param_space.params: + setattr(obj, key, None) def _append_parameters_to_name(self): if self._rfm_param_space.params: From a42498f5d5cc58b88b80e5a37f006bc7ea15b3ef Mon Sep 17 00:00:00 2001 From: Javier Otero Date: Tue, 22 Dec 2020 13:00:43 +0100 Subject: [PATCH 37/43] Test decorated parameterized test --- unittests/test_parameters.py | 65 +++++++++++++++++++++++------------- 1 file changed, 42 insertions(+), 23 deletions(-) diff --git a/unittests/test_parameters.py b/unittests/test_parameters.py index b5bc4c201c..6632558ed0 100644 --- a/unittests/test_parameters.py +++ b/unittests/test_parameters.py @@ -4,6 +4,9 @@ # SPDX-License-Identifier: BSD-3-Clause +import pytest +import inspect + import reframe as rfm @@ -39,6 +42,7 @@ class MyTest(TwoParams): assert MyTest.param_space['P0'] == ('a',) assert MyTest.param_space['P1'] == ('b',) + def test_abstract_param(): class MyTest(Abstract): pass @@ -100,8 +104,8 @@ class MyTest(ExtendParams): assert len(MyTest.param_space) == 8 -def test_param_values_are_not_set(): - class MyTest(TwoParams): +def test_instantiate_abstract_test(): + class MyTest(Abstract): pass test = MyTest() @@ -109,8 +113,8 @@ class MyTest(TwoParams): assert test.P1 is None -def test_abstract_param_values_are_not_set(): - class MyTest(Abstract): +def test_param_values_are_not_set(): + class MyTest(TwoParams): pass test = MyTest() @@ -118,27 +122,42 @@ class MyTest(Abstract): assert test.P1 is None -def test_extended_params(): +def test_consume_param_space(): class MyTest(ExtendParams): - pass + def __init__(self): + pass + + for _ in MyTest.param_space: + test = MyTest(_rfm_use_params=True) + assert test.P0 is not None + assert test.P1 is not None + assert test.P2 is not None test = MyTest() - assert hasattr(test, 'P0') - assert hasattr(test, 'P1') - assert hasattr(test, 'P2') + assert test.P0 is None + assert test.P1 is None + assert test.P2 is None + with pytest.raises(StopIteration): + test = MyTest(_rfm_use_params=True) -#def test_extended_params_are_set(): -# class MyTest(ExtendParams): -# pass -# -# for _ in MyTest.param_space: -# test = MyTest() -# assert test.P0 is not None -# assert test.P1 is not None -# assert test.P2 is not None -# -# test = MyTest() -# assert test.P0 is None -# assert test.P1 is None -# assert test.P2 is None + +def test_register_abstract_test(): + with pytest.raises(ValueError): + @rfm.simple_test + class MyTest(Abstract): + pass + + +def test_simple_test_decorator(): + @rfm.simple_test + class MyTest(ExtendParams): + pass + + mod = inspect.getmodule(MyTest) + tests = mod._rfm_gettests() + assert len(tests) == 8 + for test in tests: + assert test.P0 is not None + assert test.P1 is not None + assert test.P2 is not None From ed2e34e17b75f12d855a1fa79857a1d9aff60e74 Mon Sep 17 00:00:00 2001 From: Javier Otero Date: Fri, 8 Jan 2021 10:51:16 +0100 Subject: [PATCH 38/43] Make both parameterization approaches incompatible --- reframe/core/decorators.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/reframe/core/decorators.py b/reframe/core/decorators.py index f5ecfb7901..0216e69aa4 100644 --- a/reframe/core/decorators.py +++ b/reframe/core/decorators.py @@ -119,10 +119,13 @@ def parameterized_test(*inst): ''' def _do_register(cls): _validate_test(cls) + if not cls.param_space.is_empty(): + raise ValueError( + 'the decorated test already is a parameterized test' + ) - for _ in cls.param_space: - for args in inst: - _register_test(cls, args) + for args in inst: + _register_test(cls, args) return cls From 8d31eda80ef6890b8185eec4989cc97ea33f8e8d Mon Sep 17 00:00:00 2001 From: Javier Otero Date: Fri, 8 Jan 2021 11:06:59 +0100 Subject: [PATCH 39/43] Add unit test --- unittests/test_parameters.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/unittests/test_parameters.py b/unittests/test_parameters.py index 6632558ed0..9f93338da2 100644 --- a/unittests/test_parameters.py +++ b/unittests/test_parameters.py @@ -94,7 +94,7 @@ def test_param_len_is_zero(): class MyTest(Abstract): pass - assert len(MyTest.param_space) == 0; + assert len(MyTest.param_space) == 0 def test_extended_param_len(): @@ -161,3 +161,11 @@ class MyTest(ExtendParams): assert test.P0 is not None assert test.P1 is not None assert test.P2 is not None + + +def test_parameterized_test_is_incompatible(): + with pytest.raises(ValueError): + @rfm.parameterized_test(['var']) + class MyTest(TwoParams): + def __init__(self, var): + pass From 24c4d8c2bbcbe29e9e565cd223d67d1cf01c7924 Mon Sep 17 00:00:00 2001 From: Javier Otero Date: Fri, 8 Jan 2021 11:43:34 +0100 Subject: [PATCH 40/43] Address PR comments --- reframe/core/decorators.py | 2 +- unittests/test_parameters.py | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/reframe/core/decorators.py b/reframe/core/decorators.py index 0216e69aa4..7955a50e04 100644 --- a/reframe/core/decorators.py +++ b/reframe/core/decorators.py @@ -121,7 +121,7 @@ def _do_register(cls): _validate_test(cls) if not cls.param_space.is_empty(): raise ValueError( - 'the decorated test already is a parameterized test' + f'{cls.__qualname__!r} is already a parameterized test' ) for args in inst: diff --git a/unittests/test_parameters.py b/unittests/test_parameters.py index 9f93338da2..56e85e955e 100644 --- a/unittests/test_parameters.py +++ b/unittests/test_parameters.py @@ -124,8 +124,7 @@ class MyTest(TwoParams): def test_consume_param_space(): class MyTest(ExtendParams): - def __init__(self): - pass + pass for _ in MyTest.param_space: test = MyTest(_rfm_use_params=True) From 2978816dc5bbd0c261abc595b2abb5138d5251b5 Mon Sep 17 00:00:00 2001 From: Vasileios Karakasis Date: Mon, 18 Jan 2021 17:51:13 +0100 Subject: [PATCH 41/43] Public documentation for directives (first draft) --- docs/regression_test_api.rst | 37 ++++++++++++++++++++++++++++++++++++ reframe/core/parameters.py | 23 ++++++++-------------- 2 files changed, 45 insertions(+), 15 deletions(-) diff --git a/docs/regression_test_api.rst b/docs/regression_test_api.rst index e8c02787c0..5716f7b28b 100644 --- a/docs/regression_test_api.rst +++ b/docs/regression_test_api.rst @@ -36,6 +36,43 @@ Pipeline Hooks .. autodecorator:: reframe.core.decorators.require_deps +.. _directives: + +Directives +---------- + +Directives are functions that you can call in the body of a ReFrame regression test class, in order to control the creation of the test. +For example, you can parameterize a test using the :func:`parameter` directive as follows: + +.. code:: python + + class MyTest(rfm.RegressionTest): + parameter('variant', ['A', 'B']) + + def __init__(self): + if self.variant == 'A': + do_this() + else: + do_other() + + + +.. py:function:: reframe.core.pipeline.RegressionTest.parameter(name, *values, inherit_params=False, filter_params=None) + + Stores the attributes of a regression test parameter as defined directly in the test definition. + These attributes are the parameter's name, values, and inheritance behaviour. + This class should be thought of as a temporary storage for these parameter attributes, before the full final parameter space is built. + + :param name: parameter name + :param values: parameter values. + If no values are passed, the parameter is considered as declared but not defined (i.e. an abstract parameter). + :param inherit_params: If false, this parameter is marked to not inherit any values for the same parameter that might have been defined in a parent class. + :param filter_params: Function to filter/modify the inherited parameter values from the parent classes. + This only has an effect if used with ``inherit_params=True``. + + + + Environments and Systems ------------------------ diff --git a/reframe/core/parameters.py b/reframe/core/parameters.py index c2e0e0670a..5ebee9bdaa 100644 --- a/reframe/core/parameters.py +++ b/reframe/core/parameters.py @@ -16,21 +16,13 @@ class _TestParameter: '''Regression test paramter class. - Stores the attributes of a regression test parameter as defined directly - in the test definition. These attributes are the parameter's name, - values, and inheritance behaviour. This class should be thought of as a - temporary storage for these parameter attributes, before the full final - parameter space is built. - - :param name: parameter name - :param values: parameter values. If no values are passed, the parameter is - considered as declared but not defined (i.e. an abstract parameter). - :param inherit_params: If false, this parameter is marked to not inherit - any values for the same parameter that might have been defined in a - parent class. - :param filter_params: Function to filter/modify the inherited parameter - values from the parent classes. This only has an effect if used with - inherit_params=True. + This wraps a test a parameter that the users create through the + :func:`parameter` directive. + + .. seealso:: + + :ref:`directives` + ''' def __init__(self, name, *values, @@ -145,6 +137,7 @@ class ParamSpace: spaces from the base classes, and also the local parameter space from the target class. ''' + def __init__(self, target_cls=None): self._params = {} From fbbb5b9a5cc32565096953452afc3536d895eb17 Mon Sep 17 00:00:00 2001 From: Javier Otero <71280927+jjotero@users.noreply.github.com> Date: Tue, 19 Jan 2021 12:51:09 +0100 Subject: [PATCH 42/43] Update param docs --- docs/regression_test_api.rst | 48 ++++++++++++++++++++++++------------ reframe/core/parameters.py | 26 ++++++++++--------- 2 files changed, 47 insertions(+), 27 deletions(-) diff --git a/docs/regression_test_api.rst b/docs/regression_test_api.rst index 5716f7b28b..d3970737d7 100644 --- a/docs/regression_test_api.rst +++ b/docs/regression_test_api.rst @@ -41,33 +41,49 @@ Pipeline Hooks Directives ---------- -Directives are functions that you can call in the body of a ReFrame regression test class, in order to control the creation of the test. -For example, you can parameterize a test using the :func:`parameter` directive as follows: +Directives are functions that can be called directly in the body of a ReFrame regression test class. +These functions exert control over the test creation, and they allow adding and/or modifying certain attributes of the regression test. +For example, a test can be parameterized using the :func:`parameter` directive as follows: .. code:: python - class MyTest(rfm.RegressionTest): - parameter('variant', ['A', 'B']) + class MyTest(rfm.RegressionTest): + parameter('variant', 'A', 'B') + + def __init__(self): + if self.variant == 'A': + do_this() + else: + do_other() - def __init__(self): - if self.variant == 'A': - do_this() - else: - do_other() +One of the most powerful features about using directives is that they store their input information at the class level. +This means if one were to extend or specialize an existing regression test, the test attribute additions and modifications made throught directives in the parent class will be automatically inherited by the child test. +For instance, continuing with the example above, one could override the ``__init__`` method in the ``MyTest`` regression test as follows: +.. code:: python + + class MyModifiedTest(MyTest): + + def __init__(self): + if self.variant == 'A': + override_this() + else: + override_other() .. py:function:: reframe.core.pipeline.RegressionTest.parameter(name, *values, inherit_params=False, filter_params=None) - Stores the attributes of a regression test parameter as defined directly in the test definition. - These attributes are the parameter's name, values, and inheritance behaviour. - This class should be thought of as a temporary storage for these parameter attributes, before the full final parameter space is built. + Inserts or modifies a regression test parameter. + If a parameter with a matching name is already present in the parameter space of a parent class, the existing parameter values will be combined with those provided by this method following the inheritance behaviour set by the arguments ``inherit_params`` and ``filt_params``. + Instead, if no parameter with a matching name exist in any of the parent parameter spaces, a new regression test parameter is created. - :param name: parameter name + :param name: parameter name. :param values: parameter values. - If no values are passed, the parameter is considered as declared but not defined (i.e. an abstract parameter). - :param inherit_params: If false, this parameter is marked to not inherit any values for the same parameter that might have been defined in a parent class. - :param filter_params: Function to filter/modify the inherited parameter values from the parent classes. + If no values are passed when creating a new parameter, the parameter is considered as declared but not defined (i.e. an abstract parameter). + Instead, for an existing parameter, this depends on the parameter's inheritance behaviour and on whether any values where provided in any of the parent parameter spaces. + :param inherit_params: If false, no parameter values that may have been defined in any of the parent parameter spaces will be inherited. + :param filter_params: Function to filter/modify the inherited parameter values that may have been provided in any of the parent parameter spaces. + This function must only expect a tuple containing the inherited parameter values as its only argument. This only has an effect if used with ``inherit_params=True``. diff --git a/reframe/core/parameters.py b/reframe/core/parameters.py index 5ebee9bdaa..24320be7c8 100644 --- a/reframe/core/parameters.py +++ b/reframe/core/parameters.py @@ -16,13 +16,11 @@ class _TestParameter: '''Regression test paramter class. - This wraps a test a parameter that the users create through the - :func:`parameter` directive. - - .. seealso:: - - :ref:`directives` - + Stores the attributes of a regression test parameter as defined directly + in the test definition. These attributes are the parameter's name, + values, and inheritance behaviour. This class should be thought of as a + temporary storage for these parameter attributes, before the full final + parameter space is built. ''' def __init__(self, name, *values, @@ -84,11 +82,17 @@ def __setitem__(self, name, value): ) def add_param(self, name, *values, **kwargs): - '''Insert a new regression test parameter in the local parameter space. + '''Insert or modify a regression test parameter. + + This method must be called directly in the class body. For each + regression test class definition, this function may only be called + once per parameter. Calling this method during or after the class + instantiation has an undefined behavior. + + .. seealso:: + + :ref:`directives` - If the parameter is already present in the dictionary, raise an error. - See the _TestParameter class for further information on the - function arguments. ''' self[name] = _TestParameter(name, *values, **kwargs) From f17fea9f3c03c490e1ff28e2f653cf69c9f39542 Mon Sep 17 00:00:00 2001 From: Javier Otero <71280927+jjotero@users.noreply.github.com> Date: Wed, 20 Jan 2021 09:47:52 +0100 Subject: [PATCH 43/43] Address PR comments --- docs/regression_test_api.rst | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/docs/regression_test_api.rst b/docs/regression_test_api.rst index d3970737d7..decdb82ba5 100644 --- a/docs/regression_test_api.rst +++ b/docs/regression_test_api.rst @@ -57,8 +57,8 @@ For example, a test can be parameterized using the :func:`parameter` directive a do_other() One of the most powerful features about using directives is that they store their input information at the class level. -This means if one were to extend or specialize an existing regression test, the test attribute additions and modifications made throught directives in the parent class will be automatically inherited by the child test. -For instance, continuing with the example above, one could override the ``__init__`` method in the ``MyTest`` regression test as follows: +This means if one were to extend or specialize an existing regression test, the test attribute additions and modifications made through directives in the parent class will be automatically inherited by the child test. +For instance, continuing with the example above, one could override the :func:`__init__` method in the :class:`MyTest` regression test as follows: .. code:: python @@ -74,16 +74,16 @@ For instance, continuing with the example above, one could override the ``__init .. py:function:: reframe.core.pipeline.RegressionTest.parameter(name, *values, inherit_params=False, filter_params=None) Inserts or modifies a regression test parameter. - If a parameter with a matching name is already present in the parameter space of a parent class, the existing parameter values will be combined with those provided by this method following the inheritance behaviour set by the arguments ``inherit_params`` and ``filt_params``. - Instead, if no parameter with a matching name exist in any of the parent parameter spaces, a new regression test parameter is created. + If a parameter with a matching name is already present in the parameter space of a parent class, the existing parameter values will be combined with those provided by this method following the inheritance behaviour set by the arguments ``inherit_params`` and ``filter_params``. + Instead, if no parameter with a matching name exists in any of the parent parameter spaces, a new regression test parameter is created. - :param name: parameter name. - :param values: parameter values. - If no values are passed when creating a new parameter, the parameter is considered as declared but not defined (i.e. an abstract parameter). + :param name: the parameter name. + :param values: the parameter values. + If no values are passed when creating a new parameter, the parameter is considered as *declared* but not *defined* (i.e. an abstract parameter). Instead, for an existing parameter, this depends on the parameter's inheritance behaviour and on whether any values where provided in any of the parent parameter spaces. - :param inherit_params: If false, no parameter values that may have been defined in any of the parent parameter spaces will be inherited. + :param inherit_params: If :obj:`False`, no parameter values that may have been defined in any of the parent parameter spaces will be inherited. :param filter_params: Function to filter/modify the inherited parameter values that may have been provided in any of the parent parameter spaces. - This function must only expect a tuple containing the inherited parameter values as its only argument. + This function must accept a single argument, which will be passed as an iterable containing the inherited parameter values. This only has an effect if used with ``inherit_params=True``.