Skip to content

Introduction of CRTT0TaggingInfo#117

Closed
francescopoppi wants to merge 10 commits intodevelopfrom
feature/fp_crttpctagging
Closed

Introduction of CRTT0TaggingInfo#117
francescopoppi wants to merge 10 commits intodevelopfrom
feature/fp_crttpctagging

Conversation

@francescopoppi
Copy link
Contributor

This new class introduces additional information which can be used when studying CRT tagged tracks. This object wants to be complementary to the anab::T0 object.
I chatted a bit with @henrylay97 to have a class which can be shared also with SBND people. They will update this in the future.

…bject stores additional information on the CRT T0 tagging of a TPC track. This object is currently filled and used only in icaruscode.
…d (e.g. PCA, start-to-end-vector, kalman filter, others) and one which store the MC truth of the match if available.
…its (ICAUS-like) or crtTracks (SBND-like). Also, added prefix to the variables for future distinction.
@francescopoppi
Copy link
Contributor Author

This pull request is required for this other icaruscode PR:
SBNSoftware/icaruscode#774

@francescopoppi francescopoppi self-assigned this Jan 21, 2025
@francescopoppi francescopoppi added the enhancement New feature or request label Jan 21, 2025
@henrylay97
Copy link
Member

Hi Francesco, thanks again for producing this object, it is definitely important to have to better bookkeep our matching.

SBND actually has both single hit & track matching. I maybe wasn't brilliantly clear offline, my thought was to avoid bloating the size of these objects unnecessarily was to have two objects with names something like:

  • CRTHitT0TaggingInfo
    • the object you propose here
    • renamed (and perhaps could remove singleHit from the members if you wanted)
    • SBND will also plan to make use of it - all we would need to do is add a couple more comments in the header to describe our use of your enums
  • CRTTrackT0TaggingInfo (with SBND producing this object when we're ready to)

This is just a thought - I'm happy to go with a single object if you would rather. My worry is the objects would get a fair bit bigger for every match not just for the track ones.

Copy link
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.

I support Henry's suggestion expressed in his review.
In addition, I question the truth matching flag in the object — we can talk about it if you want.
Besides a few suggested changes, ClassVersion must be added begore we can proceed (see the specific comment in classes_def.xml).

… concerns Henry's request, the object naming infos now reflect CRTHitT0TaggingInfo. Following Gianluca's requests, I removed MC truth level info from the CRTHitT0TaggingIngo and moved them into CRTHitT0TaggingTruthInfo, I have also added the appropriate class definitions.
@francescopoppi
Copy link
Contributor Author

@henrylay97 I believe the latest changes should reflect your request.
Also @PetrilloAtWork I followed your suggestion and separated MC truth into a different Info object.

@francescopoppi francescopoppi force-pushed the feature/fp_crttpctagging branch from 00e7ea8 to 451369d Compare February 3, 2025 15:37
@henrylay97
Copy link
Member

Thanks for the update Francesco - I'm happy with this!

@kjplows
Copy link
Contributor

kjplows commented Apr 7, 2025

Hi @PetrilloAtWork, could you please take a look at this PR? I believe @francescopoppi has addressed your requested changes and I'll test this now but it'd be great if you could check. Many thanks!

@kjplows
Copy link
Contributor

kjplows commented Apr 7, 2025

trigger build

@FNALbuild
Copy link

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

@FNALbuild
Copy link

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

@FNALbuild
Copy link

❌ 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
Copy link

❌ 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

❌ 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
Copy link

❌ 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

@kjplows
Copy link
Contributor

kjplows commented Apr 7, 2025

trigger build LArSoft/lar*@LARSOFT_SUITE_v10_04_07

@FNALbuild
Copy link

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

@FNALbuild
Copy link

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

@FNALbuild
Copy link

❌ 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

❌ 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
Copy link

⚠️ 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

@FNALbuild
Copy link

⚠️ CI build for ICARUS Warning at phase ci_tests ICARUS 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 ICARUS phase logs

parent CI build details are available through the CI dashboard

@kjplows kjplows moved this from Partially reviewed to Urgent checks in SBN software development Apr 7, 2025
@kjplows
Copy link
Contributor

kjplows commented Apr 10, 2025

@PetrilloAtWork is this ok to merge or does the PR need additional work? Cheers!

@kjplows
Copy link
Contributor

kjplows commented Apr 18, 2025

@francescopoppi can you resolve these conflicts please? given #124 was merged, some things need to be resolved here (does that PR mean this can be closed?). Thanks!

@francescopoppi
Copy link
Contributor Author

After discussing with @kjplows , this PR is going top be closed. It was an earlier version of #124

@github-project-automation github-project-automation bot moved this from Urgent checks to Done in SBN software development Apr 23, 2025
@francescopoppi
Copy link
Contributor Author

I actually put the comment in the previous comment... This is old, from the larsoft v9 era

@francescopoppi francescopoppi deleted the feature/fp_crttpctagging branch April 23, 2025 08:59
@kjplows kjplows moved this from Done to 2025 PRs in SBN software development Jan 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

Status: 2025 PRs

Development

Successfully merging this pull request may close these issues.

6 participants