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

Iterative Bayesian 3D (t, x, y) S2 merging #1513

Open
wants to merge 22 commits into
base: master
Choose a base branch
from
Open

Conversation

dachengx
Copy link
Collaborator

@dachengx dachengx commented Jan 7, 2025

Before you submit this PR: make sure to put all operations-related information in a wiki-note, a PR should be about code and is publicly accessible

Depends on AxFoundation/strax#946 and AxFoundation/strax#951

Combining #1495 and #1512.

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

Merge S2 based on the probability that the two peaklets are from the same normal distribution.

Can you briefly describe how it works?

For more details please refer to https://xe1t-wiki.lngs.infn.it/doku.php?id=xenon:xenonnt:analysis:s2_merging_time_position.

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 marked this pull request as ready for review January 8, 2025 17:56
@coveralls
Copy link

coveralls commented Jan 10, 2025

Coverage Status

coverage: 89.524% (+0.05%) from 89.471%
when pulling 98a295d on merged_s2s_3d
into 21b5e34 on master.

}
local_rses = {"UC_DALI_USERDISK": r".rcc.", "SDSC_USERDISK": r".sdsc."}
local_rses = {"UC_DALI_USERDISK": r"^dali(\w*).rcc.", "SDSC_USERDISK": r".sdsc."}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Only load UC_DALI_USERDISK rucio when on dali servers.

Copy link
Contributor

Choose a reason for hiding this comment

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

Isnt selecting that the merging is type 1 or 2 a bit different from making sure it is not type 20? As maybe type 0 and other types get merged? Why not just exclude type 20 instead?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Because S0 can anyway not trigger an event before this PR. Before this PR only S2 trigger events.


n_tpc_pmts = straxen.URLConfig(type=int, help="Number of TPC PMTs")

n_top_pmts = straxen.URLConfig(type=int, help="Number of top TPC array PMTs")

default_reconstruction_algorithm = straxen.URLConfig(
Copy link
Contributor

Choose a reason for hiding this comment

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

This variable is never used, the code seems to stick to DEFAULT_POSREC_ALGO.

* Add URLconfig option and code to use contour area for weights

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* add use_uncertainty_weights to get_merge_instructions

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Resolving pre-commit-hook changes

* use copied contour variable to avoid self-modification issues

* remove doubled line

* Minor change

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: juehang <qinjuehang@rice.edu>
Co-authored-by: dachengx <dx2227@columbia.edu>
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