Zero suppression

[garciagenrique]

No description provided.

[garciagenrique]

What it should have appeared on the description :
Implementation of a volume reduction module on lstchain:

• tailcuts_clean(8/4) + 3 dilates
• and the - default - none volume reduction method.

[vuillaut]

Thanks for the implementation.
There is a couple of things to change I think. Others comments are matters of taste.

[garciagenrique]

Thanks for the implementation.
There is a couple of things to change I think. Others comments are matters of taste.

Thanks for the comments @vuillaut ! Starting to implement/change them now.

[vuillaut]

The code looks good to me.
However, in the meantime a PR was proposed in ctapipe to handle volume reduction with this method proposed by Lenka Tomankova et al so I'd propose to wait for this PR to be accepted in ctapipe and use the method here. What do you think?

[garciagenrique]

The code looks good to me.
However, in the meantime a PR was proposed in ctapipe to handle volume reduction with this method proposed by Lenka Tomankova et al so I'd propose to wait for this PR to be accepted in ctapipe and use the method here. What do you think?

No problem. Let's wait until the PR is accepted.

[rlopezcoto]

Hi, do we want to keep this opened as a PR or maybe just move to open an Issue? If you want to construct something different as that from ctapipe, then it should be ok, otherwise we may think on opening an issue to use the ctapipe module whenever it is merged.
Keep in mind that the ctapipe module will not be available for lstchain until a new ctapipe stable version is released.

[garciagenrique]

Hi @rlopezcoto,
I would say to use the ctapipe module in the moment it is merged. However, it also depends on today's need. If there is (or very soon will be) the need of starting applying the zero suppression on data we can merge this simpler version.

[vuillaut]

Hi @rlopezcoto,
I would say to use the ctapipe module in the moment it is merged. However, it also depends on today's need. If there is (or very soon will be) the need of starting applying the zero suppression on data we can merge this simpler version.

I agree. I thought the module in ctapipe would be implemented earlier.
We can implement a first version here I think and update with ctapipe in the future.

[rlopezcoto]

Shall we then move on with the validation of this PR and leave an open Issue to move to use the ctapipe module whenever it is ready?

[garciagenrique]

Sure ! Let me merge the master branch and solve the potential conflicts that will appear (the PR dates from few months...)

[garciagenrique]

Ok, this PR is ready for re-revision !

In the moment it is approved, I will open an issue to note that the method is a temporal version and that we should use the ctapipe version when available.

Please note that for the moment the computed mask that is applied for the zero suppression is compute at the dl1 level (from the integrated image) and applied to dl0 and dl1 levels. So the 'first' dl0 data computed is overwritten.

[vuillaut]

Could you also add a unit test please?

from ctapipe.io import event_source
from ctapipe.utils import get_dataset_path
from ctapipe.calib import CameraCalibrator
source = event_source(get_dataset_path('gamma_test.simtel.gz'))
from lstchain.io import get_standard_config
ev = next(iter(source))
tel_id = list(ev.r0.tels_with_data)[0]
cal = CameraCalibrator()
cal(ev)
config = get_standard_config()
config['volume_reducer'] = ...
apply_volume_reduction(...) 

you get the idea.
At least to check that the algorithms work.