cta-observatory / cta-lstchain

LST prototype testbench chain
https://cta-observatory.github.io/cta-lstchain/
BSD 3-Clause "New" or "Revised" License
24 stars 77 forks source link

Improve non-finite parameters rejection when training models #1209

Closed gabemery closed 6 months ago

gabemery commented 8 months ago

Update of the build_models method to ensure more effectively that non-finite values in features are removed.

Main change : additional call to utils.filter_events before the classifier training to account for the new 'signed_time_gradient'. Other new parameters that may be affected include the 'signed skewness', reconstructed energy and reconstructed disp.

I also moved the 'sin_az_tel' computation before the first call to utils.filter_events. I have never seen a case when 'az_tel' is not finite but this would handle it.

Dedicated tests could be added in a LHfit related PR since this is where a failure case was observed.

closes #1206

SeiyaNozaki commented 7 months ago

Hi @gabemery The code looks fine. Maybe it is possible to remove particle_classification_features for the first event filtering since it will be applied later?

gabemery commented 7 months ago

@SeiyaNozaki I would keep it there. The idea being that the best would be to filter all problematic events from the start, if possible (i.e. if events have bad features for particle classification it is better to also remove them from the energy and direction training). Then we filter again for the features which are produced later just in case.

SeiyaNozaki commented 7 months ago

Okay, thanks for the explanation! Then, it looks already fine!