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

Replace z from z_naive to z_dv_corr #1239

Merged
merged 4 commits into from Aug 23, 2023
Merged

Replace z from z_naive to z_dv_corr #1239

merged 4 commits into from Aug 23, 2023

Conversation

dachengx
Copy link
Collaborator

@dachengx dachengx commented Aug 22, 2023

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

It replaces z to z_dv_corr, alt_s1_z to alt_s1_z_dv_corr and alt_s2_z to alt_s2_z_dv_corr. Because z_dv_corr already played a good correction for z.

Reference: https://xe1t-wiki.lngs.infn.it/doku.php?id=xenon:xenonnt:terliuk:drift_field_z_bias_correction

Can you briefly describe how it works?

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.

@dachengx dachengx changed the title Change z to z_dv_corr Change z to z_dv_corr Aug 22, 2023
straxen/plugins/events/event_positions.py Outdated Show resolved Hide resolved
straxen/plugins/events/event_positions.py Outdated Show resolved Hide resolved
straxen/plugins/events/event_positions.py Show resolved Hide resolved
@dachengx dachengx changed the title Change z to z_dv_corr Replace z from z_naive to z_dv_corr Aug 22, 2023
@coveralls
Copy link

coveralls commented Aug 22, 2023

Coverage Status

coverage: 93.556% (+0.02%) from 93.534% when pulling d188688 on z_dv_corr_to_z into 9a1e1fe on master.

@dachengx dachengx marked this pull request as ready for review August 22, 2023 19:54
@JYangQi00
Copy link
Contributor

Thanks Dacheng, I will try and process some event_info on a run just to make sure z = z_dv_corr then accept!

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.

Everything looks good to me and I tested that both event_info and event_positions reflects these changes for run '053502'. The 'z' is consistent with 'z_dv_corr' now instead of 'z_naive'.

@dachengx
Copy link
Collaborator Author

Thanks Dacheng, I will try and process some event_info on a run just to make sure z = z_dv_corr then accept!

Another thing to check is the change of hash. If a PR changes fields or values of fields, the hash must be changed.

The easiest way to change the hash is to change the version of the plugin. I am sure that the version is changed so will merge it. : )

@dachengx dachengx merged commit 8f24f29 into master Aug 23, 2023
7 checks passed
@dachengx dachengx deleted the z_dv_corr_to_z branch August 23, 2023 14:24
@JYangQi00
Copy link
Contributor

Ah yup, the hash has been changed as well, sorry, I forgot to mention that.

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