Skip to content

Disable UseTimeRange in TPC-PMT matching #886

Open
rtriozzi wants to merge 1 commit intodevelopfrom
feature/rtriozzi_UseTimeRange
Open

Disable UseTimeRange in TPC-PMT matching #886
rtriozzi wants to merge 1 commit intodevelopfrom
feature/rtriozzi_UseTimeRange

Conversation

@rtriozzi
Copy link
Copy Markdown

@rtriozzi rtriozzi commented Feb 6, 2026

This PR simply disables the UseTimeRange option in the TPC-PMT barycenter-based matching.

The UseTimeRange option uses a slice's extent along the drift direction to constrain the time range for matching optical flashes, reducing the amount of candidate flashes. This time range is especially tight when the slice crosses the cathode.

While this is straightforward for tracks, it is definitely not for showers, or more generally for slices involving showery particles and deposits crossing the cathode. The code has no way of knowing the nature of the particle crossing the cathode, so the safest bet here is to just disable this option.

I have debugged and discussed this with @cerati and @PetrilloAtWork, and I'm adding them as reviewers. Thanks!

@rtriozzi rtriozzi self-assigned this Feb 6, 2026
@rtriozzi rtriozzi added bug Something isn't working enhancement New feature or request labels Feb 6, 2026
Copy link
Copy Markdown
Contributor

@cerati cerati left a comment

Choose a reason for hiding this comment

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

looks as expected!

Copy link
Copy Markdown
Member

@PetrilloAtWork PetrilloAtWork left a comment

Choose a reason for hiding this comment

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

Technically it is ok.
Practically, this has potentially implications in track-based analyses, and since it's a experiment-wide change the decision should be led by the analysis conveners.

@jas1005
Copy link
Copy Markdown
Contributor

jas1005 commented Apr 16, 2026

I'm working on incorporating all of the open PRs into develop, and this one is a bit special. @PetrilloAtWork, @cerati, and/or @rtriozzi, was this change ever discussed by analysis conveners? Is such a discussion relevant/needed anymore?

@jas1005
Copy link
Copy Markdown
Contributor

jas1005 commented Apr 16, 2026

Per discussion on the SBN Slack, I'm proceeding with integrating this PR.

@jas1005
Copy link
Copy Markdown
Contributor

jas1005 commented Apr 16, 2026

trigger build larsoft@v10_20_04 LArSoft/lar*@LARSOFT_SUITE_v10_20_04 SBNSoftware/sbnalg@v10_20_04 SBNSoftware/sbnobj@v10_20_04 SBNSoftware/sbnanaobj@v10_20_04 SBNSoftware/sbndaq-artdaq-core@v1_10_06 SBNSoftware/sbncode@v10_20_04 SBNSoftware/icarusutil@v10_15_00 SBNSoftware/icaruscode@v10_20_03

@FNALbuild
Copy link
Copy Markdown
Collaborator

✔️ CI build for LArSoft Succeeded on slf7 for c14:prof -- details available through the CI dashboard

@FNALbuild
Copy link
Copy Markdown
Collaborator

❌ CI build for ICARUS Failed at phase build ICARUS on slf7 for c14:prof -- details available through the CI dashboard

🚨 For more details about the failed phase, check the build ICARUS phase logs

parent CI build details are available through the CI dashboard

@FNALbuild
Copy link
Copy Markdown
Collaborator

✔️ CI build for LArSoft Succeeded on slf7 for e26:prof -- details available through the CI dashboard

@FNALbuild
Copy link
Copy Markdown
Collaborator

❌ CI build for ICARUS Failed at phase ci_tests ICARUS on slf7 for e26:prof - ignored failure for unit_test - ignored warnings for build -- details available through the CI dashboard

🚨 For more details about the failed phase, check the ci_tests ICARUS phase logs

parent CI build details are available through the CI dashboard

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants