Skip to content

Forced trigger#70

Closed
tdixon97 wants to merge 1 commit intomainfrom
forced_trigger
Closed

Forced trigger#70
tdixon97 wants to merge 1 commit intomainfrom
forced_trigger

Conversation

@tdixon97
Copy link
Collaborator

@tdixon97 tdixon97 commented Jan 29, 2026

This gives the basic function to create the forced trigger library in a format that https://reboost.readthedocs.io/en/stable/api/reboost.spms.html#reboost.spms.pe.corrected_photoelectrons can use.

For now we have not performed a forced trigger correction that returns the photon times, rather just the summed pe per channel. I think this is ok to start with.

If we want to instead obtain the nested vector of pe times we need new functionality in reboost.

@codecov
Copy link

codecov bot commented Jan 29, 2026 •
edited
Loading

Codecov Report

❌ Patch coverage is 5.55556% with 17 lines in your changes missing coverage. Please review.
✅ Project coverage is 62.32%. Comparing base (a8e6c1a) to head (e9f6802).
⚠️ Report is 18 commits behind head on main.

Files with missing lines Patch % Lines
workflow/src/legendsimflow/reboost.py 5.55% 17 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             main      #70      +/-   ##
==========================================
- Coverage   63.05%   62.32%   -0.74%     
==========================================
  Files          19       19              
  Lines        1394     1412      +18     
==========================================
+ Hits          879      880       +1     
- Misses        515      532      +17

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

gipert
gipert reviewed Jan 29, 2026
View reviewed changes
@gipert
Copy link
Member

gipert commented Jan 29, 2026

I can have a look at how much work is to do the correction on the times today.

@gipert gipert marked this pull request as draft January 29, 2026 08:51
@tdixon97
Copy link
Collaborator Author

tdixon97 commented Jan 29, 2026 via email

@gipert
Copy link
Member

gipert commented Feb 1, 2026

i think we should for each simulated event look for a sample of random coincident pulse times in the data (in a certain window, e.g. 6 us) and just save this as an additional field in the opt tier. i think this would be useful to develop time/pattern based veto classifiers

@tdixon97
Copy link
Collaborator Author

tdixon97 commented Feb 1, 2026 via email

@tdixon97 tdixon97 marked this pull request as ready for review February 2, 2026 15:59
Copilot AI review requested due to automatic review settings February 2, 2026 15:59
Copilot AI reviewed Feb 2, 2026
View reviewed changes
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR implements a forced trigger library extraction function for SiPM photoelectron spectrum corrections. The implementation moves an existing utility function into the local codebase and adds new functionality to process forced trigger events.

Changes:

  • Moved get_remage_detector_uids from reboost.utils to local reboost.py module
  • Added get_forced_trigger_library function to extract and format forced trigger events for SiPM corrections
  • Updated type hints to use HPGeScalarRZField instead of deprecated HPGeRZField

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.

File Description
workflow/src/legendsimflow/scripts/tier/evt.py Updated import to use local reboost_utils.get_remage_detector_uids instead of reboost.utils
workflow/src/legendsimflow/reboost.py Added get_remage_detector_uids function, implemented get_forced_trigger_library, and updated HPGe type hints

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@gipert
Copy link
Member

gipert commented Feb 3, 2026

Well @rosannadeckert is already trying to use simulations to study the classifier. In any case my point is that you already have the machinery to read in the evt data, so it should be quite simple to just make store the pulse information, instead of computing a derived quantity.

@tdixon97
Copy link
Collaborator Author

tdixon97 commented Feb 3, 2026

I think we should first do the simplest case to get our background model PDFs, then if @rosannadeckert needs the times, maybe she can contribute, since changes are then also needed in reboost

@rosannadeckert
Copy link

rosannadeckert commented Feb 3, 2026

sure, i can help! what kind of change is needed in reboost?

@gipert
Copy link
Member

gipert commented Feb 3, 2026

let me have a look at what tobi wrote here so i can advise

@tdixon97
Copy link
Collaborator Author

tdixon97 commented Feb 3, 2026

I think the change is rather minimal just append lists not sum

@gipert gipert marked this pull request as draft February 4, 2026 15:04
@gipert gipert linked an issue Feb 4, 2026 that may be closed by this pull request
Random coincidences correction to photoelectron times #50
Open
@gipert
Copy link
Member

gipert commented Feb 5, 2026

@rosannadeckert do you want to work on this?

@rosannadeckert
Copy link

rosannadeckert commented Feb 6, 2026

yes

@gipert
Copy link
Member

gipert commented Feb 6, 2026

Superseded by #93.

@gipert gipert closed this Feb 6, 2026
@tdixon97
Copy link
Collaborator Author

tdixon97 commented Feb 6, 2026 via email

@gipert
Copy link
Member

gipert commented Feb 6, 2026

#93 includes the code from this PR. we can develop the routines here for convenience and move them to reboost later, as we still have to do for all the reboost.py module

@tdixon97
Copy link
Collaborator Author

tdixon97 commented Feb 6, 2026 via email

@gipert
Copy link
Member

gipert commented Feb 6, 2026

@rosannadeckert can't push to your branch

@tdixon97
Copy link
Collaborator Author

tdixon97 commented Feb 6, 2026 via email

@gipert gipert deleted the forced_trigger branch February 15, 2026 21:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@gipert gipert gipert left review comments

Copilot code review Copilot Copilot left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Random coincidences correction to photoelectron times

3 participants

@tdixon97 @gipert @rosannadeckert