Introduce Barycenter Flash Match #641

JackSmedley · 2023-10-27T18:36:05Z

icaruscode updates - Add barycenter flash match producer, a configurations fcl, and fcls to run on data and Monte Carlo

Associated Pull Requests ---
sbnobj PR- SBNSoftware/sbnobj#99
sbnanaobj PR- SBNSoftware/sbnanaobj#111
sbncode PR- SBNSoftware/sbncode#392
icaruscode PR- #641

…lean up

PetrilloAtWork

First of all, despite the (usual) amount of comments, I congratulate for the job.
Here a few things that I would like you to consider before moving on:

the trigger flash: I would prefer fNominalTrigger to be called something like fTriggerDelay or equivalent (note that this definition involves a change of sign).
also for the trigger flash, consider if the definition as "the last flash to meet the criteria" is satisfactory.
and again about trigger flash: I think the distance to the slice should be expressed by radius, in the same fashion as the one used for picking the best flash. Note that this would imply a change or addition to more or less all the other branches.
documentation: I have added the documentation of the module. Missing is the explanation of the data members of the matching information. In principle the definition of the data member should go in the data product (as it is), and here some details of how that value is put together is due (for example, the centroid of the slice is defined as a representation of the position of the slice, in 3D, centimetres and in world coordinates, and this is in the data product; but here we are using a charge-weighted space point position rather than, let's say, the reconstructed primary vertex.

icaruscode/PMT/OpReco/BarycenterMatchProducer_module.cc

PetrilloAtWork · 2023-11-17T02:58:56Z

icaruscode/PMT/OpReco/BarycenterMatchProducer_module.cc

+  bool triggeredEvt = false;
+  if ( triggerHandle.isValid() && triggerHandle->size() == 1 ) {
+    trigger = (*triggerHandle).at(0);
+    if ( trigger.TriggerTime() >= 0 ) {


if ( trigger.TriggerTime() >= 0 ) {

Have you met a case where a trigger time is stored negative? is that a MC thing?

Yes, this is negative for simulated events that do not pass the trigger emulation.

icaruscode/PMT/OpReco/BarycenterMatchProducer_module.cc

icaruscode/PMT/OpReco/fcl/barycentermatch_data.fcl

icaruscode/PMT/OpReco/fcl/barycentermatch_mc.fcl

icaruscode/PMT/OpReco/BarycenterMatchProducer_module.cc

Co-authored-by: Gianluca Petrillo <petrillo.at.work@gmail.com>

…gerDelay, etc

Co-authored-by: Gianluca Petrillo <petrillo.at.work@gmail.com>

…ndex Co-authored-by: Gianluca Petrillo <petrillo.at.work@gmail.com>

…velop' into workingBranch

…ete later

PetrilloAtWork

Approved with my keyboard.

mmrosenberg · 2023-11-28T21:13:11Z

trigger build SBNSoftware/sbnobj#99 SBNSoftware/sbnanaobj#111 SBNSoftware/sbncode#392

FNALbuild · 2023-11-28T21:16:21Z

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

FNALbuild · 2023-11-28T21:16:38Z

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

FNALbuild · 2023-11-28T22:08:51Z

❌ 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

FNALbuild · 2023-11-28T22:13:05Z

❌ CI build for ICARUS Failed at phase ci_tests ICARUS on slf7 for c14: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

mmrosenberg · 2023-11-29T01:42:06Z

trigger build LArSoft/lar*@LARSOFT_SUITE_v09_79_00 SBNSoftware/sbndaq-artdaq-core@v1_08_00of1 SBNSoftware/sbnobj#99 SBNSoftware/sbnanaobj#111 SBNSoftware/sbncode#392

FNALbuild · 2023-11-29T02:09:57Z

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

FNALbuild · 2023-11-29T02:20:45Z

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

FNALbuild · 2023-11-29T03:11:13Z

⚠️ CI build for ICARUS Warning at phase ci_tests ICARUS on slf7 for e26:prof - ignored failure for unit_test -- details available through the CI dashboard

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

parent CI build details are available through the CI dashboard

FNALbuild · 2023-11-29T03:27:58Z

❌ CI build for ICARUS Failed at phase ci_tests ICARUS on slf7 for c14:prof - ignored failure for unit_test -- 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

SFBayLaser

Barring understanding the CI build failure I will approve to not hold this up

aheggest

All looks good to me

Jack Smedley added 7 commits October 23, 2023 10:10

Updated producer, changed fcls

e172c70

Expand on fcl configurations

a2ea98b

Move TTree behind a fcl parameter, start cleaning up comments

989202a

Make cryo loop more generic, don't shift data times here, generally c…

06b4598

…lean up

Fix typo

003aea6

Fix sbnobj naming convention

a64b8b7

Update fcls

4c3cc12

This was referenced Oct 27, 2023

Introduce Barycenter Flash Match SBNSoftware/sbnobj#99

Merged

Introduce Barycenter Flash Match SBNSoftware/sbnanaobj#111

Merged

Introduce Barycenter Flash Match SBNSoftware/sbncode#392

Merged

PetrilloAtWork requested review from PetrilloAtWork and aheggest November 9, 2023 21:54

PetrilloAtWork requested changes Nov 17, 2023

View reviewed changes

JackSmedley and others added 17 commits November 17, 2023 13:07

Replace getByLabel for trigger

97f63af

Co-authored-by: Gianluca Petrillo <petrillo.at.work@gmail.com>

Replace getByLabel for OpFlashes

2118571

Co-authored-by: Gianluca Petrillo <petrillo.at.work@gmail.com>

Replace getByLabel for slices

f62e5f2

Co-authored-by: Gianluca Petrillo <petrillo.at.work@gmail.com>

Don't copy trigger product

91cad5d

Co-authored-by: Gianluca Petrillo <petrillo.at.work@gmail.com>

Don't copy inputTag

28167d1

Co-authored-by: Gianluca Petrillo <petrillo.at.work@gmail.com>

Get flashPtr from handle

091ec68

Co-authored-by: Gianluca Petrillo <petrillo.at.work@gmail.com>

Remove flashVector

9a4a951

Co-authored-by: Gianluca Petrillo <petrillo.at.work@gmail.com>

Use unchecked access for opflash

6725702

Co-authored-by: Gianluca Petrillo <petrillo.at.work@gmail.com>

Get slcPtr from handle

2b199fb

Co-authored-by: Gianluca Petrillo <petrillo.at.work@gmail.com>

Remove sliceVector

cc1eec1

Co-authored-by: Gianluca Petrillo <petrillo.at.work@gmail.com>

Remove FindOneP include

7c35319

Co-authored-by: Gianluca Petrillo <petrillo.at.work@gmail.com>

Include suggested documentation

04e37c5

Co-authored-by: Gianluca Petrillo <petrillo.at.work@gmail.com>

Add several includes

c381d04

Co-authored-by: Gianluca Petrillo <petrillo.at.work@gmail.com>

Add consumes<> statements

5602894

Co-authored-by: Gianluca Petrillo <petrillo.at.work@gmail.com>

Add spactime.h include for microseconds

4c3ef9e

Co-authored-by: Gianluca Petrillo <petrillo.at.work@gmail.com>

Remove extraneous class definition

acfbd1d

Co-authored-by: Gianluca Petrillo <petrillo.at.work@gmail.com>

Remove extra lines in data fcl, pt.1

06c6ed2

Co-authored-by: Gianluca Petrillo <petrillo.at.work@gmail.com>

Jack Smedley and others added 4 commits November 17, 2023 16:38

Add DeltaY and Radius for trigger match, flip NominalTrigTime to Trig…

7ace536

…gerDelay, etc

Move TVector initializers

d333143

Co-authored-by: Gianluca Petrillo <petrillo.at.work@gmail.com>

Initialize TVectors here, reference by dimension method rather than i…

2206119

…ndex Co-authored-by: Gianluca Petrillo <petrillo.at.work@gmail.com>

Merge remote-tracking branch 'jsmedley/feature/jsmedley_Barycenter_de…

308da0a

…velop' into workingBranch

JackSmedley force-pushed the feature/jsmedley_Barycenter_develop branch 3 times, most recently from d333143 to 7ace536 Compare November 17, 2023 23:20

Jack Smedley added 4 commits November 17, 2023 17:24

Adding some comments so Git sees the chnages on these lines, will del…

d47b5ce

…ete later

Clean up trigger flash determination

506c9fc

Change name to be more descriptive

c78a226

Update brief to accomodate sign change in fTriggerDelay

27abcb8

PetrilloAtWork approved these changes Nov 20, 2023

View reviewed changes

Add producers to stage1, redefine label in cafmaker_defs

9926f06

PetrilloAtWork requested a review from SFBayLaser November 21, 2023 18:22

PetrilloAtWork mentioned this pull request Nov 28, 2023

Utility to compute the allowed track time from its hits [SBN2023A] SBNSoftware/icarusalg#77

Merged

SFBayLaser approved these changes Dec 14, 2023

View reviewed changes

aheggest approved these changes Dec 14, 2023

View reviewed changes

mmrosenberg merged commit 2ea582c into SBNSoftware:develop Dec 18, 2023

SFBayLaser assigned JackSmedley Feb 29, 2024

SFBayLaser added the enhancement New feature or request label Feb 29, 2024

Introduce Barycenter Flash Match #641

Introduce Barycenter Flash Match #641

Uh oh!

Conversation

JackSmedley commented Oct 27, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

PetrilloAtWork left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

PetrilloAtWork Nov 17, 2023

Choose a reason for hiding this comment

Uh oh!

JackSmedley Nov 17, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

PetrilloAtWork left a comment

Choose a reason for hiding this comment

Uh oh!

mmrosenberg commented Nov 28, 2023

Uh oh!

FNALbuild commented Nov 28, 2023

Uh oh!

FNALbuild commented Nov 28, 2023

Uh oh!

FNALbuild commented Nov 28, 2023

Uh oh!

FNALbuild commented Nov 28, 2023

Uh oh!

mmrosenberg commented Nov 29, 2023

Uh oh!

FNALbuild commented Nov 29, 2023

Uh oh!

FNALbuild commented Nov 29, 2023

Uh oh!

FNALbuild commented Nov 29, 2023

Uh oh!

FNALbuild commented Nov 29, 2023

Uh oh!

SFBayLaser left a comment

Choose a reason for hiding this comment

Uh oh!

aheggest left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

JackSmedley commented Oct 27, 2023 •

edited

Loading