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

@@ -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
 

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

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,17 +525,33 @@ 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)
 
         # The list comprehension here creates a list of all duplicate items
         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)

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.