From 915f235040576911058d36eac4b34cd58a0a8b11 Mon Sep 17 00:00:00 2001 From: Jacek Generowicz Date: Thu, 7 Sep 2017 18:51:18 +0200 Subject: [PATCH 01/12] Typo --- invisible_cities/cities/base_cities.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/invisible_cities/cities/base_cities.py b/invisible_cities/cities/base_cities.py index 386124d3bb..24c0ea1ab3 100644 --- a/invisible_cities/cities/base_cities.py +++ b/invisible_cities/cities/base_cities.py @@ -744,7 +744,7 @@ 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 From 284f27cb0cbad02b7b6eb478a1d76f388a66ac72 Mon Sep 17 00:00:00 2001 From: Jacek Generowicz Date: Thu, 7 Sep 2017 20:35:36 +0200 Subject: [PATCH 02/12] Introduce detection of unexpected parameters --- invisible_cities/cities/base_cities.py | 53 +++++++++++++++++++++++--- invisible_cities/cities/diomira.py | 2 + invisible_cities/cities/zaira.py | 6 +++ 3 files changed, 55 insertions(+), 6 deletions(-) diff --git a/invisible_cities/cities/base_cities.py b/invisible_cities/cities/base_cities.py index 24c0ea1ab3..54cd2e83d6 100644 --- a/invisible_cities/cities/base_cities.py +++ b/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 @@ -103,6 +104,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 +117,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,7 +128,7 @@ 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 @@ -131,6 +137,15 @@ def __init__(self, **kwds): 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: + assert name in known, (name, self.__class__.__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 @@ -426,6 +441,8 @@ class DeconvolutionCity(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 +492,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) @@ -527,6 +546,11 @@ class PmapCity(CalibratedCity): """ + 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 @@ -876,6 +911,10 @@ class TriggerEmulationCity(PmapCity): """ + 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() @@ -924,6 +963,8 @@ class MonteCarloCity(TriggerEmulationCity): """ + parameters = tuple("""sipm_noise_cut""".split()) + def __init__(self, **kwds): super().__init__(**kwds) # Create instance of the noise sampler diff --git a/invisible_cities/cities/diomira.py b/invisible_cities/cities/diomira.py index 736fedf184..46b169f6d1 100644 --- a/invisible_cities/cities/diomira.py +++ b/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 diff --git a/invisible_cities/cities/zaira.py b/invisible_cities/cities/zaira.py index edb60b21e1..fc49b5fe1a 100644 --- a/invisible_cities/cities/zaira.py +++ b/invisible_cities/cities/zaira.py @@ -21,6 +21,12 @@ 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 From ed955e94e93f101cfd16898fe33d29269d92349c Mon Sep 17 00:00:00 2001 From: Jacek Generowicz Date: Thu, 7 Sep 2017 20:36:48 +0200 Subject: [PATCH 03/12] Replace remaining nprint with print_mod --- invisible_cities/cities/base_cities.py | 4 ++-- invisible_cities/config/city.conf | 2 +- invisible_cities/core/configure_test.py | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/invisible_cities/cities/base_cities.py b/invisible_cities/cities/base_cities.py index 54cd2e83d6..9cddd5380b 100644 --- a/invisible_cities/cities/base_cities.py +++ b/invisible_cities/cities/base_cities.py @@ -133,7 +133,7 @@ def __init__(self, **kwds): 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() @@ -286,7 +286,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)) diff --git a/invisible_cities/config/city.conf b/invisible_cities/config/city.conf index f9b7d4ec08..cd537d78b5 100644 --- a/invisible_cities/config/city.conf +++ b/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, diff --git a/invisible_cities/core/configure_test.py b/invisible_cities/core/configure_test.py index e8a59cb529..ce98fc7197 100644 --- a/invisible_cities/core/configure_test.py +++ b/invisible_cities/core/configure_test.py @@ -291,7 +291,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) From ace87ffa11ca6ed7be1ed34758d93c906ec8d275 Mon Sep 17 00:00:00 2001 From: Jacek Generowicz Date: Thu, 7 Sep 2017 20:38:06 +0200 Subject: [PATCH 04/12] Sanitize blank lines --- invisible_cities/cities/base_cities.py | 5 +---- invisible_cities/cities/penthesilea.py | 1 + invisible_cities/cities/zaira.py | 2 +- 3 files changed, 3 insertions(+), 5 deletions(-) diff --git a/invisible_cities/cities/base_cities.py b/invisible_cities/cities/base_cities.py index 9cddd5380b..e3d3e3cecc 100644 --- a/invisible_cities/cities/base_cities.py +++ b/invisible_cities/cities/base_cities.py @@ -438,7 +438,6 @@ 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()) @@ -543,7 +542,6 @@ 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 @@ -780,6 +778,7 @@ def create_kr_event(self, pmapVectors): class HitCity(KrCity): """A city that reads PMAPS and computes/writes a hit event""" + def __init__(self, **kwds): super().__init__(**kwds) self.rebin = self.conf.rebin @@ -908,7 +907,6 @@ 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 @@ -960,7 +958,6 @@ 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()) diff --git a/invisible_cities/cities/penthesilea.py b/invisible_cities/cities/penthesilea.py index 4f34f0def8..8914e9a807 100644 --- a/invisible_cities/cities/penthesilea.py +++ b/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 diff --git a/invisible_cities/cities/zaira.py b/invisible_cities/cities/zaira.py index fc49b5fe1a..72d780ebe5 100644 --- a/invisible_cities/cities/zaira.py +++ b/invisible_cities/cities/zaira.py @@ -33,8 +33,8 @@ def __init__(self, **kwds): 2. inits counters 3. defines fiducial 4. gets dst info - """ + super().__init__(**kwds) self.set_lifetime_correction() self.set_xybins_and_range() From 6c5b110c8d67e518160bdb1e981a2bf7e0f2de71 Mon Sep 17 00:00:00 2001 From: Jacek Generowicz Date: Thu, 7 Sep 2017 20:38:27 +0200 Subject: [PATCH 05/12] Remove commented-out code --- invisible_cities/cities/base_cities.py | 6 ------ 1 file changed, 6 deletions(-) diff --git a/invisible_cities/cities/base_cities.py b/invisible_cities/cities/base_cities.py index e3d3e3cecc..38e321851d 100644 --- a/invisible_cities/cities/base_cities.py +++ b/invisible_cities/cities/base_cities.py @@ -785,15 +785,9 @@ def __init__(self, **kwds): 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, From 40285c924e8d78a5bfbbdbabb2a0ef8b2625c5d9 Mon Sep 17 00:00:00 2001 From: Jacek Generowicz Date: Thu, 7 Sep 2017 20:41:28 +0200 Subject: [PATCH 06/12] Replace remaining nmax & first_event with event_range --- invisible_cities/cities/diomira_test.py | 9 ++++----- invisible_cities/cities/dorothea_test.py | 2 +- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/invisible_cities/cities/diomira_test.py b/invisible_cities/cities/diomira_test.py index 7ff80f1400..acfbac2182 100644 --- a/invisible_cities/cities/diomira_test.py +++ b/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() diff --git a/invisible_cities/cities/dorothea_test.py b/invisible_cities/cities/dorothea_test.py index 86ec428dcf..442f375476 100644 --- a/invisible_cities/cities/dorothea_test.py +++ b/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) From 2b5337fb95e6915dd1b7b26b2147ac9766f46ec4 Mon Sep 17 00:00:00 2001 From: Jacek Generowicz Date: Thu, 7 Sep 2017 20:41:53 +0200 Subject: [PATCH 07/12] Fix filesin -> files_in --- invisible_cities/cities/irene_test.py | 2 +- invisible_cities/cities/isidora_test.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/invisible_cities/cities/irene_test.py b/invisible_cities/cities/irene_test.py index ba07509730..f8e294586c 100644 --- a/invisible_cities/cities/irene_test.py +++ b/invisible_cities/cities/irene_test.py @@ -90,7 +90,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))) diff --git a/invisible_cities/cities/isidora_test.py b/invisible_cities/cities/isidora_test.py index 99c49ae270..c888d90d87 100644 --- a/invisible_cities/cities/isidora_test.py +++ b/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))) From aa0eccba71c0863d0913cf700a6bae5ec0e7d779 Mon Sep 17 00:00:00 2001 From: Jacek Generowicz Date: Thu, 7 Sep 2017 20:42:21 +0200 Subject: [PATCH 08/12] Eliminate print_empty_events (not *used* anywhere) --- invisible_cities/config/irene.conf | 2 -- invisible_cities/config/irene_daemon_example.conf | 2 -- invisible_cities/core/configure_test.py | 4 ---- 3 files changed, 8 deletions(-) diff --git a/invisible_cities/config/irene.conf b/invisible_cities/config/irene.conf index c2cd5ebef4..f1dcd1c16b 100644 --- a/invisible_cities/config/irene.conf +++ b/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'] diff --git a/invisible_cities/config/irene_daemon_example.conf b/invisible_cities/config/irene_daemon_example.conf index 0a511c8124..e4ce0812cc 100644 --- a/invisible_cities/config/irene_daemon_example.conf +++ b/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'] diff --git a/invisible_cities/core/configure_test.py b/invisible_cities/core/configure_test.py index ce98fc7197..61622e81cf 100644 --- a/invisible_cities/core/configure_test.py +++ b/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, From 0b8e5bbf1e9fdf562ff39bc01214664df5c18d3c Mon Sep 17 00:00:00 2001 From: Jacek Generowicz Date: Thu, 7 Sep 2017 20:45:46 +0200 Subject: [PATCH 09/12] Remove unused s1_string, s2_string and s1_rebin_stride --- invisible_cities/cities/irene_test.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/invisible_cities/cities/irene_test.py b/invisible_cities/cities/irene_test.py index f8e294586c..e4104222ee 100644 --- a/invisible_cities/cities/irene_test.py +++ b/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) From dc6e6bc3e7668b383a195e0a48f8a58e06e5aed1 Mon Sep 17 00:00:00 2001 From: Jacek Generowicz Date: Fri, 8 Sep 2017 19:35:45 +0200 Subject: [PATCH 10/12] Replace assert with raise UnknownParameter --- invisible_cities/cities/base_cities.py | 4 +++- invisible_cities/core/exceptions.py | 3 +++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/invisible_cities/cities/base_cities.py b/invisible_cities/cities/base_cities.py index 38e321851d..de479cab40 100644 --- a/invisible_cities/cities/base_cities.py +++ b/invisible_cities/cities/base_cities.py @@ -30,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 @@ -140,7 +141,8 @@ def __init__(self, **kwds): def detect_unknown_parameters(self, kwds): known = self.allowed_parameters() for name in kwds: - assert name in known, (name, self.__class__.__name__) + if name not in known: + raise UnknownParameter('{} does not expect {}.'.format(self.__class__.__name__, name)) @classmethod def allowed_parameters(cls): diff --git a/invisible_cities/core/exceptions.py b/invisible_cities/core/exceptions.py index 41804d3d68..6b20121feb 100644 --- a/invisible_cities/core/exceptions.py +++ b/invisible_cities/core/exceptions.py @@ -65,3 +65,6 @@ class NegativeThresholdNotAllowed(ICException): class InitializedEmptyPmapObject(ICException): pass + +class UnknownParameter(ICException): + pass From 2aed8b2c9c0bfda1e2ed0e2158405526a629e74a Mon Sep 17 00:00:00 2001 From: Jacek Generowicz Date: Fri, 8 Sep 2017 19:36:12 +0200 Subject: [PATCH 11/12] Add test for detection of unknown parameters --- invisible_cities/cities/city_test.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/invisible_cities/cities/city_test.py b/invisible_cities/cities/city_test.py index 9208d0fa6e..1d0afcdbe5 100644 --- a/invisible_cities/cities/city_test.py +++ b/invisible_cities/cities/city_test.py @@ -12,6 +12,7 @@ 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) From bc8ee2ae1962b9233e371a1982563ebaeed4205a Mon Sep 17 00:00:00 2001 From: Jacek Generowicz Date: Fri, 8 Sep 2017 19:37:49 +0200 Subject: [PATCH 12/12] Cosmetics --- invisible_cities/cities/city_test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/invisible_cities/cities/city_test.py b/invisible_cities/cities/city_test.py index 1d0afcdbe5..2cea126324 100644 --- a/invisible_cities/cities/city_test.py +++ b/invisible_cities/cities/city_test.py @@ -8,7 +8,7 @@ last changed: 01-12-2017 """ -from pytest import raises +from pytest import raises from .. core.exceptions import NoInputFiles from .. core.exceptions import NoOutputFile