search

XENONnT / straxen

Streaming analysis for XENON
BSD 3-Clause "New" or "Revised" License
20 stars 33 forks source link

Manually check event overlapping #1214

Closed FaroutYLq closed 1 year ago

FaroutYLq commented 1 year ago

What does the code in this PR do / what does it improve?

Manually check event overlapping, since we are skipping such assertion in the updated strax.abs_time_to_prev_next_interval.

Can you briefly describe how it works?

Did what strax.abs_time_to_prev_next_interval did before modification:

_things_do_not_overlap = (strax.endtime(things)[:-1] - things['time'][1:]) <= 0
assert np.all(_things_do_not_overlap), 'Things overlap!'

Can you give a minimal working example (or illustrate with a figure)?

FaroutYLq commented 1 year ago

Trying to solve this issue

dachengx commented 1 year ago

Maybe we should do this in Events https://github.com/XENONnT/straxen/blob/master/straxen/plugins/events/events.py?

dachengx commented 1 year ago

We currently allow peaks to be overlapping but not events.

FaroutYLq commented 1 year ago

We currently allow peaks to be overlapping but not events.

@dachengx sorry I am confused. In the linked Events script there is no abs_time_to_prev_next_interval?

dachengx commented 1 year ago

@dachengx sorry I am confused. In the linked Events script there is no abs_time_to_prev_next_interval?

Yes, you are correct. I was wondering whether we should completely prohibit overlapping events in straxen. Then the warning related to this will not appear. abs_time_to_prev_next_interval in VetoProximity also just test whether events are overlapping.

FaroutYLq commented 1 year ago

Oh I see the argument. I think what you said is fully sensible. So a better place to do this is in events.

FaroutYLq commented 1 year ago

But I would suggest we keep doing what this PR did for now, since right now I just want to make sure we are not missing any functionality from the straxPR. Let's do one thing each time

dachengx commented 1 year ago

But I would suggest we keep doing what this PR did for now, since right now I just want to make sure we are not missing any functionality from the straxPR. Let's do one thing each time

No objection, my friend.

coveralls commented 1 year ago

Coverage Status

coverage: 93.556% (+0.03%) from 93.531% when pulling 93f3a2a107887dd8bc55efc1df3b7a5e4184eaff on assert_no_overlapping into 5f232eb2c1ab39e11fb14d4e6ee2db369ed2c2ec on master.

dachengx commented 1 year ago

How's this now @FaroutYLq

FaroutYLq commented 1 year ago

Sorry is there something you want me to update here?

dachengx commented 1 year ago

Sorry is there something you want me to update here?

I moved the dingoes directly to Event plugin. Do you think it is OK?

FaroutYLq commented 1 year ago

No objection from me