From fb3bd0524d7bbb698df1a6827e140486ca987b5f Mon Sep 17 00:00:00 2001 From: Mark Nestor Costantini Date: Fri, 10 May 2024 15:41:06 +0100 Subject: [PATCH 01/24] rules are tuple instead of list -> makes them hashable --- validphys2/src/validphys/config.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/validphys2/src/validphys/config.py b/validphys2/src/validphys/config.py index c746995017..bf550e5789 100644 --- a/validphys2/src/validphys/config.py +++ b/validphys2/src/validphys/config.py @@ -1331,7 +1331,7 @@ def produce_rules( except RuleProcessingError as e: raise ConfigError(f"Error processing added rule {i}: {e}") from e - return rule_list + return tuple(rule_list) @configparser.record_from_defaults def parse_default_filter_settings(self, spec: (str, type(None))): From df6a42b5475e8bf80724f0bff0529652d9739f09 Mon Sep 17 00:00:00 2001 From: Mark Nestor Costantini Date: Sat, 11 May 2024 12:39:14 +0100 Subject: [PATCH 02/24] return empty tuple instead of None -> None is not iterable --- validphys2/src/validphys/config.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/validphys2/src/validphys/config.py b/validphys2/src/validphys/config.py index bf550e5789..ce326edc4c 100644 --- a/validphys2/src/validphys/config.py +++ b/validphys2/src/validphys/config.py @@ -1296,7 +1296,7 @@ def produce_rules( if filter_rules is None: # Don't bother loading the rules if we are not using them. if use_cuts is not CutsPolicy.INTERNAL: - return None + return tuple() if default_filter_rules_recorded_spec_ is not None: filter_rules = default_filter_rules_recorded_spec_[default_filter_rules] else: From 82cd476f4ce48ff82c6d8bdadc7983ddc93a96e8 Mon Sep 17 00:00:00 2001 From: Mark Nestor Costantini Date: Sat, 11 May 2024 13:59:28 +0100 Subject: [PATCH 03/24] added FilterDefaults frozen dataclass --- validphys2/src/validphys/filters.py | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/validphys2/src/validphys/filters.py b/validphys2/src/validphys/filters.py index cded762eab..d5dcb780cf 100644 --- a/validphys2/src/validphys/filters.py +++ b/validphys2/src/validphys/filters.py @@ -7,6 +7,7 @@ from importlib.resources import read_text import logging import re +import dataclasses import numpy as np @@ -99,11 +100,25 @@ class FatalRuleError(Exception): """Exception raised when a rule application failed at runtime.""" +@dataclasses.dataclass(frozen=True) +class FilterDefaults: + """ + Dataclass carrying default values for filters (cuts) taking into + account the values of ``q2min``, ``w2min`` and ``maxTau``. + """ + q2min: float + w2min: float + maxTau: float + + def to_dict(self): + return dataclasses.asdict(self) + + def default_filter_settings_input(): """Return a dictionary with the default hardcoded filter settings. These are defined in ``defaults.yaml`` in the ``validphys.cuts`` module. """ - return yaml.safe_load(read_text(validphys.cuts, "defaults.yaml")) + return FilterDefaults(**yaml.safe_load(read_text(validphys.cuts, "defaults.yaml"))) def default_filter_rules_input(): From 90f04daef7f7d85292fd47b0ea31c128971ddc03 Mon Sep 17 00:00:00 2001 From: Mark Nestor Costantini Date: Sat, 11 May 2024 14:00:18 +0100 Subject: [PATCH 04/24] filter defaults returns FilterDefaults dataclass instead of unhashable dict --- validphys2/src/validphys/config.py | 63 +++++++++++++++++++++-------- validphys2/src/validphys/filters.py | 4 +- 2 files changed, 48 insertions(+), 19 deletions(-) diff --git a/validphys2/src/validphys/config.py b/validphys2/src/validphys/config.py index ce326edc4c..f373db9062 100644 --- a/validphys2/src/validphys/config.py +++ b/validphys2/src/validphys/config.py @@ -42,6 +42,8 @@ from validphys.plotoptions.core import get_info import validphys.scalevariations from validphys.utils import freeze_args +from validphys.filters import FilterDefaults + log = logging.getLogger(__name__) @@ -1359,10 +1361,30 @@ def load_default_default_filter_settings(self, spec): def parse_filter_defaults(self, filter_defaults: (dict, type(None))): """A mapping containing the default kinematic limits to be used when filtering data (when using internal cuts). - Currently these limits are ``q2min`` and ``w2min``. + Currently these limits are ``q2min``, ``w2min``, and ``maxTau``. + + Parameters + ---------- + filter_defaults: dict, None + A mapping containing the default kinematic limits to be used when + filtering data (when using internal cuts). + Currently these limits are ``q2min``, ``w2min``, and ``maxTau``. + + Returns + ------- + FilterDefaults + A hashable object containing the default kinematic limits to be used when + filtering data (when using internal cuts). + Currently these limits are ``q2min``, ``w2min``, and ``maxTau``. """ log.warning("Overwriting filter defaults") - return filter_defaults + + parsed_filter_defaults = FilterDefaults( + q2min = filter_defaults["q2min"] if "q2min" in filter_defaults else None, + w2min = filter_defaults["w2min"] if "w2min" in filter_defaults else None, + maxTau = filter_defaults["maxTau"] if "maxTau" in filter_defaults else None, + ) + return parsed_filter_defaults def produce_defaults( self, @@ -1370,34 +1392,39 @@ def produce_defaults( w2min=None, maxTau=None, default_filter_settings=None, - filter_defaults={}, + filter_defaults=FilterDefaults, default_filter_settings_recorded_spec_=None, ): """Produce default values for filters taking into account the values of ``q2min``, ``w2min`` and ``maxTau`` defined at namespace level and those inside a ``filter_defaults`` mapping. + + Within this function the hashable type FilterDefaults is turned into + a dictionary so as to allow for overwriting of the values of q2min, w2min and maxTau. + The dictionary is then turned back into a FilterDefaults object. """ from validphys.filters import default_filter_settings_input + + if isinstance(filter_defaults, FilterDefaults): + filter_defaults = filter_defaults.to_dict() - if q2min is not None and "q2min" in filter_defaults and q2min != filter_defaults["q2min"]: - raise ConfigError("q2min defined multiple times with different values") - if w2min is not None and "w2min" in filter_defaults and w2min != filter_defaults["w2min"]: - raise ConfigError("w2min defined multiple times with different values") + if q2min is not None and filter_defaults["q2min"] != q2min: + raise ConfigError("q2min defined multiple times with different values") + + if w2min is not None and filter_defaults["w2min"] != w2min: + raise ConfigError("w2min defined multiple times with different values") - if ( - maxTau is not None - and "maxTau" in filter_defaults - and maxTau != filter_defaults["maxTau"] - ): - raise ConfigError("maxTau defined multiple times with different values") + if maxTau is not None and filter_defaults["maxTau"] != maxTau: + raise ConfigError("maxTau defined multiple times with different values") if default_filter_settings_recorded_spec_ is not None: - filter_defaults = default_filter_settings_recorded_spec_[default_filter_settings] + filter_defaults = FilterDefaults(**default_filter_settings_recorded_spec_[default_filter_settings]) # If we find recorded specs return immediately and don't read q2min and w2min # from runcard return filter_defaults - elif not filter_defaults: - filter_defaults = default_filter_settings_input() + elif not isinstance(filter_defaults, FilterDefaults): + # if filter_defaults have not been set, load the defaults with default_filter_settings_input + filter_defaults = default_filter_settings_input().to_dict() defaults_loaded = True else: defaults_loaded = False @@ -1413,7 +1440,9 @@ def produce_defaults( if maxTau is not None and defaults_loaded: log.warning("Using maxTau from runcard") filter_defaults["maxTau"] = maxTau - + + # Turn the dictionary back into a hashable FilterDefaults object + filter_defaults = FilterDefaults(**filter_defaults) return filter_defaults def produce_data(self, data_input, *, group_name="data"): diff --git a/validphys2/src/validphys/filters.py b/validphys2/src/validphys/filters.py index d5dcb780cf..e685deaa70 100644 --- a/validphys2/src/validphys/filters.py +++ b/validphys2/src/validphys/filters.py @@ -526,7 +526,7 @@ def __init__(self, initial_data: dict, *, defaults: dict, theory_parameters: dic self.rule_string = self.rule self.defaults = defaults self.theory_params = theory_parameters - ns = {*self.numpy_functions, *self.defaults, *self.variables, "idat", "central_value"} + ns = {*self.numpy_functions, *self.defaults.to_dict().keys(), *self.variables, "idat", "central_value"} for k, v in self.local_variables.items(): try: self._local_variables_code[k] = lcode = compile( @@ -607,7 +607,7 @@ def __call__(self, dataset, idat): return eval( self.rule, self.numpy_functions, - {**{"idat": idat, "central_value": central_value}, **self.defaults, **ns}, + {**{"idat": idat, "central_value": central_value}, **self.defaults.to_dict(), **ns}, ) except Exception as e: # pragma: no cover raise FatalRuleError(f"Error when applying rule {self.rule_string!r}: {e}") from e From a9b7e1ca8e3c81609f149a2f9cff87830f55061a Mon Sep 17 00:00:00 2001 From: Mark Nestor Costantini Date: Sat, 11 May 2024 15:00:38 +0100 Subject: [PATCH 05/24] make inputs to produce_rules hashable types. no need for freeze args --- validphys2/src/validphys/config.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/validphys2/src/validphys/config.py b/validphys2/src/validphys/config.py index f373db9062..b03a265a00 100644 --- a/validphys2/src/validphys/config.py +++ b/validphys2/src/validphys/config.py @@ -41,7 +41,6 @@ from validphys.paramfits.config import ParamfitsConfig from validphys.plotoptions.core import get_info import validphys.scalevariations -from validphys.utils import freeze_args from validphys.filters import FilterDefaults @@ -1273,12 +1272,11 @@ def parse_default_filter_rules_recorded_spec_(self, spec): return spec def parse_added_filter_rules(self, rules: (list, type(None)) = None): - return rules + return tuple(frozendict(rule) for rule in rules) if rules else None # Every parallel replica triggers a series of calls to this function, # which should not happen since the rules are identical among replicas. # E.g for NNPDF4.0 with 2 parallel replicas 693 calls, 3 parallel replicas 1001 calls... - @freeze_args @functools.lru_cache def produce_rules( self, @@ -1288,7 +1286,7 @@ def produce_rules( default_filter_rules=None, filter_rules=None, default_filter_rules_recorded_spec_=None, - added_filter_rules: (list, type(None)) = None, + added_filter_rules: (tuple, type(None)) = None, ): """Produce filter rules based on the user defined input and defaults.""" from validphys.filters import Rule, RuleProcessingError, default_filter_rules_input From a600a3f5a728828406cdb0e0636ffe7407015dd1 Mon Sep 17 00:00:00 2001 From: Mark Nestor Costantini Date: Sun, 12 May 2024 15:26:23 +0100 Subject: [PATCH 06/24] import validphys.filters at top of module --- validphys2/src/validphys/config.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/validphys2/src/validphys/config.py b/validphys2/src/validphys/config.py index b03a265a00..966bddc17d 100644 --- a/validphys2/src/validphys/config.py +++ b/validphys2/src/validphys/config.py @@ -41,7 +41,7 @@ from validphys.paramfits.config import ParamfitsConfig from validphys.plotoptions.core import get_info import validphys.scalevariations -from validphys.filters import FilterDefaults +from validphys.filters import FilterDefaults, default_filter_settings_input, Rule, RuleProcessingError, default_filter_rules_input log = logging.getLogger(__name__) @@ -1289,7 +1289,6 @@ def produce_rules( added_filter_rules: (tuple, type(None)) = None, ): """Produce filter rules based on the user defined input and defaults.""" - from validphys.filters import Rule, RuleProcessingError, default_filter_rules_input theory_parameters = theoryid.get_description() @@ -1401,7 +1400,6 @@ def produce_defaults( a dictionary so as to allow for overwriting of the values of q2min, w2min and maxTau. The dictionary is then turned back into a FilterDefaults object. """ - from validphys.filters import default_filter_settings_input if isinstance(filter_defaults, FilterDefaults): filter_defaults = filter_defaults.to_dict() From a256b454e8c553b199ce4e72090ff92fd7ded385 Mon Sep 17 00:00:00 2001 From: Mark Nestor Costantini Date: Sun, 12 May 2024 15:27:14 +0100 Subject: [PATCH 07/24] parse_filter_rules returns tuple of frozendicts --- validphys2/src/validphys/config.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/validphys2/src/validphys/config.py b/validphys2/src/validphys/config.py index 966bddc17d..ad8023d623 100644 --- a/validphys2/src/validphys/config.py +++ b/validphys2/src/validphys/config.py @@ -1262,7 +1262,7 @@ def parse_filter_rules(self, filter_rules: (list, type(None))): """A list of filter rules. See https://docs.nnpdf.science/vp/filters.html for details on the syntax""" log.warning("Overwriting filter rules") - return filter_rules + return tuple(frozendict(rule) for rule in filter_rules) if filter_rules else None def parse_default_filter_rules_recorded_spec_(self, spec): """This function is a hacky fix for parsing the recorded spec From ad75add2d6ce6345d94734bfd7601cc797298924 Mon Sep 17 00:00:00 2001 From: Mark Nestor Costantini Date: Sun, 12 May 2024 16:41:28 +0100 Subject: [PATCH 08/24] check_default_filter_rules returns tuple --- validphys2/src/validphys/loader.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/validphys2/src/validphys/loader.py b/validphys2/src/validphys/loader.py index 74e9cf5bd2..8eddccd406 100644 --- a/validphys2/src/validphys/loader.py +++ b/validphys2/src/validphys/loader.py @@ -674,10 +674,10 @@ def check_default_filter_rules(self, theoryid, defaults=None): th_params = theoryid.get_description() if defaults is None: defaults = default_filter_settings_input() - return [ + return tuple( Rule(inp, defaults=defaults, theory_parameters=th_params, loader=self) for inp in default_filter_rules_input() - ] + ) def _check_theory_old_or_new(self, theoryid, commondata, cfac): """Given a theory and a commondata and a theory load the right fktable From 9d08c617de1f0aa721c139e3a60567a3ad2d77fc Mon Sep 17 00:00:00 2001 From: Mark Nestor Costantini Date: Mon, 13 May 2024 10:59:26 +0100 Subject: [PATCH 09/24] applied review changes --- validphys2/src/validphys/config.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/validphys2/src/validphys/config.py b/validphys2/src/validphys/config.py index ad8023d623..5f812ae63f 100644 --- a/validphys2/src/validphys/config.py +++ b/validphys2/src/validphys/config.py @@ -1259,8 +1259,8 @@ def load_default_default_filter_rules(self, spec): ) def parse_filter_rules(self, filter_rules: (list, type(None))): - """A list of filter rules. See https://docs.nnpdf.science/vp/filters.html - for details on the syntax""" + """A tutple of filter rules. The rules are immutable dictionaries (frozendicts). + See https://docs.nnpdf.science/vp/filters.html for details on the syntax""" log.warning("Overwriting filter rules") return tuple(frozendict(rule) for rule in filter_rules) if filter_rules else None @@ -1295,7 +1295,7 @@ def produce_rules( if filter_rules is None: # Don't bother loading the rules if we are not using them. if use_cuts is not CutsPolicy.INTERNAL: - return tuple() + return None if default_filter_rules_recorded_spec_ is not None: filter_rules = default_filter_rules_recorded_spec_[default_filter_rules] else: @@ -1389,7 +1389,7 @@ def produce_defaults( w2min=None, maxTau=None, default_filter_settings=None, - filter_defaults=FilterDefaults, + filter_defaults=None, default_filter_settings_recorded_spec_=None, ): """Produce default values for filters taking into account the @@ -1400,6 +1400,8 @@ def produce_defaults( a dictionary so as to allow for overwriting of the values of q2min, w2min and maxTau. The dictionary is then turned back into a FilterDefaults object. """ + if filter_defaults is None: + filter_defaults = {} if isinstance(filter_defaults, FilterDefaults): filter_defaults = filter_defaults.to_dict() From 0c085e043e303e2b047cd312d94cae13ba0b64e9 Mon Sep 17 00:00:00 2001 From: Mark Nestor Costantini Date: Mon, 13 May 2024 16:27:15 +0100 Subject: [PATCH 10/24] Introduced FilterRule and AddedFilterRule objects --- validphys2/src/validphys/config.py | 26 +++++++++---- validphys2/src/validphys/filters.py | 38 +++++++++++++++++-- validphys2/src/validphys/loader.py | 2 +- .../src/validphys/tests/test_filter_rules.py | 2 +- 4 files changed, 55 insertions(+), 13 deletions(-) diff --git a/validphys2/src/validphys/config.py b/validphys2/src/validphys/config.py index 5f812ae63f..acc92f692c 100644 --- a/validphys2/src/validphys/config.py +++ b/validphys2/src/validphys/config.py @@ -41,7 +41,14 @@ from validphys.paramfits.config import ParamfitsConfig from validphys.plotoptions.core import get_info import validphys.scalevariations -from validphys.filters import FilterDefaults, default_filter_settings_input, Rule, RuleProcessingError, default_filter_rules_input +from validphys.filters import (FilterDefaults, + AddedFilterRule, + FilterRule, + default_filter_settings_input, + Rule, + RuleProcessingError, + default_filter_rules_input + ) log = logging.getLogger(__name__) @@ -1259,10 +1266,10 @@ def load_default_default_filter_rules(self, spec): ) def parse_filter_rules(self, filter_rules: (list, type(None))): - """A tutple of filter rules. The rules are immutable dictionaries (frozendicts). + """A tuple of FilterRule objects. The rules are frozen dataclasses. See https://docs.nnpdf.science/vp/filters.html for details on the syntax""" log.warning("Overwriting filter rules") - return tuple(frozendict(rule) for rule in filter_rules) if filter_rules else None + return tuple(FilterRule(**rule) for rule in filter_rules) if filter_rules else None def parse_default_filter_rules_recorded_spec_(self, spec): """This function is a hacky fix for parsing the recorded spec @@ -1272,7 +1279,11 @@ def parse_default_filter_rules_recorded_spec_(self, spec): return spec def parse_added_filter_rules(self, rules: (list, type(None)) = None): - return tuple(frozendict(rule) for rule in rules) if rules else None + """ + Returns a tuple of AddedFilterRule objects. The rules are frozen dataclasses. + AddedFilterRule objects inherit from FilterRule objects. + """ + return tuple(AddedFilterRule(**rule) for rule in rules) if rules else None # Every parallel replica triggers a series of calls to this function, # which should not happen since the rules are identical among replicas. @@ -1304,7 +1315,7 @@ def produce_rules( try: rule_list = [ Rule( - initial_data=rule, + initial_data=rule.to_dict(), defaults=defaults, theory_parameters=theory_parameters, loader=self.loader, @@ -1316,12 +1327,11 @@ def produce_rules( if added_filter_rules: for i, rule in enumerate(added_filter_rules): - if not isinstance(rule, (dict, frozendict)): - raise ConfigError(f"added rule {i} is not a dict") + try: rule_list.append( Rule( - initial_data=rule, + initial_data=rule.to_dict(), defaults=defaults, theory_parameters=theory_parameters, loader=self.loader, diff --git a/validphys2/src/validphys/filters.py b/validphys2/src/validphys/filters.py index e685deaa70..62b561afe9 100644 --- a/validphys2/src/validphys/filters.py +++ b/validphys2/src/validphys/filters.py @@ -8,6 +8,7 @@ import logging import re import dataclasses +from typing import Union import numpy as np @@ -114,18 +115,49 @@ def to_dict(self): return dataclasses.asdict(self) +@dataclasses.dataclass(frozen=True) +class FilterRule: + """ + Dataclass which carries the filter rule information. + """ + dataset: str = None + process_type: str = None + rule: str = None + reason: str = None + local_variables: Mapping[str, Union[str, float]] = None + PTO: str = None + FNS: str = None + IC: str = None + + def to_dict(self): + rule_dict = dataclasses.asdict(self) + filtered_dict = {k: v for k, v in rule_dict.items() if v is not None} + return filtered_dict + + +@dataclasses.dataclass(frozen=True) +class AddedFilterRule(FilterRule): + """ + Dataclass which carries extra filter rule that is added to the + default rule. + """ + pass + + def default_filter_settings_input(): - """Return a dictionary with the default hardcoded filter settings. + """Return a FilterDefaults dataclass with the default hardcoded filter settings. These are defined in ``defaults.yaml`` in the ``validphys.cuts`` module. """ return FilterDefaults(**yaml.safe_load(read_text(validphys.cuts, "defaults.yaml"))) def default_filter_rules_input(): - """Return a dictionary with the input settings. + """ + Return a tuple of FilterRule objects. These are defined in ``filters.yaml`` in the ``validphys.cuts`` module. """ - return yaml.safe_load(read_text(validphys.cuts, "filters.yaml")) + list_rules = yaml.safe_load(read_text(validphys.cuts, "filters.yaml")) + return tuple(FilterRule(**rule) for rule in list_rules) def check_nonnegative(var: str): diff --git a/validphys2/src/validphys/loader.py b/validphys2/src/validphys/loader.py index 8eddccd406..405a7f4b1d 100644 --- a/validphys2/src/validphys/loader.py +++ b/validphys2/src/validphys/loader.py @@ -675,7 +675,7 @@ def check_default_filter_rules(self, theoryid, defaults=None): if defaults is None: defaults = default_filter_settings_input() return tuple( - Rule(inp, defaults=defaults, theory_parameters=th_params, loader=self) + Rule(inp.to_dict(), defaults=defaults, theory_parameters=th_params, loader=self) for inp in default_filter_rules_input() ) diff --git a/validphys2/src/validphys/tests/test_filter_rules.py b/validphys2/src/validphys/tests/test_filter_rules.py index 05ee29827d..4d864edd02 100644 --- a/validphys2/src/validphys/tests/test_filter_rules.py +++ b/validphys2/src/validphys/tests/test_filter_rules.py @@ -120,7 +120,7 @@ def test_added_rules(): { "speclabel": "fewer data", "added_filter_rules": [ - {"dataset": "ATLAS_1JET_8TEV_R06_PTY", "rule": "pT < 1000", "reson": "pt cut"} + {"dataset": "ATLAS_1JET_8TEV_R06_PTY", "rule": "pT < 1000", "reason": "pt cut"} ], }, { From 2d022aa59b192a12608820b1e56a6421d1dbb2a2 Mon Sep 17 00:00:00 2001 From: Mark Nestor Costantini Date: Mon, 13 May 2024 16:27:35 +0100 Subject: [PATCH 11/24] removed frozendict import in config --- validphys2/src/validphys/config.py | 1 - 1 file changed, 1 deletion(-) diff --git a/validphys2/src/validphys/config.py b/validphys2/src/validphys/config.py index acc92f692c..5d6e7b2764 100644 --- a/validphys2/src/validphys/config.py +++ b/validphys2/src/validphys/config.py @@ -8,7 +8,6 @@ import numbers import pathlib -from frozendict import frozendict import pandas as pd from nnpdf_data import legacy_to_new_map From fd9024f9577880c7fc7d3a0c25d489547bfdb79b Mon Sep 17 00:00:00 2001 From: Mark Nestor Costantini Date: Tue, 14 May 2024 12:08:09 +0100 Subject: [PATCH 12/24] applied comments of review --- validphys2/src/validphys/config.py | 14 +++----------- validphys2/src/validphys/filters.py | 19 ++++++++++++++----- validphys2/src/validphys/loader.py | 2 +- 3 files changed, 18 insertions(+), 17 deletions(-) diff --git a/validphys2/src/validphys/config.py b/validphys2/src/validphys/config.py index 5d6e7b2764..399407d64d 100644 --- a/validphys2/src/validphys/config.py +++ b/validphys2/src/validphys/config.py @@ -1284,9 +1284,6 @@ def parse_added_filter_rules(self, rules: (list, type(None)) = None): """ return tuple(AddedFilterRule(**rule) for rule in rules) if rules else None - # Every parallel replica triggers a series of calls to this function, - # which should not happen since the rules are identical among replicas. - # E.g for NNPDF4.0 with 2 parallel replicas 693 calls, 3 parallel replicas 1001 calls... @functools.lru_cache def produce_rules( self, @@ -1314,7 +1311,7 @@ def produce_rules( try: rule_list = [ Rule( - initial_data=rule.to_dict(), + initial_data=rule, defaults=defaults, theory_parameters=theory_parameters, loader=self.loader, @@ -1330,7 +1327,7 @@ def produce_rules( try: rule_list.append( Rule( - initial_data=rule.to_dict(), + initial_data=rule, defaults=defaults, theory_parameters=theory_parameters, loader=self.loader, @@ -1384,12 +1381,7 @@ def parse_filter_defaults(self, filter_defaults: (dict, type(None))): Currently these limits are ``q2min``, ``w2min``, and ``maxTau``. """ log.warning("Overwriting filter defaults") - - parsed_filter_defaults = FilterDefaults( - q2min = filter_defaults["q2min"] if "q2min" in filter_defaults else None, - w2min = filter_defaults["w2min"] if "w2min" in filter_defaults else None, - maxTau = filter_defaults["maxTau"] if "maxTau" in filter_defaults else None, - ) + parsed_filter_defaults = FilterDefaults(**filter_defaults) return parsed_filter_defaults def produce_defaults( diff --git a/validphys2/src/validphys/filters.py b/validphys2/src/validphys/filters.py index 62b561afe9..5eb3f57473 100644 --- a/validphys2/src/validphys/filters.py +++ b/validphys2/src/validphys/filters.py @@ -107,12 +107,14 @@ class FilterDefaults: Dataclass carrying default values for filters (cuts) taking into account the values of ``q2min``, ``w2min`` and ``maxTau``. """ - q2min: float - w2min: float - maxTau: float + q2min: float = None + w2min: float = None + maxTau: float = None def to_dict(self): - return dataclasses.asdict(self) + default_dict = dataclasses.asdict(self) + filtered_dict = {k: v for k, v in default_dict.items() if v is not None} + return filtered_dict @dataclasses.dataclass(frozen=True) @@ -504,10 +506,17 @@ class Rule: numpy_functions = {"sqrt": np.sqrt, "log": np.log, "fabs": np.fabs} - def __init__(self, initial_data: dict, *, defaults: dict, theory_parameters: dict, loader=None): + def __init__(self, initial_data: dataclasses.dataclass, *, defaults: dict, theory_parameters: dict, loader=None): self.dataset = None self.process_type = None self._local_variables_code = {} + + # For compatibility with legacy code that passed a dictionary + if isinstance(initial_data, FilterRule): + initial_data = initial_data.to_dict() + else: + raise RuleProcessingError("Expecting initial_data to be an instance of a FilterRule dataclass.") + for key in initial_data: setattr(self, key, initial_data[key]) diff --git a/validphys2/src/validphys/loader.py b/validphys2/src/validphys/loader.py index 405a7f4b1d..8eddccd406 100644 --- a/validphys2/src/validphys/loader.py +++ b/validphys2/src/validphys/loader.py @@ -675,7 +675,7 @@ def check_default_filter_rules(self, theoryid, defaults=None): if defaults is None: defaults = default_filter_settings_input() return tuple( - Rule(inp.to_dict(), defaults=defaults, theory_parameters=th_params, loader=self) + Rule(inp, defaults=defaults, theory_parameters=th_params, loader=self) for inp in default_filter_rules_input() ) From 4bf928df800631e12d6f87d19079ec47d2b859d2 Mon Sep 17 00:00:00 2001 From: Mark Nestor Costantini <85164495+comane@users.noreply.github.com> Date: Tue, 14 May 2024 11:52:19 +0100 Subject: [PATCH 13/24] Update validphys2/src/validphys/config.py Co-authored-by: Juan M. Cruz-Martinez --- validphys2/src/validphys/config.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/validphys2/src/validphys/config.py b/validphys2/src/validphys/config.py index 399407d64d..b0f0998af9 100644 --- a/validphys2/src/validphys/config.py +++ b/validphys2/src/validphys/config.py @@ -1265,7 +1265,7 @@ def load_default_default_filter_rules(self, spec): ) def parse_filter_rules(self, filter_rules: (list, type(None))): - """A tuple of FilterRule objects. The rules are frozen dataclasses. + """A tuple of FilterRule objects. Rules are immutable after parsing. See https://docs.nnpdf.science/vp/filters.html for details on the syntax""" log.warning("Overwriting filter rules") return tuple(FilterRule(**rule) for rule in filter_rules) if filter_rules else None From d9e3c182cd9d8eabdcf6ad478fb30f159ced53f0 Mon Sep 17 00:00:00 2001 From: Mark Nestor Costantini <85164495+comane@users.noreply.github.com> Date: Tue, 14 May 2024 11:52:32 +0100 Subject: [PATCH 14/24] Update validphys2/src/validphys/config.py Co-authored-by: Juan M. Cruz-Martinez --- validphys2/src/validphys/config.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/validphys2/src/validphys/config.py b/validphys2/src/validphys/config.py index b0f0998af9..c8fc6728ac 100644 --- a/validphys2/src/validphys/config.py +++ b/validphys2/src/validphys/config.py @@ -1279,7 +1279,7 @@ def parse_default_filter_rules_recorded_spec_(self, spec): def parse_added_filter_rules(self, rules: (list, type(None)) = None): """ - Returns a tuple of AddedFilterRule objects. The rules are frozen dataclasses. + Returns a tuple of AddedFilterRule objects. Rules are immutable after parsing. AddedFilterRule objects inherit from FilterRule objects. """ return tuple(AddedFilterRule(**rule) for rule in rules) if rules else None From d542c7dbe3173c15ac274162b6819fa334008b24 Mon Sep 17 00:00:00 2001 From: Mark Nestor Costantini Date: Tue, 14 May 2024 12:23:32 +0100 Subject: [PATCH 15/24] Rule accepts both FilterRule objects as well as Mappings --- validphys2/src/validphys/filters.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/validphys2/src/validphys/filters.py b/validphys2/src/validphys/filters.py index 5eb3f57473..b97848185d 100644 --- a/validphys2/src/validphys/filters.py +++ b/validphys2/src/validphys/filters.py @@ -514,8 +514,11 @@ def __init__(self, initial_data: dataclasses.dataclass, *, defaults: dict, theor # For compatibility with legacy code that passed a dictionary if isinstance(initial_data, FilterRule): initial_data = initial_data.to_dict() + elif isinstance(initial_data, Mapping): + initial_data = dict(initial_data) else: - raise RuleProcessingError("Expecting initial_data to be an instance of a FilterRule dataclass.") + raise RuleProcessingError("Expecting initial_data to be an instance of a FilterRule dataclass, \ + Mappings are also supported.") for key in initial_data: setattr(self, key, initial_data[key]) From 7da9282550915c2fda3d3d988c777f9a32d650bb Mon Sep 17 00:00:00 2001 From: Mark Nestor Costantini <85164495+comane@users.noreply.github.com> Date: Tue, 14 May 2024 17:43:31 +0100 Subject: [PATCH 16/24] Update validphys2/src/validphys/filters.py Co-authored-by: Juan M. Cruz-Martinez --- validphys2/src/validphys/filters.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/validphys2/src/validphys/filters.py b/validphys2/src/validphys/filters.py index b97848185d..6ac165e9c5 100644 --- a/validphys2/src/validphys/filters.py +++ b/validphys2/src/validphys/filters.py @@ -506,7 +506,7 @@ class Rule: numpy_functions = {"sqrt": np.sqrt, "log": np.log, "fabs": np.fabs} - def __init__(self, initial_data: dataclasses.dataclass, *, defaults: dict, theory_parameters: dict, loader=None): + def __init__(self, initial_data: FilterRule, *, defaults: dict, theory_parameters: dict, loader=None): self.dataset = None self.process_type = None self._local_variables_code = {} From 610a1425d202cdfd59854efb2cb5d4b94ea7116a Mon Sep 17 00:00:00 2001 From: Mark Nestor Costantini Date: Tue, 14 May 2024 17:46:28 +0100 Subject: [PATCH 17/24] to_dict method of DefaultFilters also takes default None values --- validphys2/src/validphys/filters.py | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/validphys2/src/validphys/filters.py b/validphys2/src/validphys/filters.py index 6ac165e9c5..8dc8526fb7 100644 --- a/validphys2/src/validphys/filters.py +++ b/validphys2/src/validphys/filters.py @@ -112,9 +112,7 @@ class FilterDefaults: maxTau: float = None def to_dict(self): - default_dict = dataclasses.asdict(self) - filtered_dict = {k: v for k, v in default_dict.items() if v is not None} - return filtered_dict + return dataclasses.asdict(self) @dataclasses.dataclass(frozen=True) @@ -517,8 +515,7 @@ def __init__(self, initial_data: FilterRule, *, defaults: dict, theory_parameter elif isinstance(initial_data, Mapping): initial_data = dict(initial_data) else: - raise RuleProcessingError("Expecting initial_data to be an instance of a FilterRule dataclass, \ - Mappings are also supported.") + raise RuleProcessingError("Expecting initial_data to be an instance of a FilterRule dataclass.") for key in initial_data: setattr(self, key, initial_data[key]) From 214a75a3b90b2388d585290ad2dfc3c9851d36f1 Mon Sep 17 00:00:00 2001 From: Mark Nestor Costantini Date: Thu, 16 May 2024 18:36:20 +0100 Subject: [PATCH 18/24] restored previous behaviour of produce_defaults function --- validphys2/src/validphys/config.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/validphys2/src/validphys/config.py b/validphys2/src/validphys/config.py index c8fc6728ac..a0288419f4 100644 --- a/validphys2/src/validphys/config.py +++ b/validphys2/src/validphys/config.py @@ -1407,14 +1407,14 @@ def produce_defaults( if isinstance(filter_defaults, FilterDefaults): filter_defaults = filter_defaults.to_dict() - if q2min is not None and filter_defaults["q2min"] != q2min: - raise ConfigError("q2min defined multiple times with different values") - - if w2min is not None and filter_defaults["w2min"] != w2min: - raise ConfigError("w2min defined multiple times with different values") - - if maxTau is not None and filter_defaults["maxTau"] != maxTau: - raise ConfigError("maxTau defined multiple times with different values") + if q2min is not None and "q2min" in filter_defaults and q2min != filter_defaults["q2min"]: + raise ConfigError("q2min defined multiple times with different values") + + if w2min is not None and "w2min" in filter_defaults and w2min != filter_defaults["w2min"]: + raise ConfigError("w2min defined multiple times with different values") + + if maxTau is not None and filter_defaults["maxTau"] != maxTau: + raise ConfigError("maxTau defined multiple times with different values") if default_filter_settings_recorded_spec_ is not None: filter_defaults = FilterDefaults(**default_filter_settings_recorded_spec_[default_filter_settings]) From bbfe2c9a59274850d08c0221d473ec5476fcb76e Mon Sep 17 00:00:00 2001 From: Mark Nestor Costantini Date: Sat, 18 May 2024 11:19:59 +0100 Subject: [PATCH 19/24] removed freeze args from get_cuts_for_dataset as args are now hashable --- validphys2/src/validphys/filters.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/validphys2/src/validphys/filters.py b/validphys2/src/validphys/filters.py index 8dc8526fb7..e4b937c894 100644 --- a/validphys2/src/validphys/filters.py +++ b/validphys2/src/validphys/filters.py @@ -16,7 +16,7 @@ from reportengine.compat import yaml import validphys.cuts from validphys.process_options import PROCESSES -from validphys.utils import freeze_args, generate_path_filtered_data +from validphys.utils import generate_path_filtered_data log = logging.getLogger(__name__) @@ -684,7 +684,6 @@ def _make_point_namespace(self, dataset, idat) -> dict: return ns -@freeze_args @functools.lru_cache def get_cuts_for_dataset(commondata, rules) -> list: """Function to generate a list containing the index From 8858d30491646c898298444074b81247a0142528 Mon Sep 17 00:00:00 2001 From: Mark Nestor Costantini Date: Sat, 18 May 2024 11:20:29 +0100 Subject: [PATCH 20/24] removed freeze_args and make_hashable functions as they are not used anywhere --- validphys2/src/validphys/utils.py | 31 ------------------------------- 1 file changed, 31 deletions(-) diff --git a/validphys2/src/validphys/utils.py b/validphys2/src/validphys/utils.py index 64e0e6d5de..1c77fd5b5d 100644 --- a/validphys2/src/validphys/utils.py +++ b/validphys2/src/validphys/utils.py @@ -1,42 +1,11 @@ import contextlib -import functools import pathlib import shutil import tempfile -from typing import Any, Hashable, Mapping, Sequence -from frozendict import frozendict import numpy as np -def make_hashable(obj: Any): - # So that we don't infinitely recurse since frozenset and tuples - # are Sequences. - if isinstance(obj, Hashable): - return obj - elif isinstance(obj, Mapping): - return frozendict(obj) - elif isinstance(obj, Sequence): - return tuple([make_hashable(i) for i in obj]) - else: - raise ValueError("Object is not hashable") - - -def freeze_args(func): - """Transform mutable dictionary - Into immutable - Useful to be compatible with cache - """ - - @functools.wraps(func) - def wrapped(*args, **kwargs): - args = tuple([make_hashable(arg) for arg in args]) - kwargs = {k: make_hashable(v) for k, v in kwargs.items()} - return func(*args, **kwargs) - - return wrapped - - def generate_path_filtered_data(fit_path, setname): """Utility to ensure that both the loader and tools like setupfit utilize the same convention to generate the names of generated pseudodata""" From feae109b61069efa926d4f3c1e1cd8ddb1d10e00 Mon Sep 17 00:00:00 2001 From: Mark Nestor Costantini Date: Sat, 18 May 2024 11:22:13 +0100 Subject: [PATCH 21/24] removed frozendict dependency from pyproject and conda-recipe/meta.yaml file --- conda-recipe/meta.yaml | 1 - pyproject.toml | 1 - 2 files changed, 2 deletions(-) diff --git a/conda-recipe/meta.yaml b/conda-recipe/meta.yaml index 2a3adf3859..03cb440bca 100644 --- a/conda-recipe/meta.yaml +++ b/conda-recipe/meta.yaml @@ -39,7 +39,6 @@ requirements: - pineappl >=0.7.3 - eko >=0.14.2 - fiatlux - - frozendict # needed for caching of data loading - sphinx >=5.0.2,<6 # documentation. Needs pinning temporarily due to markdown - recommonmark - sphinx_rtd_theme >0.5 diff --git a/pyproject.toml b/pyproject.toml index 083545744f..970d3d5b1b 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -67,7 +67,6 @@ pandas = "*" numpy = "*" validobj = "*" prompt_toolkit = "*" -frozendict = "*" # validphys: needed for caching of data loading # Reportengine needs to be installed from git reportengine = { git = "https://github.com/NNPDF/reportengine" } # Fit From 3d7f3194ace3c51435d4d0f7e429ef578dc90f68 Mon Sep 17 00:00:00 2001 From: Mark Nestor Costantini Date: Sat, 18 May 2024 12:16:59 +0100 Subject: [PATCH 22/24] pass rules as tuple in test --- validphys2/src/validphys/tests/test_filter_rules.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/validphys2/src/validphys/tests/test_filter_rules.py b/validphys2/src/validphys/tests/test_filter_rules.py index 4d864edd02..09055fa821 100644 --- a/validphys2/src/validphys/tests/test_filter_rules.py +++ b/validphys2/src/validphys/tests/test_filter_rules.py @@ -103,7 +103,7 @@ def test_good_rules(): dsnames = ['ATLAS_1JET_8TEV_R06_PTY', 'NMC_NC_NOTFIXED_DW_EM-F2'] for dsname in dsnames: ds = l.check_dataset( - dsname, cuts='internal', rules=rules, theoryid=THEORYID, variant="legacy" + dsname, cuts='internal', rules=tuple(rules), theoryid=THEORYID, variant="legacy" ) assert ds.cuts.load() is not None From c2b5659e429d0aa91367e0fa73d91021547accd5 Mon Sep 17 00:00:00 2001 From: Mark Nestor Costantini <85164495+comane@users.noreply.github.com> Date: Mon, 20 May 2024 18:27:35 +0100 Subject: [PATCH 23/24] Update validphys2/src/validphys/config.py Co-authored-by: Juan M. Cruz-Martinez --- validphys2/src/validphys/config.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/validphys2/src/validphys/config.py b/validphys2/src/validphys/config.py index a0288419f4..15d71d694e 100644 --- a/validphys2/src/validphys/config.py +++ b/validphys2/src/validphys/config.py @@ -1413,7 +1413,7 @@ def produce_defaults( if w2min is not None and "w2min" in filter_defaults and w2min != filter_defaults["w2min"]: raise ConfigError("w2min defined multiple times with different values") - if maxTau is not None and filter_defaults["maxTau"] != maxTau: + if maxTau is not None and filter_defaults.get("maxTau", maxTau) != maxTau: raise ConfigError("maxTau defined multiple times with different values") if default_filter_settings_recorded_spec_ is not None: From fac52184d0a2c1d1900f2add54272c479d608dd7 Mon Sep 17 00:00:00 2001 From: Mark Nestor Costantini <85164495+comane@users.noreply.github.com> Date: Mon, 20 May 2024 18:28:01 +0100 Subject: [PATCH 24/24] Update validphys2/src/validphys/config.py Co-authored-by: Juan M. Cruz-Martinez --- validphys2/src/validphys/config.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/validphys2/src/validphys/config.py b/validphys2/src/validphys/config.py index 15d71d694e..669e95553f 100644 --- a/validphys2/src/validphys/config.py +++ b/validphys2/src/validphys/config.py @@ -1421,7 +1421,7 @@ def produce_defaults( # If we find recorded specs return immediately and don't read q2min and w2min # from runcard return filter_defaults - elif not isinstance(filter_defaults, FilterDefaults): + elif not filter_defaults: # if filter_defaults have not been set, load the defaults with default_filter_settings_input filter_defaults = default_filter_settings_input().to_dict() defaults_loaded = True