Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement the dead time calculation #52

Merged
merged 13 commits into from
Jun 4, 2022
Merged

Implement the dead time calculation #52

merged 13 commits into from
Jun 4, 2022

Conversation

YoshikiOhtani
Copy link
Collaborator

@YoshikiOhtani YoshikiOhtani commented Apr 27, 2022

This pull request includes the dead time calculation in the combined analysis pipeline. It needs the newest version of ctapipe_io_magic to get the time differences of MAGIC events. It closes #33.

@YoshikiOhtani YoshikiOhtani marked this pull request as ready for review May 16, 2022 14:02
@YoshikiOhtani
Copy link
Collaborator Author

So I implemented a function to calculate the dead time correction factor. The final correction factor is the multiplicity of those of LST-1 and MAGIC as suggested by @jsitarek in the issue #33. The factor for LST-1 is around 4%, and the one for MAGIC is around 1%, so in total it will be 5%. It would be great if @jsitarek could review it. Thank you in advance!

Copy link
Collaborator

@jsitarek jsitarek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry for a late feedback. The deadtime calculation looks fine, but I put two sugestions to improve it further (to save us a headache in the future)
there seems to be also some other changes related to hot pixel finding and pedestals in the same PR, I guess those were not intended.

condition = '(time_diff > 0) & (time_diff < 0.1)'

time_diffs_lst = df_events.query(f'(tel_id == 1) & {condition}')['time_diff'].to_numpy()
time_diffs_magic = df_events.query(f'(tel_id == 2) & {condition}')['time_diff'].to_numpy()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

M1 and M2 dead time is strongly correlated so it is fine to take only one as you do, but maybe we can make it more "democratic", such that it would work also in the case we have observations when M1 was not working at all.
How about counting the M1 and M2 events and using for this one the one with more events?

delta_time = time_end.value - time_start.value

deadc = effective_time / elapsed_time
elapsed_time = time_end.value - time_start.value
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure over how big chunks of data this function can be run, but a safer way to compute the elapsed time is to take all the time difference, exclude big gaps (e.g. > 0.1 s), and sum them

magicctapipe/scripts/lst1_magic/magic_calib_to_dl1.py Outdated Show resolved Hide resolved
magicctapipe/scripts/lst1_magic/magic_calib_to_dl1.py Outdated Show resolved Hide resolved
magicctapipe/scripts/lst1_magic/magic_calib_to_dl1.py Outdated Show resolved Hide resolved
@YoshikiOhtani
Copy link
Collaborator Author

Hi @jsitarek, thank you for your checks and comments! I removed the unrelated changes and implemented your suggestions. Regarding the elapsed time, I saw that the cut of the time diffs (< 0.1 sec) is too small for the LST-1 + MAGIC DL3 events. Please find the attached plot of the time difference distribution of DL3 events surviving gammaness cuts (dynamic, g/h efficiency = 0.9). It seems to me that the number of events reconstructed by the three telescopes is actually small, and in addition to this the superior gamma/hadron separation removes background events much better, and so gamma-ray events may already be dominant in the DL3 event list, which makes the time diffs bigger (than LST-1 mono and MAGIC-stereo). So at the moment I don't apply any time diff cuts when calculating the elapsed time, but please let me know what you think.

time_diffs

@YoshikiOhtani
Copy link
Collaborator Author

@jsitarek by the way it would be great if you could give an approval when you think it is ok to merge, thanks!

Copy link
Collaborator

@jsitarek jsitarek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry for late feedback. I approve it if you want to merge it, but I have two remarks:

  1. concerning the calculation of the elapsed time - I see your point, this is actually very tricky to get properly if the rate of events is low. But I think it is unlikely that the rate will be dominated by gamma-rays: this would be the case around the source position, but not for the whole camera. If you keep all the events in one basket it should be fine, but if you start dividing into 4 types (following the RF) this indeed can be an issue. The current code with just adding the time differences look dangerous to me - it is quite likely to have some subruns gets missing in processing and then you have runs with holes in them what are not reflected in the effective time. We could use a larger time difference (10s maybe), or apply a fit and get the elapsed time from that fit, but those would require more tests, so it is fine for me to merge it as it is.
  2. github reports that the last line is missing the newline symbol, maybe you can check if it was not removed by mistake

@YoshikiOhtani
Copy link
Collaborator Author

Thanks @jsitarek, I added a new line at the end of the file. Regarding the elapsed time, you are right and I overstated that gamma-rays are dominant in a DL3 file. Let's make another pull request when the current code of the elapsed time calculation makes a problem. I will merge this to the master now.

@YoshikiOhtani YoshikiOhtani merged commit 47e8a95 into master Jun 4, 2022
@YoshikiOhtani YoshikiOhtani deleted the dead-time-calc branch June 4, 2022 08:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Effective time for the software coincidence
2 participants