Bug in image extractor handling

[rlopezcoto]

Hi all,
I open this issue for discussion, I will open a separate PR for the fixing. @FrancaCassol, @moralejo and I have been investigating the light collection efficiency derived using muon analysis. It seems that we have found a bug on how the selected image extractor is passed to the analysis chain. At the moment, the image extractor is defined in:

cta-lstchain/lstchain/data/lstchain_standard_config.json

Line 100 in 921c5cc

"image_extractor": "LocalPeakWindowSum",

the script of data analysis calls:

cta-lstchain/lstchain/scripts/lstchain_data_r0_to_dl1.py

Line 67 in 921c5cc

dl0_to_dl1.r0_to_dl1(args.infile,

that calls the LSTCameraCalibrator

cta-lstchain/lstchain/reco/dl0_to_dl1.py

Line 192 in 921c5cc

r1_dl1_calibrator = LSTCameraCalibrator(calibration_path=calibration_path,

here we pass the variable image_extractor

cta-lstchain/lstchain/reco/dl0_to_dl1.py

Line 194 in 921c5cc

image_extractor=config['image_extractor'],

but the one LSTCameraCalibrator expects is extractor_product:

cta-lstchain/lstchain/calib/camera/calibrator.py

Line 20 in 921c5cc

extractor_product = Unicode(

The fix is just to change image_extractor -> extractor_product in

cta-lstchain/lstchain/reco/dl0_to_dl1.py

Line 194 in 921c5cc

image_extractor=config['image_extractor'],

I'm currently running tests, I'll make the PR afterwards. Any opinions?

Consequences:

• All the real data that has been processed up to now are suffering from this bug, so any signal extractor that you have been using was not passed to the analysis. I checked this using the data processed by @morcuended using different signal extractors and results are identical.
• The data was processed using the default image extractor defined in extractor_product: NeighborPeakWindowSum, with its default initialization ( window_shift: 3, window_width: 7). This is specially bad because of time misalignment in neighbor modules (@FrancaCassol can comment further on that).

[kosack]

Do you use your own config file system, or are you using the ctapipe one? If the ctapipe one (i.e your scipt inherits from ctapipe.core.Tool), you are construction your CameraCalibrator incorrectly: you can just do something like this:

self.image_extractor = self.add_component(
            ImageExtractor.from_name(
                self.image_extractor_type,  # where this is some config variable with a string name
                parent=self,
                subarray=self.event_source.subarray,
            )
  )
 self.calibrate = self.add_component(
            CameraCalibrator(parent=self, image_extractor=self.image_extractor)
 )

It will be configured automatically (setting the "parent" attribute will pass the config from the parent Tool - this is part of traitlets.config). note that the add_component wrapper function is only used in the development version of ctapipe, but isn't there in older version (it allows the configuration to be tracked and validated against what the user requested, so would avoid this bug you have).

[FrancaCassol]

Hi @kosack,
yes, the script is not based on the ctapipe.core.Tool and it uses its own config file system. We should indeed move as soon as possible to a ctapipe.core.Tool and with the occasion to update the code to the master of ctapipe

[rlopezcoto]

Thanks for the comment @kosack. @FrancaCassol is right, but we would probably need a new stable release of ctapipe for it. I've been also discussing with @maxnoe, are you guys planning to do a release soon?

[rlopezcoto]

Solved in #297