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

Top and bottom timing parameters at event and peak level #1119

Merged
merged 3 commits into from
Dec 13, 2022

Conversation

terliuk
Copy link
Contributor

@terliuk terliuk commented Nov 10, 2022

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

This PR adds plugins computing timing parameters (center time, rise time, 50% and 50% ranges) at peak level as well as event level, depending on needs of an individual analyst.

Can you briefly describe how it works?

Technical remark, in order to compute parameter, it forms "fake peaks" based on data_top for top array or (data - data_top) for bottom array. And then reuses the quantile calculation from strax to avoid unnecessary code duplication.

To get parameters at peak level, one needs to run:

st.get_array(run_id, 'peak_top_bottom_params')

which will return fields

  • center_time_top, center_time_bot
  • rise_time_top , rise_time_bot
  • range_50p_area_top, range_50p_area_bot
  • range_90p_area_top, range_90p_area_bot
    with definitions the same as in peak_basics.

To get it at event level one needs:

st.get_array(run_ids, "event_top_bottom_params")

which will provide the similar fields, but for main/alt s1/s2 forming the event, e.g. s1_rise_time_top, alt_s2_center_time etc.

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

Few plots from few Kr83m runs:

time difference between top and bottom center times as function of Z:

or rise time of bottom array

Please include the following if applicable:

  • Update the docstring(s)

@coveralls
Copy link

coveralls commented Nov 10, 2022

Coverage Status

Coverage increased (+0.04%) to 93.971% when pulling cdd153d on top_bottom_parameters into 1a4a294 on master.

@terliuk terliuk marked this pull request as ready for review November 10, 2022 15:21
@FaroutYLq
Copy link
Contributor

FaroutYLq commented Nov 20, 2022

Something general idea: do we want to put the parameters in peaklets level instead of above peaks, so these info can be used in peak classification?
nvm I was saying something stupid.

Copy link
Contributor

@FaroutYLq FaroutYLq left a comment

Choose a reason for hiding this comment

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

Looks good to me! Check here for plots.

@JoranAngevaare
Copy link
Contributor

Thanks Andrii for the PR and Lanqing for the nice review!

@JoranAngevaare JoranAngevaare merged commit 6a1a9bd into master Dec 13, 2022
@JoranAngevaare JoranAngevaare deleted the top_bottom_parameters branch December 13, 2022 10:23
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.

4 participants