From 2330f2652b4fae1290006479cf5f3b23a01eaa57 Mon Sep 17 00:00:00 2001 From: Gareth S Cabourn Davies Date: Tue, 13 Jun 2023 13:02:37 +0100 Subject: [PATCH] Add a possibility for defaultvalues sections in config files (#4385) * Add a possibility for defaultvalues sections in config files * do not allow [section] and [section-defaultvalues] options * defaultvalues for top-level sections only * underscore on add_ini_opts --- examples/search/analysis.ini | 10 ++++-- examples/search/plotting.ini | 4 ++- pycbc/types/config.py | 63 +++++++++++++++++++++++++++++------- pycbc/workflow/core.py | 26 +++++++++++---- 4 files changed, 82 insertions(+), 21 deletions(-) diff --git a/examples/search/analysis.ini b/examples/search/analysis.ini index 74a47baaac2..7a405001444 100644 --- a/examples/search/analysis.ini +++ b/examples/search/analysis.ini @@ -150,9 +150,11 @@ autogating-pad = ${inspiral|autogating-pad} [bank2hdf] +[fit_by_template-defaultvalues] +sngl-ranking = newsnr_sgveto_psdvar_scaled_threshold + [fit_by_template] fit-function = exponential -sngl-ranking = newsnr_sgveto_psdvar_scaled_threshold stat-threshold = 4.0 prune-param = mtotal log-prune-param = @@ -172,17 +174,19 @@ sngl-ranking = newsnr_sgveto_psdvar randomize-template-order = statistic-files = ${resolve:./statHL.hdf} ${resolve:./statLV.hdf} ${resolve:./statHV.hdf} ${resolve:./statHLV.hdf} -[coinc-full_data] +[coinc-defaultvalues] timeslide-interval = 0.1 +[coinc-full_data] + [coinc-full_data-2det] loudest-keep-values = [-1:5,1:5] [coinc-full_data-3det] loudest-keep-values = [-3:5,-1:5] +timeslide-interval = 0.11 [coinc-fullinj&coinc-injfull] -timeslide-interval = ${coinc-full_data|timeslide-interval} cluster-window = ${statmap|cluster-window} loudest-keep-values = 15.0:9999999999999 diff --git a/examples/search/plotting.ini b/examples/search/plotting.ini index 525baeae13c..5fdd63e181c 100644 --- a/examples/search/plotting.ini +++ b/examples/search/plotting.ini @@ -193,8 +193,10 @@ x-var = newsnr x-min = 6 x-max = 15 +[plot_binnedhist-defaultvalues] +sngl-ranking = ${fit_by_template-defaultvalues|sngl-ranking} + [plot_binnedhist] -sngl-ranking = ${fit_by_template|sngl-ranking} fit-function = ${fit_by_template|fit-function} ; limit the number of triggers for which duration is calculated stat-threshold = 5.0 diff --git a/pycbc/types/config.py b/pycbc/types/config.py index e7ef900aa02..455f39f26bc 100644 --- a/pycbc/types/config.py +++ b/pycbc/types/config.py @@ -241,7 +241,7 @@ def get_subsections(self, section_name): for sec in subsections: sp = sec.split("-") - # This is unusual, but a format [section-subsection-tag] is okay. Just + # The format [section-subsection-tag] is okay. Just # check that [section-subsection] section exists. If not it is possible # the user is trying to use an subsection name with '-' in it if (len(sp) > 1) and not self.has_section( @@ -442,7 +442,7 @@ def add_options_to_section(self, section, items, overwrite_options=False): provided items. This will override so that the options+values given in items will replace the original values if the value is set to True. - Default = True + Default = False """ # Sanity checking if not self.has_section(section): @@ -463,22 +463,34 @@ def add_options_to_section(self, section, items, overwrite_options=False): def sanity_check_subsections(self): """ - This function goes through the ConfigParset and checks that any options + This function goes through the ConfigParser and checks that any options given in the [SECTION_NAME] section are not also given in any [SECTION_NAME-SUBSECTION] sections. """ # Loop over the sections in the ini file for section in self.sections(): - # [pegasus_profile] specially is allowed to be overriden by + # [pegasus_profile] is specially allowed to be overriden by # sub-sections if section == "pegasus_profile": continue + if section.endswith('-defaultvalues') and \ + not len(section.split('-')) == 2: + # Only allow defaultvalues for top-level sections + raise NotImplementedError( + "-defaultvalues subsections are only allowed for " + "top-level sections; given %s" % section + ) + # Loop over the sections again for section2 in self.sections(): # Check if any are subsections of section if section2.startswith(section + "-"): + if section2.endswith("defaultvalues"): + # defaultvalues is storage for defaults, and will + # be over-written by anything in the sections-proper + continue # Check for duplicate options whenever this exists self.check_duplicate_options( section, section2, raise_error=True @@ -513,6 +525,21 @@ def check_duplicate_options(self, section1, section2, raise_error=False): "Section %s not present in ConfigParser." % (section2,) ) + # Are section1 and section2 a section-and-defaultvalues pair? + section_and_default = (section1 == f"{section2}-defaultvalues" or + section2 == f"{section1}-defaultvalues") + + # Is one the sections defaultvalues, but the other is not the + # top-level section? This is to catch the case where we are + # comparing section-defaultvalues with section-subsection + if section1.endswith("-defaultvalues") or \ + section2.endswith("-defaultvalues"): + if not section_and_default: + # Override the raise_error variable not to error when + # defaultvalues are given and the sections are not + # otherwise the same + raise_error = False + items1 = self.options(section1) items2 = self.options(section2) @@ -520,10 +547,11 @@ def check_duplicate_options(self, section1, section2, raise_error=False): duplicates = [x for x in items1 if x in items2] if duplicates and raise_error: - raise ValueError( - "The following options appear in both section " - + "%s and %s: %s" % (section1, section2, " ".join(duplicates)) - ) + err_msg = ("The following options appear in both section " + f"{section1} and {section2}: " + ", ".join(duplicates)) + if section_and_default: + err_msg += ". Default values are unused in this case." + raise ValueError(err_msg) return duplicates @@ -556,8 +584,9 @@ def get_opt_tags(self, section, option, tags): """ Supplement to ConfigParser.ConfigParser.get(). This will search for an option in [section] and if it doesn't find it will also try in - [section-tag] for every value of tag in tags. - Will raise a ConfigParser.Error if it cannot find a value. + [section-defaultvalues], and [section-tag] for every value of tag + in tags. [section-tag] will be preferred to [section-defaultvalues] + values. Will raise a ConfigParser.Error if it cannot find a value. Parameters ----------- @@ -587,6 +616,14 @@ def get_opt_tags(self, section, option, tags): if not tags: raise ConfigParser.Error(err_string + ".") return_vals = [] + # First, check if there are any default values set: + has_defaultvalue = False + if self.has_section(f"{section}-defaultvalues"): + return_vals.append( + self.get(f"{section}-defaultvalues", option) + ) + has_defaultvalue = True + sub_section_list = [] for sec_len in range(1, len(tags) + 1): for tag_permutation in itertools.permutations(tags, sec_len): @@ -602,8 +639,12 @@ def get_opt_tags(self, section, option, tags): self.get("%s-%s" % (section, sub), option) ) - # We also want to recursively go into sections + if has_defaultvalue and len(return_vals) > 1: + # option supplied which should overwrite the default; + # default will be first in the list, so remove it + return_vals = return_vals[1:] + # We also want to recursively go into sections if not return_vals: err_string += "or in sections [%s]." % ( "] [".join(section_list) diff --git a/pycbc/workflow/core.py b/pycbc/workflow/core.py index d0bdcccba2a..28c8fa05cad 100644 --- a/pycbc/workflow/core.py +++ b/pycbc/workflow/core.py @@ -315,22 +315,30 @@ def add_ini_profile(self, cp, sec): key = opt.split('|')[1] self.add_profile(namespace, key, value) - def add_ini_opts(self, cp, sec): + def _add_ini_opts(self, cp, sec, ignore_existing=False): """Add job-specific options from configuration file. Parameters ----------- cp : ConfigParser object - The ConfigParser object holding the workflow configuration settings + The ConfigParser object holding the workflow configuration + settings sec : string The section containing options for this job. """ for opt in cp.options(sec): + if opt in self.all_added_options: + if ignore_existing: + continue + else: + raise ValueError("Option %s has already been added" % opt) + self.all_added_options.add(opt) + value = cp.get(sec, opt).strip() - opt = '--%s' %(opt,) + opt = f'--{opt}' if opt in self.file_input_options: # This now expects the option to be a file - # Check is we have a list of files + # Check if we have a list of files values = [path for path in value.split(' ') if path] self.common_raw_options.append(opt) @@ -380,6 +388,7 @@ def add_ini_opts(self, cp, sec): # stuff later. self.unresolved_td_options[opt] = value else: + # This option comes from the config file(s) self.common_options += [opt, value] def add_opt(self, opt, value=None): @@ -536,7 +545,6 @@ def update_current_tags(self, tags): self.tagged_name = "{0}-{1}".format(self.tagged_name, self.ifo_string) - # Determine the sections from the ini file that will configure # this executable sections = [self.name] @@ -562,18 +570,24 @@ def update_current_tags(self, tags): # collect the options and profile information # from the ini file section(s) + self.all_added_options = set() self.common_options = [] self.common_raw_options = [] self.unresolved_td_options = {} self.common_input_files = [] for sec in sections: if self.cp.has_section(sec): - self.add_ini_opts(self.cp, sec) + self._add_ini_opts(self.cp, sec) else: warn_string = "warning: config file is missing section " warn_string += "[{0}]".format(sec) logging.warn(warn_string) + # get uppermost section + if self.cp.has_section(f'{self.name}-defaultvalues'): + self._add_ini_opts(self.cp, f'{self.name}-defaultvalues', + ignore_existing=True) + def update_output_directory(self, out_dir=None): """Update the default output directory for output files.