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

Find time difference and properties of nearest triggering peaks #1301

Merged
merged 5 commits into from Dec 4, 2023

Conversation

dachengx
Copy link
Collaborator

@dachengx dachengx commented Dec 3, 2023

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

Add a plugin to find the nearest triggering peaks. "triggering" is defined the same as the Events plugin.

The plugin is required by study https://xe1t-wiki.lngs.infn.it/doku.php?id=xenon:xenonnt:ar37_l_shell_sensi:ac_validation, which is further required by https://xe1t-wiki.lngs.infn.it/doku.php?id=xenon:xenonnt:ar37_l_shell_sensi.

Can you briefly describe how it works?

First look for all triggering peaks, then find the groups of triggering peaks for each peak within a two-side window defined by shadow_time_window_backward. Then calculate the time difference of the nearest triggering peak to the left and right the each peak. If not found, assign shadow_time_window_backward to the time difference.

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

Please include the following if applicable:

  • Update the docstring(s)
  • Update the documentation
  • Tests to check the (new) code is working as desired.
  • Does it solve one of the open issues on github?

Notes on testing

  • Until the automated tests pass, please mark the PR as a draft.
  • On the XENONnT fork we test with database access, on private forks there is no database access for security considerations.

All italic comments can be removed from this template.

@coveralls
Copy link

coveralls commented Dec 3, 2023

Coverage Status

coverage: 92.841% (+0.09%) from 92.753%
when pulling 64704b4 on nearest_triggering
into 4c00a9a on master.

@dachengx dachengx marked this pull request as ready for review December 3, 2023 06:59
argsort = np.argsort(peaks["center_time"], kind="mergesort")
_peaks = np.sort(peaks, order="center_time")
result = np.zeros(len(peaks), self.dtype)
_quick_assign(argsort, result, self.compute_triggering(peaks, _peaks))
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes.

Copy link
Contributor

@JYangQi00 JYangQi00 left a comment

Choose a reason for hiding this comment

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

Hi Dacheng, everything looks good to me! Would you like me to simulate some waveforms to test these plugins? If you think it's necessary, I can do that.

Just to summarize these plugins to make sure we're on the same page:

  • _is_triggering is moved from some statements in the compute function of Events into a separate function
  • PeakNearestTriggering does exactly what you said in your original description
  • EventNearestTriggering copies the information from peak_nearest_triggering over for the main S1 and S2 for each event

If I am understanding this correctly then I will approve.

@dachengx
Copy link
Collaborator Author

dachengx commented Dec 4, 2023

Hi Dacheng, everything looks good to me! Would you like me to simulate some waveforms to test these plugins? If you think it's necessary, I can do that.

Just to summarize these plugins to make sure we're on the same page:

  • _is_triggering is moved from some statements in the compute function of Events into a separate function
  • PeakNearestTriggering does exactly what you said in your original description
  • EventNearestTriggering copies the information from peak_nearest_triggering over for the main S1 and S2 for each event

If I am understanding this correctly then I will approve.

Thank you @JYangQi00 ! The plugin is validated in the study linked above so not need to WFSim. And yes your understandings are all correct.

@JYangQi00
Copy link
Contributor

Hi Dacheng, everything looks good to me! Would you like me to simulate some waveforms to test these plugins? If you think it's necessary, I can do that.
Just to summarize these plugins to make sure we're on the same page:

  • _is_triggering is moved from some statements in the compute function of Events into a separate function
  • PeakNearestTriggering does exactly what you said in your original description
  • EventNearestTriggering copies the information from peak_nearest_triggering over for the main S1 and S2 for each event

If I am understanding this correctly then I will approve.

Thank you @JYangQi00 ! The plugin is validated in the study linked above so not need to WFSim. And yes your understandings are all correct.

Great!

@dachengx dachengx merged commit 933cc54 into master Dec 4, 2023
9 checks passed
@dachengx dachengx deleted the nearest_triggering branch December 4, 2023 19:54
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.

None yet

3 participants