From b9a3ca061aaa4ceb39fe16153bfcf23dcaef3b65 Mon Sep 17 00:00:00 2001 From: vuillaut Date: Fri, 16 Jun 2023 12:05:54 +0200 Subject: [PATCH 01/20] write modified images in dl1ab --- lstchain/scripts/lstchain_dl1ab.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lstchain/scripts/lstchain_dl1ab.py b/lstchain/scripts/lstchain_dl1ab.py index 79250b1e90..35551e9cbf 100644 --- a/lstchain/scripts/lstchain_dl1ab.py +++ b/lstchain/scripts/lstchain_dl1ab.py @@ -329,6 +329,9 @@ def main(): for p in parameters_to_update: params[ii][p] = u.Quantity(dl1_container[p]).value + if not args.no_image: + image_table[ii]['image'] = image + if image_mask_save: image_mask[ii] = signal_pixels From 5901b0d97d70937b3a482c3da8d5fe205d44f98e Mon Sep 17 00:00:00 2001 From: vuillaut Date: Fri, 16 Jun 2023 12:29:14 +0200 Subject: [PATCH 02/20] images will be saved --- lstchain/scripts/lstchain_dl1ab.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lstchain/scripts/lstchain_dl1ab.py b/lstchain/scripts/lstchain_dl1ab.py index 35551e9cbf..0f9584e8c9 100644 --- a/lstchain/scripts/lstchain_dl1ab.py +++ b/lstchain/scripts/lstchain_dl1ab.py @@ -118,10 +118,10 @@ def main(): transition_charge = imconfig["transition_charge"] extra_noise_in_bright_pixels = imconfig["extra_noise_in_bright_pixels"] smeared_light_fraction = imconfig["smeared_light_fraction"] - if (increase_nsb or increase_psf) and args.no_image is False: - log.info("NOTE: Using the image_modifier options means images will " - "not be saved.") - args.no_image = True + # if (increase_nsb or increase_psf) and args.no_image is False: + # log.info("NOTE: Using the image_modifier options means images will " + # "not be saved.") + # args.no_image = True if is_simulation: args.pedestal_cleaning = False From 7c5e7e5316a43179e93d1a0de9b9bf7ef2721607 Mon Sep 17 00:00:00 2001 From: vuillaut Date: Fri, 16 Jun 2023 12:36:45 +0200 Subject: [PATCH 03/20] images will be saved, really --- lstchain/scripts/lstchain_dl1ab.py | 1 + 1 file changed, 1 insertion(+) diff --git a/lstchain/scripts/lstchain_dl1ab.py b/lstchain/scripts/lstchain_dl1ab.py index 0f9584e8c9..3a409130dc 100644 --- a/lstchain/scripts/lstchain_dl1ab.py +++ b/lstchain/scripts/lstchain_dl1ab.py @@ -336,6 +336,7 @@ def main(): image_mask[ii] = signal_pixels outfile.root[dl1_params_lstcam_key][:] = params + outfile.root[dl1_images_lstcam_key][:] = image_table if image_mask_save: outfile.root[dl1_images_lstcam_key].modify_column(colname='image_mask', column=image_mask) From 6d5a4ff2954ff4e6cc439723b7ca1d69322bbda3 Mon Sep 17 00:00:00 2001 From: vuillaut Date: Fri, 16 Jun 2023 12:38:46 +0200 Subject: [PATCH 04/20] images will be saved, really --- lstchain/scripts/lstchain_dl1ab.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lstchain/scripts/lstchain_dl1ab.py b/lstchain/scripts/lstchain_dl1ab.py index 3a409130dc..14a1138ebc 100644 --- a/lstchain/scripts/lstchain_dl1ab.py +++ b/lstchain/scripts/lstchain_dl1ab.py @@ -336,7 +336,7 @@ def main(): image_mask[ii] = signal_pixels outfile.root[dl1_params_lstcam_key][:] = params - outfile.root[dl1_images_lstcam_key][:] = image_table + outfile.root[dl1_images_lstcam_key].modify_column(colname='image', column=image_table['image']) if image_mask_save: outfile.root[dl1_images_lstcam_key].modify_column(colname='image_mask', column=image_mask) From 7733fc3be614f3ceb933e1d0e1cb38bbf7a7c673 Mon Sep 17 00:00:00 2001 From: vuillaut Date: Fri, 16 Jun 2023 12:42:26 +0200 Subject: [PATCH 05/20] images will be saved, really --- lstchain/scripts/lstchain_dl1ab.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lstchain/scripts/lstchain_dl1ab.py b/lstchain/scripts/lstchain_dl1ab.py index 14a1138ebc..f92c285ce8 100644 --- a/lstchain/scripts/lstchain_dl1ab.py +++ b/lstchain/scripts/lstchain_dl1ab.py @@ -204,6 +204,7 @@ def main(): with tables.open_file(args.input_file, mode='r') as infile: image_table = infile.root[dl1_images_lstcam_key] + images = image_table.col('image') dl1_params_input = infile.root[dl1_params_lstcam_key].colnames disp_params = {'disp_dx', 'disp_dy', 'disp_norm', 'disp_angle', 'disp_sign'} if set(dl1_params_input).intersection(disp_params): @@ -330,13 +331,13 @@ def main(): params[ii][p] = u.Quantity(dl1_container[p]).value if not args.no_image: - image_table[ii]['image'] = image + images[ii] = image if image_mask_save: image_mask[ii] = signal_pixels outfile.root[dl1_params_lstcam_key][:] = params - outfile.root[dl1_images_lstcam_key].modify_column(colname='image', column=image_table['image']) + outfile.root[dl1_images_lstcam_key].modify_column(colname='image', column=images) if image_mask_save: outfile.root[dl1_images_lstcam_key].modify_column(colname='image_mask', column=image_mask) From b54d133670c401d78f520a45ad3ab3be5d07b0ad Mon Sep 17 00:00:00 2001 From: vuillaut Date: Fri, 16 Jun 2023 16:59:57 +0200 Subject: [PATCH 06/20] images will be saved, really --- lstchain/scripts/lstchain_dl1ab.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/lstchain/scripts/lstchain_dl1ab.py b/lstchain/scripts/lstchain_dl1ab.py index f92c285ce8..61233809b3 100644 --- a/lstchain/scripts/lstchain_dl1ab.py +++ b/lstchain/scripts/lstchain_dl1ab.py @@ -197,8 +197,8 @@ def main(): ] nodes_keys = get_dataset_keys(args.input_file) - if args.no_image: - nodes_keys.remove(dl1_images_lstcam_key) + # if args.no_image: + # nodes_keys.remove(dl1_images_lstcam_key) metadata = global_metadata() @@ -220,7 +220,8 @@ def main(): if increase_psf: set_numba_seed(infile.root.dl1.event.subarray.trigger.col('obs_id')[0]) - image_mask_save = not args.no_image and 'image_mask' in infile.root[dl1_images_lstcam_key].colnames + # image_mask_save = not args.no_image and 'image_mask' in infile.root[dl1_images_lstcam_key].colnames + image_mask_save = 'image_mask' in infile.root[dl1_images_lstcam_key].colnames with tables.open_file(args.output_file, mode='a', filters=HDF5_ZSTD_FILTERS) as outfile: copy_h5_nodes(infile, outfile, nodes=nodes_keys) @@ -330,8 +331,7 @@ def main(): for p in parameters_to_update: params[ii][p] = u.Quantity(dl1_container[p]).value - if not args.no_image: - images[ii] = image + images[ii] = image if image_mask_save: image_mask[ii] = signal_pixels From 805397231dc3dc17b9d6964c15e01edebb68b18b Mon Sep 17 00:00:00 2001 From: vuillaut Date: Mon, 24 Jul 2023 12:02:34 +0200 Subject: [PATCH 07/20] fix bugs with no_image --- lstchain/scripts/lstchain_dl1ab.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/lstchain/scripts/lstchain_dl1ab.py b/lstchain/scripts/lstchain_dl1ab.py index 0d4d3cd790..b9fd145036 100644 --- a/lstchain/scripts/lstchain_dl1ab.py +++ b/lstchain/scripts/lstchain_dl1ab.py @@ -82,7 +82,7 @@ parser.add_argument( '--no-image', action='store_true', - help='Pass this argument to avoid writing the images in the new DL1 files. Beware, if `increase_nsb` or `increase_psf` are True in the config, the images will not be written.', + help='Pass this argument to avoid writing the images in the new DL1 files.', ) parser.add_argument( @@ -148,7 +148,7 @@ def main(): if increase_nsb or increase_psf: log.info(f"image_modifier configuration: {imconfig}") - if ~args.no_image: + if not args.no_image: log.info("Modified images are saved in the output file.") if increase_nsb: @@ -243,7 +243,7 @@ def main(): with tables.open_file(args.input_file, mode='r') as infile: image_table = read_table(infile, dl1_images_lstcam_key) - images = image_table.col('image') + images = image_table['image'] params = read_table(infile, dl1_params_lstcam_key) dl1_params_input = params.colnames @@ -279,7 +279,6 @@ def main(): copy_h5_nodes(infile, outfile, nodes=nodes_keys) add_source_filenames(outfile, [args.input_file]) - # need container to use lstchain.io.add_global_metadata and lstchain.io.add_config_metadata for k, item in metadata.as_dict().items(): outfile.root[dl1_params_lstcam_key].attrs[k] = item @@ -422,7 +421,7 @@ def main(): if 'image_mask' in image_table.colnames: image_table['image_mask'][ii] = signal_pixels - if ~args.no_image: + if not args.no_image: write_table(image_table, outfile, dl1_images_lstcam_key, overwrite=True, filters=HDF5_ZSTD_FILTERS) write_table(params, outfile, dl1_params_lstcam_key, overwrite=True, filters=HDF5_ZSTD_FILTERS) From 03a968b8cfd0f89bbe4b1704487efe9cf6a513f5 Mon Sep 17 00:00:00 2001 From: vuillaut Date: Mon, 4 Sep 2023 14:07:00 +0200 Subject: [PATCH 08/20] adding the image config in the metadata and use it to check of the images already have been modified --- lstchain/scripts/lstchain_dl1ab.py | 29 +++++++++++++++++++ lstchain/scripts/tests/test_dl1ab.py | 23 +++++++++++++++ .../scripts/tests/test_lstchain_scripts.py | 18 ++++++++++++ 3 files changed, 70 insertions(+) create mode 100644 lstchain/scripts/tests/test_dl1ab.py diff --git a/lstchain/scripts/lstchain_dl1ab.py b/lstchain/scripts/lstchain_dl1ab.py index b9fd145036..3fe9e01f46 100644 --- a/lstchain/scripts/lstchain_dl1ab.py +++ b/lstchain/scripts/lstchain_dl1ab.py @@ -15,6 +15,7 @@ import argparse import logging from pathlib import Path +import json import astropy.units as u import numpy as np @@ -92,6 +93,27 @@ ) +def image_modifier_checker(config): + """ + Check if the image modifier has been used in the given configuration. + + Parameters + ---------- + config : `dict` + The configuration dictionary to check. + + Returns + ------- + `bool` + `True` if the image modifier has been used, `False` otherwise. + """ + imconfig = config.get('image_modifier', {}) + increase_nsb = imconfig.get("increase_nsb", False) + increase_psf = imconfig.get("increase_psf", False) + + return increase_nsb or increase_psf + + def main(): args = parser.parse_args() @@ -243,6 +265,13 @@ def main(): with tables.open_file(args.input_file, mode='r') as infile: image_table = read_table(infile, dl1_images_lstcam_key) + # if the image modifier has been used to produce these images, stop here + config_from_image_table = json.loads(image_table.meta['config']) + if image_modifier_checker(config_from_image_table) and image_modifier_checker(config): + log.error(f"The image modifier has already been used to produce the images in file {args.input_file}." + "Re-applying the image modifier is not a good practice, start again from unmodified images please.") + sys.exit(0) + images = image_table['image'] params = read_table(infile, dl1_params_lstcam_key) dl1_params_input = params.colnames diff --git a/lstchain/scripts/tests/test_dl1ab.py b/lstchain/scripts/tests/test_dl1ab.py new file mode 100644 index 0000000000..ae2632d470 --- /dev/null +++ b/lstchain/scripts/tests/test_dl1ab.py @@ -0,0 +1,23 @@ +from lstchain.scripts.lstchain_dl1ab import image_modifier_checker + + +def test_image_modifier_checker_no_modifiers(): + config = {} + assert not image_modifier_checker(config) + + +def test_image_modifier_checker_empty_modifier(): + config = {"image_modifier": {}} + assert not image_modifier_checker(config) + + +def test_image_modifier_checker_with_modifiers(): + config = {"image_modifier": {"increase_psf": True, "increase_nsb": False}} + assert image_modifier_checker(config) + config = {"image_modifier": {"increase_nsb": True, "increase_psf": False}} + assert image_modifier_checker(config) + config = {"image_modifier": {"increase_psf": True, "increase_nsb": True}} + assert image_modifier_checker(config) + config = {"image_modifier": {"increase_psf": False, "increase_nsb": False}} + assert not image_modifier_checker(config) + diff --git a/lstchain/scripts/tests/test_lstchain_scripts.py b/lstchain/scripts/tests/test_lstchain_scripts.py index 1d39b6029a..0a10497848 100644 --- a/lstchain/scripts/tests/test_lstchain_scripts.py +++ b/lstchain/scripts/tests/test_lstchain_scripts.py @@ -430,6 +430,24 @@ def test_dl1ab_no_images(simulated_dl1_file, tmp_path): assert (new_parameters['length'] != old_parameters['length']).any() +@pytest.mark.xfail(raises=ValueError, reason="One should not be able to re-modify already modified images") +def test_dl1ab_on_modified_images(simulated_dl1ab, tmp_path): + config_path = tmp_path / 'config.json' + output_file = tmp_path / 'dl1ab_on_modified_images.h5' + with config_path.open('w') as f: + config = get_standard_config() + config['image_modifier'] = {'increase_psf': True, 'increase_nsb': True} + json.dump(config, f) + + run_program( + 'lstchain_dl1ab', + '-f', simulated_dl1ab, + '-o', output_file, + '-c', config_path, + '--no-image', + ) + + @pytest.mark.private_data def test_observed_dl1ab(tmp_path, observed_dl1_files): output_dl1ab = tmp_path / "dl1ab.h5" From f041555d7dd16d3450d293f20963635c88c42dae Mon Sep 17 00:00:00 2001 From: vuillaut Date: Mon, 4 Sep 2023 15:36:32 +0200 Subject: [PATCH 09/20] dump config in dl1ab --- lstchain/scripts/lstchain_dl1ab.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/lstchain/scripts/lstchain_dl1ab.py b/lstchain/scripts/lstchain_dl1ab.py index 3fe9e01f46..2b21bc191c 100644 --- a/lstchain/scripts/lstchain_dl1ab.py +++ b/lstchain/scripts/lstchain_dl1ab.py @@ -32,7 +32,7 @@ from lstchain.calib.camera.pixel_threshold_estimation import get_threshold_from_dl1_file from lstchain.image.cleaning import apply_dynamic_cleaning from lstchain.image.modifier import random_psf_smearer, set_numba_seed, add_noise_in_pixels -from lstchain.io import get_dataset_keys, copy_h5_nodes, HDF5_ZSTD_FILTERS, add_source_filenames +from lstchain.io import get_dataset_keys, copy_h5_nodes, HDF5_ZSTD_FILTERS, add_source_filenames, add_config_metadata from lstchain.io.config import ( get_cleaning_parameters, @@ -268,9 +268,9 @@ def main(): # if the image modifier has been used to produce these images, stop here config_from_image_table = json.loads(image_table.meta['config']) if image_modifier_checker(config_from_image_table) and image_modifier_checker(config): - log.error(f"The image modifier has already been used to produce the images in file {args.input_file}." + log.critical(f"\nThe image modifier has already been used to produce the images in file {args.input_file}." "Re-applying the image modifier is not a good practice, start again from unmodified images please.") - sys.exit(0) + sys.exit(1) images = image_table['image'] params = read_table(infile, dl1_params_lstcam_key) @@ -450,9 +450,12 @@ def main(): if 'image_mask' in image_table.colnames: image_table['image_mask'][ii] = signal_pixels + + add_config_metadata(image_table, config) if not args.no_image: write_table(image_table, outfile, dl1_images_lstcam_key, overwrite=True, filters=HDF5_ZSTD_FILTERS) + add_config_metadata(params, config) write_table(params, outfile, dl1_params_lstcam_key, overwrite=True, filters=HDF5_ZSTD_FILTERS) # write a cat-B calibrations in DL1b From bdb3eab939973cb426d640506b7c9e68e340a425 Mon Sep 17 00:00:00 2001 From: vuillaut Date: Mon, 4 Sep 2023 15:38:27 +0200 Subject: [PATCH 10/20] rename function includes image modification --- lstchain/scripts/lstchain_dl1ab.py | 4 ++-- lstchain/scripts/tests/test_dl1ab.py | 14 +++++++------- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/lstchain/scripts/lstchain_dl1ab.py b/lstchain/scripts/lstchain_dl1ab.py index 2b21bc191c..02df3422f4 100644 --- a/lstchain/scripts/lstchain_dl1ab.py +++ b/lstchain/scripts/lstchain_dl1ab.py @@ -93,7 +93,7 @@ ) -def image_modifier_checker(config): +def includes_image_modification(config): """ Check if the image modifier has been used in the given configuration. @@ -267,7 +267,7 @@ def main(): image_table = read_table(infile, dl1_images_lstcam_key) # if the image modifier has been used to produce these images, stop here config_from_image_table = json.loads(image_table.meta['config']) - if image_modifier_checker(config_from_image_table) and image_modifier_checker(config): + if includes_image_modification(config_from_image_table) and includes_image_modification(config): log.critical(f"\nThe image modifier has already been used to produce the images in file {args.input_file}." "Re-applying the image modifier is not a good practice, start again from unmodified images please.") sys.exit(1) diff --git a/lstchain/scripts/tests/test_dl1ab.py b/lstchain/scripts/tests/test_dl1ab.py index ae2632d470..0c14e8c965 100644 --- a/lstchain/scripts/tests/test_dl1ab.py +++ b/lstchain/scripts/tests/test_dl1ab.py @@ -1,23 +1,23 @@ -from lstchain.scripts.lstchain_dl1ab import image_modifier_checker +from lstchain.scripts.lstchain_dl1ab import includes_image_modification def test_image_modifier_checker_no_modifiers(): config = {} - assert not image_modifier_checker(config) + assert not includes_image_modification(config) def test_image_modifier_checker_empty_modifier(): config = {"image_modifier": {}} - assert not image_modifier_checker(config) + assert not includes_image_modification(config) def test_image_modifier_checker_with_modifiers(): config = {"image_modifier": {"increase_psf": True, "increase_nsb": False}} - assert image_modifier_checker(config) + assert includes_image_modification(config) config = {"image_modifier": {"increase_nsb": True, "increase_psf": False}} - assert image_modifier_checker(config) + assert includes_image_modification(config) config = {"image_modifier": {"increase_psf": True, "increase_nsb": True}} - assert image_modifier_checker(config) + assert includes_image_modification(config) config = {"image_modifier": {"increase_psf": False, "increase_nsb": False}} - assert not image_modifier_checker(config) + assert not includes_image_modification(config) From d5cca4744db3cb2dc8f8a7281ed99d7b2158e5ad Mon Sep 17 00:00:00 2001 From: vuillaut Date: Mon, 4 Sep 2023 15:48:01 +0200 Subject: [PATCH 11/20] formatting --- lstchain/image/modifier.py | 3 --- lstchain/scripts/lstchain_dl1ab.py | 2 +- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/lstchain/image/modifier.py b/lstchain/image/modifier.py index a7a6ae5d11..29a28e8348 100644 --- a/lstchain/image/modifier.py +++ b/lstchain/image/modifier.py @@ -299,9 +299,6 @@ def calculate_noise_parameters(simtel_filename, data_dl1_filename, pedestal_calibrator.image_extractors[ped_config['charge_product']].apply_integration_correction = True shower_calibrator.image_extractors[shower_extractor_type].apply_integration_correction = True - - - # Pulse integration window width of the (biased) extractor for showers: shower_extractor_window_width = config[config['image_extractor']]['window_width'] diff --git a/lstchain/scripts/lstchain_dl1ab.py b/lstchain/scripts/lstchain_dl1ab.py index 02df3422f4..f6afe572ea 100644 --- a/lstchain/scripts/lstchain_dl1ab.py +++ b/lstchain/scripts/lstchain_dl1ab.py @@ -268,7 +268,7 @@ def main(): # if the image modifier has been used to produce these images, stop here config_from_image_table = json.loads(image_table.meta['config']) if includes_image_modification(config_from_image_table) and includes_image_modification(config): - log.critical(f"\nThe image modifier has already been used to produce the images in file {args.input_file}." + log.critical(f"\nThe image modifier has already been used to produce the images in file {args.input_file}.\n" "Re-applying the image modifier is not a good practice, start again from unmodified images please.") sys.exit(1) From 867872a80540a094c87b3bc22a2385b0873edcea Mon Sep 17 00:00:00 2001 From: vuillaut Date: Mon, 4 Sep 2023 15:59:19 +0200 Subject: [PATCH 12/20] fix test --- lstchain/scripts/tests/test_lstchain_scripts.py | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/lstchain/scripts/tests/test_lstchain_scripts.py b/lstchain/scripts/tests/test_lstchain_scripts.py index 0a10497848..35b0d4442c 100644 --- a/lstchain/scripts/tests/test_lstchain_scripts.py +++ b/lstchain/scripts/tests/test_lstchain_scripts.py @@ -432,8 +432,9 @@ def test_dl1ab_no_images(simulated_dl1_file, tmp_path): @pytest.mark.xfail(raises=ValueError, reason="One should not be able to re-modify already modified images") def test_dl1ab_on_modified_images(simulated_dl1ab, tmp_path): - config_path = tmp_path / 'config.json' + config_path = tmp_path / 'config_image_modifier.json' output_file = tmp_path / 'dl1ab_on_modified_images.h5' + reprocess_output_file = tmp_path / 'dl1ab_on_modified_images_reprocess.h5' with config_path.open('w') as f: config = get_standard_config() config['image_modifier'] = {'increase_psf': True, 'increase_nsb': True} @@ -444,7 +445,14 @@ def test_dl1ab_on_modified_images(simulated_dl1ab, tmp_path): '-f', simulated_dl1ab, '-o', output_file, '-c', config_path, - '--no-image', + ) + + # second process should fail as we are trying to re-modify already modified images + run_program( + 'lstchain_dl1ab', + '-f', output_file, + '-o', reprocess_output_file, + '-c', config_path, ) From 54d20c4882feb5468efaba3534352b1b9cc5c5d3 Mon Sep 17 00:00:00 2001 From: vuillaut Date: Tue, 5 Sep 2023 14:27:24 +0200 Subject: [PATCH 13/20] fix config serialization and add unit test --- lstchain/io/io.py | 63 ++++++++++++++++++++++------ lstchain/io/tests/test_io.py | 80 ++++++++++++++++++++++++++++-------- 2 files changed, 113 insertions(+), 30 deletions(-) diff --git a/lstchain/io/io.py b/lstchain/io/io.py index eb6c6abe99..0d990fe822 100644 --- a/lstchain/io/io.py +++ b/lstchain/io/io.py @@ -10,6 +10,9 @@ import tables from tables import open_file from tqdm import tqdm +import json +from traitlets.config.loader import DeferredConfigString, LazyConfigValue +from pathlib import PosixPath import astropy.units as u from astropy.table import Table, vstack, QTable @@ -31,6 +34,8 @@ ThrownEventsHistogram, ) + + log = logging.getLogger(__name__) __all__ = [ @@ -657,6 +662,50 @@ def add_global_metadata(container, metadata): container.meta[k] = item + + +def serialize_config(obj): + """ + Serialize an object to a JSON-serializable format. + + Parameters + ---------- + obj : object + The object to serialize. + + Returns + ------- + object + The serialized object. + + Raises + ------ + TypeError + If the object is not serializable. + + Notes + ----- + This function serializes an object to a JSON-serializable format. It supports the following types: + - LazyConfigValue + - DeferredConfigString + - PosixPath + - numpy.ndarray + + If the object is not one of the above types, a TypeError is raised. + + """ + if isinstance(obj, LazyConfigValue): + return obj.to_dict() + elif isinstance(obj, DeferredConfigString): + return str(obj) + elif isinstance(obj, PosixPath): + return obj.as_posix() + elif isinstance(obj, np.ndarray): + return obj.tolist() + else: + raise TypeError(f"Type {type(obj).__name__} not serializable") + + def add_config_metadata(container, configuration): """ Add configuration parameters to a container in container.meta.config @@ -666,18 +715,7 @@ def add_config_metadata(container, configuration): container: `ctapipe.containers.Container` configuration: config dict """ - linted_config = str(configuration) - linted_config = linted_config.replace("", "None") - linted_config = re.sub(r"", "\\1", linted_config) - linted_config = re.sub(r"DeferredConfigString\((.*?)\)", "\\1", linted_config) - linted_config = re.sub(r"PosixPath\((.*?)\)", "\\1", linted_config) - linted_config = linted_config.replace("\'", "\"") - linted_config = linted_config.replace("None", "\"None\"") - linted_config = linted_config.replace("inf", "\"inf\"") - linted_config = linted_config.replace("True", "true") - linted_config = linted_config.replace("False", "false") - - container.meta["config"] = linted_config + container.meta["config"] = json.dumps(configuration, default=serialize_config) def write_subarray_tables(writer, event, metadata=None): @@ -1261,3 +1299,4 @@ def get_mc_fov_offset(filename): mean_offset = min_viewcone + 0.5 * (max_viewcone - min_viewcone) return mean_offset + diff --git a/lstchain/io/tests/test_io.py b/lstchain/io/tests/test_io.py index b177ac6b49..9c9357132a 100644 --- a/lstchain/io/tests/test_io.py +++ b/lstchain/io/tests/test_io.py @@ -1,11 +1,15 @@ import tempfile - +import json +import math import numpy as np import pandas as pd import pytest import tables from astropy.table import Table, QTable from ctapipe.instrument import SubarrayDescription +from lstchain.io import add_config_metadata +from pathlib import PosixPath +from traitlets.config.loader import DeferredConfigString, LazyConfigValue @pytest.fixture @@ -149,6 +153,7 @@ def test_trigger_type_in_dl1_params(simulated_dl1_file): def test_extract_simulation_nsb(mc_gamma_testfile): from lstchain.io.io import extract_simulation_nsb + nsb = extract_simulation_nsb(mc_gamma_testfile) assert np.isclose(nsb[0], 0.246, rtol=0.1) assert np.isclose(nsb[1], 0.217, rtol=0.1) @@ -156,32 +161,71 @@ def test_extract_simulation_nsb(mc_gamma_testfile): def test_remove_duplicated_events(): from lstchain.io.io import remove_duplicated_events - - d = {'event_id': [1, 2, 3, - 1, 2, 4, - 1, 2, 3], - 'gh_score': [0.1, 0.5, 0.7, - 0.5, 0.8, 0.1, - 0.9, 0.1, 0.5], - 'alpha': range(9) - } + + d = { + "event_id": [1, 2, 3, 1, 2, 4, 1, 2, 3], + "gh_score": [0.1, 0.5, 0.7, 0.5, 0.8, 0.1, 0.9, 0.1, 0.5], + "alpha": range(9), + } df = pd.DataFrame(data=d) data1 = QTable.from_pandas(df) remove_duplicated_events(data1) - d2 = {'event_id': [3, 2, 4, 1], - 'gh_score': [0.7, 0.8, 0.1, 0.9], - 'alpha': [2, 4, 5, 6] - } + d2 = { + "event_id": [3, 2, 4, 1], + "gh_score": [0.7, 0.8, 0.1, 0.9], + "alpha": [2, 4, 5, 6], + } df2 = pd.DataFrame(data=d2) - data2= QTable.from_pandas(df2) + data2 = QTable.from_pandas(df2) - assert np.all(data1==data2) + assert np.all(data1 == data2) def test_check_mc_type(simulated_dl1_file): from lstchain.io.io import check_mc_type mc_type = check_mc_type(simulated_dl1_file) - assert mc_type == 'diffuse' - + assert mc_type == "diffuse" + + +def test_add_config_metadata(): + class Container: + meta = {} + + lazy_value = LazyConfigValue() + lazy_value.update({"key": "new_value"}) + + config = { + "param1": 1, + "param2": "value2", + "param3": [1, 2, 3], + "param4": {"a": 1, "b": 2}, + "param5": None, + "param6": lazy_value, + "param7": DeferredConfigString("some_string"), + "param8": PosixPath("/path/to/file"), + "param9": np.inf, + "param10": True, + "param11": False, + "param12": np.array([1, 2, 3]), + } + + expected_config = { + "param1": 1, + "param2": "value2", + "param3": [1, 2, 3], + "param4": {"a": 1, "b": 2}, + "param5": None, + "param6": {"update": {"key": "new_value"}}, + "param7": "some_string", + "param8": "/path/to/file", + "param9": math.inf, + "param10": True, + "param11": False, + "param12": [1, 2, 3], + } + + container = Container() + add_config_metadata(container, config) + assert json.loads(container.meta["config"]) == expected_config From d8b354ac41a15617d962bc0b33f808bab9f1585a Mon Sep 17 00:00:00 2001 From: vuillaut Date: Tue, 5 Sep 2023 14:29:43 +0200 Subject: [PATCH 14/20] add standard config in test --- lstchain/io/tests/test_io.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/lstchain/io/tests/test_io.py b/lstchain/io/tests/test_io.py index 9c9357132a..afe049be7b 100644 --- a/lstchain/io/tests/test_io.py +++ b/lstchain/io/tests/test_io.py @@ -229,3 +229,10 @@ class Container: container = Container() add_config_metadata(container, config) assert json.loads(container.meta["config"]) == expected_config + + # test also with standard config in case of future changes + from lstchain.io.config import get_standard_config + config = get_standard_config() + container = Container() + add_config_metadata(container, config) + assert json.loads(container.meta["config"]) == config From f478f0a08940305c02278e91b089e5404561979d Mon Sep 17 00:00:00 2001 From: vuillaut Date: Tue, 5 Sep 2023 14:48:54 +0200 Subject: [PATCH 15/20] correct way of testing the failure of the function --- lstchain/scripts/tests/test_lstchain_scripts.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/lstchain/scripts/tests/test_lstchain_scripts.py b/lstchain/scripts/tests/test_lstchain_scripts.py index 35b0d4442c..80a52a9361 100644 --- a/lstchain/scripts/tests/test_lstchain_scripts.py +++ b/lstchain/scripts/tests/test_lstchain_scripts.py @@ -430,7 +430,6 @@ def test_dl1ab_no_images(simulated_dl1_file, tmp_path): assert (new_parameters['length'] != old_parameters['length']).any() -@pytest.mark.xfail(raises=ValueError, reason="One should not be able to re-modify already modified images") def test_dl1ab_on_modified_images(simulated_dl1ab, tmp_path): config_path = tmp_path / 'config_image_modifier.json' output_file = tmp_path / 'dl1ab_on_modified_images.h5' @@ -448,12 +447,13 @@ def test_dl1ab_on_modified_images(simulated_dl1ab, tmp_path): ) # second process should fail as we are trying to re-modify already modified images - run_program( - 'lstchain_dl1ab', - '-f', output_file, - '-o', reprocess_output_file, - '-c', config_path, - ) + with pytest.raises(ValueError): + run_program( + 'lstchain_dl1ab', + '-f', output_file, + '-o', reprocess_output_file, + '-c', config_path, + ) @pytest.mark.private_data From 6b949fc994e592bdb3ea426b0ad8cc862874df6e Mon Sep 17 00:00:00 2001 From: vuillaut Date: Wed, 6 Sep 2023 11:27:49 +0200 Subject: [PATCH 16/20] fix test dl1ab on modified images --- lstchain/scripts/tests/test_lstchain_scripts.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/lstchain/scripts/tests/test_lstchain_scripts.py b/lstchain/scripts/tests/test_lstchain_scripts.py index 80a52a9361..caed37a137 100644 --- a/lstchain/scripts/tests/test_lstchain_scripts.py +++ b/lstchain/scripts/tests/test_lstchain_scripts.py @@ -6,6 +6,7 @@ import pkg_resources import pytest import tables +from pathlib import Path from astropy import units as u from astropy.time import Time from ctapipe.instrument import SubarrayDescription @@ -434,10 +435,7 @@ def test_dl1ab_on_modified_images(simulated_dl1ab, tmp_path): config_path = tmp_path / 'config_image_modifier.json' output_file = tmp_path / 'dl1ab_on_modified_images.h5' reprocess_output_file = tmp_path / 'dl1ab_on_modified_images_reprocess.h5' - with config_path.open('w') as f: - config = get_standard_config() - config['image_modifier'] = {'increase_psf': True, 'increase_nsb': True} - json.dump(config, f) + config_path = Path(__file__).parent.joinpath("../../data/lstchain_dl1ab_tune_MC_to_Crab_config.json") run_program( 'lstchain_dl1ab', From 9b4888f0f4946f00b799f05c4211de84b18639a0 Mon Sep 17 00:00:00 2001 From: vuillaut Date: Wed, 6 Sep 2023 12:50:20 +0200 Subject: [PATCH 17/20] use importlib resources to find the tuned config file --- lstchain/scripts/tests/test_lstchain_scripts.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lstchain/scripts/tests/test_lstchain_scripts.py b/lstchain/scripts/tests/test_lstchain_scripts.py index caed37a137..d2841b357b 100644 --- a/lstchain/scripts/tests/test_lstchain_scripts.py +++ b/lstchain/scripts/tests/test_lstchain_scripts.py @@ -6,6 +6,7 @@ import pkg_resources import pytest import tables +from importlib.resources import files from pathlib import Path from astropy import units as u from astropy.time import Time @@ -435,7 +436,7 @@ def test_dl1ab_on_modified_images(simulated_dl1ab, tmp_path): config_path = tmp_path / 'config_image_modifier.json' output_file = tmp_path / 'dl1ab_on_modified_images.h5' reprocess_output_file = tmp_path / 'dl1ab_on_modified_images_reprocess.h5' - config_path = Path(__file__).parent.joinpath("../../data/lstchain_dl1ab_tune_MC_to_Crab_config.json") + config_path = files("lstchain.data").joinpath("lstchain_dl1ab_tune_MC_to_Crab_config.json") run_program( 'lstchain_dl1ab', From d9e75a774790ba06d607418b7f4b5bcb577cb735 Mon Sep 17 00:00:00 2001 From: vuillaut Date: Wed, 6 Sep 2023 13:52:02 +0200 Subject: [PATCH 18/20] remove unused import pathlib --- lstchain/scripts/tests/test_lstchain_scripts.py | 1 - 1 file changed, 1 deletion(-) diff --git a/lstchain/scripts/tests/test_lstchain_scripts.py b/lstchain/scripts/tests/test_lstchain_scripts.py index caaa34d5bd..c425abde7b 100644 --- a/lstchain/scripts/tests/test_lstchain_scripts.py +++ b/lstchain/scripts/tests/test_lstchain_scripts.py @@ -7,7 +7,6 @@ import pytest import tables from importlib.resources import files -from pathlib import Path from astropy import units as u from astropy.time import Time from ctapipe.instrument import SubarrayDescription From 613a6d4a58505a9aa2b6925d041013a09c594255 Mon Sep 17 00:00:00 2001 From: vuillaut Date: Thu, 7 Sep 2023 11:18:11 +0200 Subject: [PATCH 19/20] move includes_image_modification to io/config --- lstchain/io/config.py | 21 +++++++++++++++++++++ lstchain/io/tests/test_config.py | 21 ++++++++++++++++++++- lstchain/scripts/lstchain_dl1ab.py | 22 +--------------------- lstchain/scripts/tests/test_dl1ab.py | 23 ----------------------- 4 files changed, 42 insertions(+), 45 deletions(-) delete mode 100644 lstchain/scripts/tests/test_dl1ab.py diff --git a/lstchain/io/config.py b/lstchain/io/config.py index f7ee52e020..3ed12cec48 100644 --- a/lstchain/io/config.py +++ b/lstchain/io/config.py @@ -126,3 +126,24 @@ def dump_config(config, filename, overwrite=False): raise FileExistsError(f"File {filename} exists, use overwrite=True") with open(filename, 'w') as file: json.dump(config, file, indent=2) + + +def includes_image_modification(config): + """ + Check if the image modifier has been used in the given configuration. + + Parameters + ---------- + config : `dict` + The configuration dictionary to check. + + Returns + ------- + `bool` + `True` if the image modifier has been used, `False` otherwise. + """ + imconfig = config.get('image_modifier', {}) + increase_nsb = imconfig.get("increase_nsb", False) + increase_psf = imconfig.get("increase_psf", False) + + return increase_nsb or increase_psf diff --git a/lstchain/io/tests/test_config.py b/lstchain/io/tests/test_config.py index c6cc4c73a4..d8f8963312 100644 --- a/lstchain/io/tests/test_config.py +++ b/lstchain/io/tests/test_config.py @@ -1,6 +1,7 @@ +import tempfile from lstchain.io import config from lstchain.io.config import get_cleaning_parameters -import tempfile +from lstchain.scripts.lstchain_dl1ab import includes_image_modification def test_get_standard_config(): @@ -52,3 +53,21 @@ def test_dump_config(): config.dump_config(cfg, file.name, overwrite=True) read_cfg = config.read_configuration_file(file.name) assert read_cfg['myconf'] == 1 + + +def test_includes_image_modification_no_modif(): + cfg = {} + assert not includes_image_modification(cfg) + cfg = {"image_modifier": {}} + assert not includes_image_modification(cfg) + + +def test_includes_image_modification_with_modif(): + cfg = {"image_modifier": {"increase_psf": True, "increase_nsb": False}} + assert includes_image_modification(cfg) + cfg = {"image_modifier": {"increase_nsb": True, "increase_psf": False}} + assert includes_image_modification(cfg) + cfg = {"image_modifier": {"increase_psf": True, "increase_nsb": True}} + assert includes_image_modification(cfg) + cfg = {"image_modifier": {"increase_psf": False, "increase_nsb": False}} + assert not includes_image_modification(cfg) diff --git a/lstchain/scripts/lstchain_dl1ab.py b/lstchain/scripts/lstchain_dl1ab.py index f6afe572ea..549e787c29 100644 --- a/lstchain/scripts/lstchain_dl1ab.py +++ b/lstchain/scripts/lstchain_dl1ab.py @@ -39,6 +39,7 @@ get_standard_config, read_configuration_file, replace_config, + includes_image_modification, ) from lstchain.io.io import ( dl1_images_lstcam_key, @@ -93,27 +94,6 @@ ) -def includes_image_modification(config): - """ - Check if the image modifier has been used in the given configuration. - - Parameters - ---------- - config : `dict` - The configuration dictionary to check. - - Returns - ------- - `bool` - `True` if the image modifier has been used, `False` otherwise. - """ - imconfig = config.get('image_modifier', {}) - increase_nsb = imconfig.get("increase_nsb", False) - increase_psf = imconfig.get("increase_psf", False) - - return increase_nsb or increase_psf - - def main(): args = parser.parse_args() diff --git a/lstchain/scripts/tests/test_dl1ab.py b/lstchain/scripts/tests/test_dl1ab.py deleted file mode 100644 index 0c14e8c965..0000000000 --- a/lstchain/scripts/tests/test_dl1ab.py +++ /dev/null @@ -1,23 +0,0 @@ -from lstchain.scripts.lstchain_dl1ab import includes_image_modification - - -def test_image_modifier_checker_no_modifiers(): - config = {} - assert not includes_image_modification(config) - - -def test_image_modifier_checker_empty_modifier(): - config = {"image_modifier": {}} - assert not includes_image_modification(config) - - -def test_image_modifier_checker_with_modifiers(): - config = {"image_modifier": {"increase_psf": True, "increase_nsb": False}} - assert includes_image_modification(config) - config = {"image_modifier": {"increase_nsb": True, "increase_psf": False}} - assert includes_image_modification(config) - config = {"image_modifier": {"increase_psf": True, "increase_nsb": True}} - assert includes_image_modification(config) - config = {"image_modifier": {"increase_psf": False, "increase_nsb": False}} - assert not includes_image_modification(config) - From f7403ab22b9dca0dcff2559c6742669c2d7d40d0 Mon Sep 17 00:00:00 2001 From: vuillaut Date: Thu, 14 Sep 2023 15:10:05 +0200 Subject: [PATCH 20/20] import includes_image_modfication from config --- lstchain/io/tests/test_config.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lstchain/io/tests/test_config.py b/lstchain/io/tests/test_config.py index d8f8963312..84028164b8 100644 --- a/lstchain/io/tests/test_config.py +++ b/lstchain/io/tests/test_config.py @@ -1,7 +1,6 @@ import tempfile from lstchain.io import config -from lstchain.io.config import get_cleaning_parameters -from lstchain.scripts.lstchain_dl1ab import includes_image_modification +from lstchain.io.config import get_cleaning_parameters, includes_image_modification def test_get_standard_config():