Conversation
PetrilloAtWork
left a comment
There was a problem hiding this comment.
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.
|
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? |
|
Per discussion on the SBN Slack, I'm proceeding with integrating this PR. |
|
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 |
|
✔️ CI build for LArSoft Succeeded on slf7 for c14:prof -- details available through the CI dashboard |
|
❌ 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 |
|
✔️ CI build for LArSoft Succeeded on slf7 for e26:prof -- details available through the CI dashboard |
|
❌ 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 |
This PR simply disables the
UseTimeRangeoption in the TPC-PMT barycenter-based matching.The
UseTimeRangeoption 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!