Detect unknown parameters

Given that we now treat the parameter namespaces completly dynamically
with **kwds; typos, obsolete parameter names and other mistakes pass
silently. This leads to lots of annoying and difficult to track bugs
and confusion.

This PR introduces detection of unexpected parameters:

+ Each city may provide a class attribute named parameters, being a
  sequence of parameter names that are recognized at this level, but
  not recognized by the superclass.

+ At construction time, the construction arguments are checked for the
  presence of parameters which have not been declared at the city's
  level or its superclasses. If an unexpected parameter is found, an
  AssertionError is raised. (This should probably be replaced by a new
  IC exception.)

invisible_cities/cities/base_cities.py

@@ -7,10 +7,11 @@
 """
 
 import sys
-from argparse import Namespace
-from glob     import glob
-from time     import time
-from os.path  import expandvars
+from argparse  import Namespace
+from glob      import glob
+from time      import time
+from os.path   import expandvars
+from itertools import chain
 
 import numpy  as np
 import tables as tb
@@ -29,6 +30,7 @@
 from .. core.exceptions         import ClusterEmptyList
 from .. core.exceptions         import XYRecoFail
 from .. core.exceptions         import InitializedEmptyPmapObject
+from .. core.exceptions         import UnknownParameter
 from .. core.system_of_units_c  import units
 from .. core.random_sampling    import NoiseSampler as SiPMsNoiseSampler
 
@@ -103,6 +105,11 @@ class City:
        several calibration coefficients
     """
 
+    parameters = tuple("""print_config_only hide_config no_overrides
+        no_files full_files config_file
+        files_in file_out compression
+        run_number print_mod event_range verbosity print_mod daemons""".split())
+
     def __init__(self, **kwds):
         """The init method of a city handles:
         1. provides access to an instance of counters (cnt) to be used by derived cities.
@@ -111,7 +118,7 @@ def __init__(self, **kwds):
         4. provides access to the data base.
         """
 
-        self.cnt = Counters(n_events_for_range = 0)
+        self.detect_unknown_parameters(kwds)
 
         conf = Namespace(**kwds)
         self.conf = conf
@@ -122,15 +129,25 @@ def __init__(self, **kwds):
         if not hasattr(conf, 'file_out'):
             raise NoOutputFile
 
-
+        self.cnt = Counters(n_events_for_range = 0)
         self.input_files  = sorted(glob(expandvars(conf.files_in)))
         self.output_file  =             expandvars(conf.file_out)
         self.compression  = conf.compression
         self.run_number   = conf.run_number
-        self.nprint       = conf.nprint  # default print frequency
+        self.print_mod    = conf.print_mod  # default print frequency
         self.first_event, self.last_event = self._event_range()
         self.set_up_database()
 
+    def detect_unknown_parameters(self, kwds):
+        known = self.allowed_parameters()
+        for name in kwds:
+            if name not in known:
+                raise UnknownParameter('{} does not expect {}.'.format(self.__class__.__name__, name))
+
+    @classmethod
+    def allowed_parameters(cls):
+        return set(chain.from_iterable(base.parameters for base in cls.__mro__ if hasattr(base, 'parameters')))
+
     def _event_range(self):
         if not hasattr(self.conf, 'event_range'): return None, None
         er = self.conf.event_range
@@ -271,7 +288,7 @@ def monte_carlo(self):
         return self.run_number <= 0
 
     def conditional_print(self, evt, n_events_tot):
-        if n_events_tot % self.nprint == 0:
+        if n_events_tot % self.print_mod == 0:
             print('event in file = {}, total = {}'
                   .format(evt, n_events_tot))
 
@@ -423,9 +440,10 @@ class DeconvolutionCity(RawCity):
        thr_trigger in the rising signal (thr_trigger).
 
        Since a Deconvolution city reads RWF, it is also a RawCity.
-
     """
 
+    parameters = tuple("""raw_data_type n_baseline thr_trigger acum_discharge_length""".split())
+
     def __init__(self, **kwds):
         super().__init__(**kwds)
         conf = self.conf
@@ -475,6 +493,8 @@ class CalibratedCity(DeconvolutionCity):
              MAU + threshold.
        """
 
+    parameters = tuple("""n_mau thr_mau thr_csum_s1 thr_csum_s2 n_mau_sipm thr_sipm""".split())
+
     def __init__(self, **kwds):
 
         super().__init__(**kwds)
@@ -524,9 +544,13 @@ def calibrated_signal_sipm(self, SiRWF):
 class PmapCity(CalibratedCity):
     """A PMAP city extends a CalibratedCity, computing the S1, S2 and S2Si
        objects that togehter constitute a PMAP.
-
     """
 
+    parameters = tuple("""compute_ipmt_pmaps
+      s1_tmin s1_tmax s1_stride s1_lmin s1_lmax s1_rebin_stride
+      s2_tmin s2_tmax s2_stride s2_lmin s2_lmax s2_rebin_stride
+      thr_sipm_s2""".split())
+
     def __init__(self, **kwds):
         super().__init__(**kwds)
         conf = self.conf
@@ -585,6 +609,9 @@ def pmaps(self, s1_indx, s2_indx, ccwf, csum, sipmzs):
 
 class DstCity(City):
     """A DstCity reads a list of KDSTs """
+
+    parameters = tuple("""dst_group dst_node""".split())
+
     def __init__(self, **kwds):
         super().__init__(**kwds)
 
@@ -639,6 +666,14 @@ def __init__(self, **kwds):
         #self.reco_algorithm = find_algorithm(self.conf.reco_algorithm)
 
 
+    parameters = tuple("""""".split())
+    parameters = tuple("""lm_radius new_lm_radius
+        msipm drift_v
+        qlm qthr rebin
+        s1_nmin s1_nmax s1_emin s1_emax s1_wmin s1_wmax s1_hmin s1_hmax s1_ethr
+        s2_nmin s2_nmax s2_emin s2_emax s2_wmin s2_wmax s2_hmin s2_hmax s2_ethr
+        s2_nsipmmin s2_nsipmmax""".split())
+
     def compute_xy_position(self, s2si, peak_no):
         """
         Computes position using the integral of the charge
@@ -744,22 +779,17 @@ def create_kr_event(self, pmapVectors):
 
 
 class HitCity(KrCity):
-    """A city that reads PMPAS and computes/writes a hit event"""
+    """A city that reads PMAPS and computes/writes a hit event"""
+
     def __init__(self, **kwds):
         super().__init__(**kwds)
         self.rebin  = self.conf.rebin
 
     def compute_xy_position(self, s2sid_peak, slice_no):
         """Compute x-y position for each time slice. """
-        #import pdb; pdb.set_trace()
         IDs, Qs  = cpmp.sipm_ids_and_charges_in_slice(s2sid_peak, slice_no)
         xs, ys   = self.xs[IDs], self.ys[IDs]
 
-        # print('compute_xy_position')
-        # print('s2si.s2sid[peak_no] = {}'.format(s2sid_peak))
-        # print('IDs = {}'.format(IDs))
-        # print('Qs = {}'.format(Qs))
-
         return corona(np.stack((xs, ys)).T, Qs,
                       Qthr           =  self.conf.qthr,
                       Qlm            =  self.conf.qlm,
@@ -873,9 +903,12 @@ class TriggerEmulationCity(PmapCity):
     """Emulates the trigger in the FPGA.
        1. It is a PmapCity since the FPGA performs deconvolution and PMAP
        searches to set the trigger.
-
     """
 
+    parameters = tuple("""tr_channels min_number_channels
+        min_height max_height min_width max_width min_charge max_charge
+        data_mc_ratio""".split())
+
     def __init__(self, **kwds):
         super().__init__(**kwds)
         self.trigger_params   = self.trigger_parameters()
@@ -921,9 +954,10 @@ class MonteCarloCity(TriggerEmulationCity):
         that transforms MCRD in RWF.
      2. Emulates the trigger prepocessor: the functionality is provided
         by the inheritance from TriggerEmulationCity.
-
     """
 
+    parameters = tuple("""sipm_noise_cut""".split())
+
     def __init__(self, **kwds):
         super().__init__(**kwds)
         # Create instance of the noise sampler

invisible_cities/cities/city_test.py

@@ -8,10 +8,11 @@
 last changed: 01-12-2017
 """
 
-from   pytest import raises
+from pytest import raises
 
 from .. core.exceptions import NoInputFiles
 from .. core.exceptions import NoOutputFile
+from .. core.exceptions import UnknownParameter
 from .  base_cities     import City
 
 
@@ -22,3 +23,7 @@ def test_no_input_files_raises_NoInputFiles():
 def test_no_output_files_raises_NoOutptFiles():
     with raises(NoOutputFile):
         City(files_in = 'dummy/input/files')
+
+def test_unknown_parameters_are_dectected():
+    with raises(UnknownParameter):
+        City(a_parameter_that_was_not_expected=0)

invisible_cities/cities/diomira.py

@@ -46,6 +46,8 @@ class Diomira(MonteCarloCity):
     2. the response of the trigger
     """
 
+    parameters = tuple("""trigger_type""".split())
+
     def __init__(self, **kwds):
         """Diomira Init:
         1. inits base city

invisible_cities/cities/diomira_test.py

@@ -89,11 +89,10 @@ def test_diomira_copy_mc_and_offset(config_tmpdir):
 
     conf = configure('diomira invisible_cities/config/diomira.conf'.split())
 
-    conf.update(dict(run_number = run_number,
-                     files_in   = PATH_IN,
-                     file_out   = PATH_OUT,
-                     first_evt  = start_evt,
-                     nmax       = nrequired))
+    conf.update(dict(run_number  = run_number,
+                     files_in    = PATH_IN,
+                     file_out    = PATH_OUT,
+                     event_range = (start_evt, start_evt + nrequired)))
 
     diomira = Diomira(**conf)
     diomira.run()

invisible_cities/cities/dorothea_test.py

@@ -44,7 +44,7 @@ def test_dorothea_KrMC(config_tmpdir, KrMC_pmaps):
                      s2_ethr     =      1 * pes,
                      s2_nsipmmin =      2,
                      s2_nsipmmax =   1000,
-                     nmax        = nrequired))
+                     event_range = (0, nrequired)))
 
 
     dorothea = Dorothea(**conf)

invisible_cities/cities/irene_test.py

@@ -46,14 +46,11 @@ def unpack_s12params(s12params):
     s1par, s2par = s12params
     return dict(s1_tmin          = s1par.time.min,
                 s1_tmax          = s1par.time.max,
-                s1_string        = s1par.stride,
-                s1_rebin_stride  = s1par.rebin_stride,
                 s1_lmin          = s1par.length.min,
                 s1_lmax          = s1par.length.max,
 
                 s2_tmin          = s2par.time.min,
                 s2_tmax          = s2par.time.max,
-                s2_string        = s2par.stride,
                 s2_rebin_stride  = s2par.rebin_stride,
                 s2_lmin          = s2par.length.min,
                 s2_lmax          = s2par.length.max)
@@ -90,7 +87,7 @@ def test_irene_electrons_40keV(config_tmpdir, ICDIR, s12params):
 
     conf = configure('dummy invisible_cities/config/irene.conf'.split())
     conf.update(dict(run_number   = 0,
-                     filesin      = PATH_IN,
+                     files_in     = PATH_IN,
                      file_out     = PATH_OUT,
                      event_range  = (0, nrequired),
                      **unpack_s12params(s12params)))

invisible_cities/cities/isidora_test.py

@@ -35,7 +35,7 @@ def test_isidora_electrons_40keV(config_tmpdir, ICDIR):
 
     conf = configure('dummy invisible_cities/config/isidora.conf'.split())
     conf.update(dict(run_number   = 0,
-                     filesin      = PATH_IN,
+                     files_in     = PATH_IN,
                      file_out     = PATH_OUT,
                      event_range  = (0, nrequired)))
 

invisible_cities/cities/penthesilea.py

@@ -26,6 +26,7 @@
 
 class Penthesilea(HitCity):
     """Read PMAPS and produces hits and beyond"""
+
     def __init__(self, **kwds):
         """actions:
         1. inits base city

invisible_cities/cities/zaira.py

@@ -21,14 +21,20 @@
 
 class Zaira(DstCity):
 
+    parameters = tuple("""lifetime u_lifetime
+        xbins xmin xmax
+        ybins ymin ymax
+        zbins zmin zmax     tbins
+        fiducial_r fiducial_z fiducial_e""".split())
+
     def __init__(self, **kwds):
         """Zaira Init:
         1. inits base city
         2. inits counters
         3. defines fiducial
         4. gets dst info
-
         """
+
         super().__init__(**kwds)
         self.set_lifetime_correction()
         self.set_xybins_and_range()

invisible_cities/config/city.conf

@@ -19,7 +19,7 @@ compression = 'ZLIB4'
 run_number = 0
 
 # How frequently to print events
-nprint = 1
+print_mod = 1
 
 # max number of events to run
 event_range =  1,

invisible_cities/config/irene.conf

@@ -5,7 +5,5 @@
 
 include('$ICDIR/config/pmap_city.conf')
 
-# print empty events
-print_empty_events = 1
 # daemons
 # daemons = ['lyra','asriel']

invisible_cities/config/irene_daemon_example.conf

@@ -5,7 +5,5 @@
 
 include('$ICDIR/config/pmap_city.conf')
 
-# print empty events
-print_empty_events = 1
 # daemons
 daemons = ['lyra','asriel']

invisible_cities/core/configure_test.py

@@ -32,9 +32,6 @@
 # set_print
 nprint = {nprint}
 
-# print empty events (skipped)
-print_empty_events = {print_empty_events}
-
 # set_blr
 nbaseline   = {nbaseline}
 thr_trigger = {thr_trigger}
@@ -74,7 +71,6 @@
                         compression        = 'ZLIB4',
                         run_number         = 23,
                         nprint             = 24,
-                        print_empty_events = 25,
                         nbaseline          = 26,
                         thr_trigger        = 27,
                         nmau               = 28,
@@ -291,7 +287,7 @@ def simple_conf_file_name(tmpdir_factory):
     write_config_file(file_name, """
 compression  = 'ZLIB4'
 run_number   = 12
-nprint       = 13
+print_mod    = 13
 event_range  = 14,
 """)
     return str(file_name)

invisible_cities/core/exceptions.py

@@ -65,3 +65,6 @@ class NegativeThresholdNotAllowed(ICException):
 
 class InitializedEmptyPmapObject(ICException):
     pass
+
+class UnknownParameter(ICException):
+    pass