cta-observatory / pyeventio

Python read-only implementation for the EventIO data format used by the CORSIKA 7 IACT extension and sim_telarray
MIT License
10 stars 12 forks source link

Improve handling of calibration events #246

Closed maxnoe closed 2 years ago

maxnoe commented 2 years ago

Instead of giving no event it, we assign the array event id from within the array event object in the calibration container and just add a minus sign so we don't produce duplicated event ids with the later shower events.

This also now works for the case of files run through extract_calib_events, which look like normal array events without associated mc shower / mc event objects. These raised errors before.

codecov[bot] commented 2 years ago

Codecov Report

Merging #246 (d5d6749) into master (cb83102) will increase coverage by 0.10%. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #246      +/-   ##
==========================================
+ Coverage   86.35%   86.46%   +0.10%     
==========================================
  Files          24       24              
  Lines        2140     2157      +17     
==========================================
+ Hits         1848     1865      +17     
  Misses        292      292              
Impacted Files Coverage Δ
eventio/simtel/simtelfile.py 97.86% <100.00%> (+0.16%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update cb83102...d5d6749. Read the comment docs.

maxnoe commented 2 years ago

A more general question I have is whether you currently expect to have calibration and shower events in the same file.

Both ctapipe and pyeventio support this and we have test files I produced with for example LASER_EVENTS=2 DARK_PEDESTALS=2 ...

However, until now the event ids were all 0 for calibration events, which for example made using the ctapipe.io.TableLoader impossible, since it joins information based on event_id.

This PR is about both improving that and supporting the case of calibration events extracted from normal runs that contain both calibration and physics events.

orelgueta commented 2 years ago

OK, thanks for the explanation! I am merging.