This PR introduces CRT Tagging usage into cafs for analyzers #536

francescopoppi · 2025-04-24T14:46:26Z

This Pull request introduces the CRTHit tagging into CAF, so that ICARUS analyzers can use relevant information for out of time rejection.
This PR uses an existing SR and Fill function which have been modified accordingly.
This PR needs a new dependence on this PR for sbnanaobj:
SBNSoftware/sbnanaobj#139

…g the usage of CRTT0Tagged tracks for analysis and out of time cosmic rejection

gputnam

Approved, modulo this change.

sbncode/CAFMaker/FillReco.cxx

…ak withcout a check on the hitmatchinfo size

francescopoppi · 2025-04-24T15:18:47Z

Request from @gputnam addressed

kjplows · 2025-04-24T18:38:30Z

sbncode/CAFMaker/CAFMaker_module.cc

+
+    // note: in the ICARUS case we do not need a slice_tag_stuff for CRTHitMatch as it is
+    // common to East and West
+    /*
    art::FindManyP<anab::T0> fmCRTHitMatch =
      FindManyPStrict<anab::T0>(slcTracks, evt,
               fParams.CRTHitMatchLabel() + slice_tag_suff);


I'm hesitant to just get rid off the slice_tag_suff, as I suspect SBND may need it. Summoning @henrylay97 to comment on this

Could anybody explain how SBND stores the tracks? ICARUS splits them in two data products by their cryostat, which I assume SBND does not.

At SBND we store our tracks as a single collection - due to the fact we have just one cryostat. So we don't use the suffix approach.

PetrilloAtWork

It looks like there are three points to address:

Whether SBND needs the suffix on CRT data product tag.
The strategy for identifying the location of a hit.
The support for multiple CRT hits associated to the same track.

Possibly some off-GitHub discussion is needed too.

PetrilloAtWork · 2025-04-24T19:53:51Z

sbncode/CAFMaker/CAFMakerParams.h

@@ -275,7 +275,13 @@ namespace caf
    Atom<string> CRTHitMatchLabel {
      Name("CRTHitMatchLabel"),
      Comment("Base label of track to CRT hit matching producer."),
-      "pandoraTrackCRTHit"
+      "CRTT0Tagging"


I recommend the default value not to be silently changed.
Is SBND already using this feature? if so, do they abide either to the old default, or to the newly proposed one?

So I agree with Gianluca on the general principle.

Although for this scenario it is okay for SBND. We use the CRTSpacePoint matching rather than CRTHit we also have different methods for the track matching products so I think we are totally factorised from each other here.

silent change reverted.

but I will also have to change the carmaker icarus definitions (which I should do anyway)

sbncode/CAFMaker/CAFMaker_module.cc

PetrilloAtWork · 2025-04-24T20:03:08Z

sbncode/CAFMaker/CAFMaker_module.cc

+
+    // note: in the ICARUS case we do not need a slice_tag_stuff for CRTHitMatch as it is
+    // common to East and West
+    /*
    art::FindManyP<anab::T0> fmCRTHitMatch =
      FindManyPStrict<anab::T0>(slcTracks, evt,
               fParams.CRTHitMatchLabel() + slice_tag_suff);


Could anybody explain how SBND stores the tracks? ICARUS splits them in two data products by their cryostat, which I assume SBND does not.

sbncode/CAFMaker/CAFMaker_module.cc

sbncode/CAFMaker/FillReco.cxx

henrylay97

In general this looks fine from an SBND point of view. Our CRT reconstruction uses different objects to ICARUS and therefore the CAF filling is completely factorised for the two experiments. Nothing Francesco does here will impact us. I would like to see that verified in the CI tests once this PR is ready to be tested.

I will leave others (as Gianluca already has) to comment on the implementation details.

henrylay97 · 2025-04-25T15:21:10Z

sbncode/CAFMaker/CAFMakerParams.h

@@ -275,7 +275,13 @@ namespace caf
    Atom<string> CRTHitMatchLabel {
      Name("CRTHitMatchLabel"),
      Comment("Base label of track to CRT hit matching producer."),
-      "pandoraTrackCRTHit"
+      "CRTT0Tagging"


So I agree with Gianluca on the general principle.

Although for this scenario it is okay for SBND. We use the CRTSpacePoint matching rather than CRTHit we also have different methods for the track matching products so I think we are totally factorised from each other here.

henrylay97 · 2025-04-25T15:24:01Z

sbncode/CAFMaker/CAFMaker_module.cc

+
+    // note: in the ICARUS case we do not need a slice_tag_stuff for CRTHitMatch as it is
+    // common to East and West
+    /*
    art::FindManyP<anab::T0> fmCRTHitMatch =
      FindManyPStrict<anab::T0>(slcTracks, evt,
               fParams.CRTHitMatchLabel() + slice_tag_suff);


At SBND we store our tracks as a single collection - due to the fact we have just one cryostat. So we don't use the suffix approach.

francescopoppi · 2025-04-28T15:06:48Z

Addressed the requests from @PetrilloAtWork, but I would like clarification on the point:
The strategy for identifying the location of a hit.
What do you mean exactly?

PetrilloAtWork

Thank you for the clarifications.
I would just remove the code commented out, since it is not good for SBND and it is not good for ICARUS, and it never was.

kjplows · 2025-04-29T16:55:49Z

trigger build

FNALbuild · 2025-04-29T16:58:47Z

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

FNALbuild · 2025-04-29T16:58:51Z

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

FNALbuild · 2025-04-29T16:59:49Z

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

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

parent CI build details are available through the CI dashboard

FNALbuild · 2025-04-29T16:59:50Z

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

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

parent CI build details are available through the CI dashboard

FNALbuild · 2025-04-29T16:59:54Z

❌ 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 · 2025-04-29T16:59:56Z

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

PetrilloAtWork

Approved!

kjplows · 2025-04-29T19:41:07Z

trigger build LArSoft/lar*@LARSOFT_SUITE_v10_05_00 SBNSoftware/sbnanaobj#139

FNALbuild · 2025-04-29T19:54:25Z

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

FNALbuild · 2025-04-29T20:03:12Z

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

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

parent CI build details are available through the CI dashboard

FNALbuild · 2025-04-29T20:04:00Z

❌ 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 · 2025-04-29T20:06:56Z

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

FNALbuild · 2025-04-29T20:24:01Z

❌ CI build for ICARUS Failed at phase build ICARUS on slf7 for e26: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 · 2025-04-29T20:28:12Z

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

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

parent CI build details are available through the CI dashboard

…Cafs

kjplows · 2025-05-01T06:07:24Z

trigger build LArSoft/lar*@LARSOFT_SUITE_v10_05_00 SBNSoftware/sbnanaobj#139

FNALbuild · 2025-05-01T06:19:10Z

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

FNALbuild · 2025-05-01T06:20:52Z

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

FNALbuild · 2025-05-01T06:32:13Z

❌ 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 · 2025-05-01T06:32:41Z

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

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

parent CI build details are available through the CI dashboard

FNALbuild · 2025-05-01T06:33:05Z

❌ CI build for ICARUS Failed at phase build ICARUS on slf7 for e26: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 · 2025-05-01T06:33:59Z

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

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

parent CI build details are available through the CI dashboard

kjplows · 2025-05-01T06:46:43Z

checked merge diff manually and no impact is made on the CRT Tagging info. Weird how the CI didn't seem to pick up sbnanaobj/#139 .

Built on sbndbuild03 with both branches so given this passed CI before merge commit this is approved

This commit introduces CRTT0Tagging for ICARUS into the CAFs, enablin…

ca8a96e

…g the usage of CRTT0Tagged tracks for analysis and out of time cosmic rejection

francescopoppi added the enhancement New feature or request label Apr 24, 2025

francescopoppi requested review from gputnam and kjplows April 24, 2025 14:46

francescopoppi self-assigned this Apr 24, 2025

francescopoppi requested a review from PetrilloAtWork April 24, 2025 14:58

gputnam requested changes Apr 24, 2025

View reviewed changes

sbncode/CAFMaker/FillReco.cxx Outdated Show resolved Hide resolved

Addressed a request from Gray that correctly pointed out it could bre…

afe40cd

…ak withcout a check on the hitmatchinfo size

kjplows reviewed Apr 24, 2025

View reviewed changes

kjplows requested a review from henrylay97 April 24, 2025 18:40

PetrilloAtWork requested changes Apr 24, 2025

View reviewed changes

kjplows added this to SBN software development Apr 25, 2025

kjplows moved this to Open pull requests in SBN software development Apr 25, 2025

henrylay97 reviewed Apr 25, 2025

View reviewed changes

This commit addresses requests from the reviewers.

92ee59d

gputnam self-requested a review April 28, 2025 20:52

gputnam approved these changes Apr 28, 2025

View reviewed changes

PetrilloAtWork approved these changes Apr 29, 2025

View reviewed changes

kjplows moved this from Open pull requests to Testing in SBN software development Apr 29, 2025

removed outdated comment.

e86e34e

PetrilloAtWork self-requested a review April 29, 2025 17:37

PetrilloAtWork approved these changes Apr 29, 2025

View reviewed changes

Merge branch 'release/Mar25Production' into feature/fp_CRTT0Taggingin…

e0e6d1c

…Cafs

kjplows moved this from Testing to To merge in SBN software development May 1, 2025

kjplows merged commit afb8350 into release/Mar25Production May 1, 2025
2 of 6 checks passed

github-project-automation bot moved this from To merge to Done in SBN software development May 1, 2025

kjplows moved this from Done to Recently done in SBN software development May 1, 2025

kjplows mentioned this pull request May 1, 2025

Merge release/Mar25Production into develop #538

Merged

kjplows moved this from Recently done to Done in SBN software development May 8, 2025

This PR introduces CRT Tagging usage into cafs for analyzers #536

This PR introduces CRT Tagging usage into cafs for analyzers #536

Uh oh!

Conversation

francescopoppi commented Apr 24, 2025 • edited by PetrilloAtWork Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gputnam left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

francescopoppi commented Apr 24, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

PetrilloAtWork left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

henrylay97 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

francescopoppi commented Apr 28, 2025

Uh oh!

PetrilloAtWork left a comment

Choose a reason for hiding this comment

Uh oh!

kjplows commented Apr 29, 2025

Uh oh!

FNALbuild commented Apr 29, 2025

Uh oh!

FNALbuild commented Apr 29, 2025

Uh oh!

FNALbuild commented Apr 29, 2025

Uh oh!

FNALbuild commented Apr 29, 2025

Uh oh!

FNALbuild commented Apr 29, 2025

Uh oh!

FNALbuild commented Apr 29, 2025

Uh oh!

PetrilloAtWork left a comment

Choose a reason for hiding this comment

Uh oh!

kjplows commented Apr 29, 2025

Uh oh!

FNALbuild commented Apr 29, 2025

Uh oh!

FNALbuild commented Apr 29, 2025

Uh oh!

FNALbuild commented Apr 29, 2025

Uh oh!

FNALbuild commented Apr 29, 2025

Uh oh!

FNALbuild commented Apr 29, 2025

Uh oh!

FNALbuild commented Apr 29, 2025

Uh oh!

kjplows commented May 1, 2025

francescopoppi commented Apr 24, 2025 •

edited by PetrilloAtWork

Loading

kjplows commented May 1, 2025 •

edited

Loading