Skip to content

Disable UseTimeRange in TPC-PMT matching #886

Open
rtriozzi wants to merge 1 commit into
developfrom
feature/rtriozzi_UseTimeRange
Open

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

Conversation

@rtriozzi

@rtriozzi rtriozzi commented Feb 6, 2026

Copy link
Copy Markdown

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

@cerati cerati left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

looks as expected!

@PetrilloAtWork PetrilloAtWork left a comment

Copy link
Copy Markdown
Member

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

jas1005 commented Apr 16, 2026

Copy link
Copy Markdown
Contributor

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

jas1005 commented Apr 16, 2026

Copy link
Copy Markdown
Contributor

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

@jas1005

jas1005 commented Apr 16, 2026

Copy link
Copy Markdown
Contributor

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