Skip to content

Conversation

@francescopoppi
Copy link
Contributor

This PR introduce a new producer module: CRTT0Tagging
This new module produces an association with tracks and and a T0 object following CRT-TPC association. It also fills a new icarus only data product with some summary information on the result of the match, this product is called CRTTPCMatchingInfo.
Additionally associations of anab::T0 and tracks and CRT and CRTTPCMatchingInfo are stored.
This PR introduces also the CRTMatching class which contains the matching functions. In future, TripleMatching will exploits the same functions, this is why, for example, there is the GetTrackBarycenter function which is not yet used.
A brief story: there is an already existent CRTT0Matching object, but that is very outdated and not mantained anymore. The problem is that that product is widely used around our code. Not to create confusion, I added here the updated version under a new name. The new matching is based on a PCA fit of the track direction and extrapolated to the CRT plane of the CRT hit under test.
At the state of this PR, the module lives on its own and it is not included into any stage0/1/caf definitions. It is there for people to use if needed. I will follow up, once this PR is considered ok, with an additional one which starts to implement the CRTTagging of the TPC tracks in our production and analysis flow.
This module works on both data and MC! The only caveat is that for data there is an additional implementation which re-align the Top CRT modules to the TPCs, following to the alignment campaign which I performed on Run2 data. It is expected that for Run3 a new alignment txt file will be produced, but I do not have yet since we did not reprocess Run3, so using the Run2 one is way better than not using it.
I believe this SBN DocDB summarizes everything that it is introduced here: https://sbn-docdb.fnal.gov/cgi-bin/sso/RetrieveFile?docid=38008&filename=TPCCalibrationWithCRTTPCTaggedTracks.pdf&version=1

@francescopoppi francescopoppi added the enhancement New feature or request label Oct 23, 2024
@francescopoppi francescopoppi self-assigned this Oct 23, 2024
…the data product. Another improvements which is introduced is the access of truth level information for MC. In particular, I set up a variabile which is called TruthMatch which address whether at truth level the CRT hit and the Tracks are generated by the same Geant4 ID particle. So far, this variable is only stored in the tree, but I am planning to include it in the icart::CRTTPCMatchingInfo object
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 have left std::numeric_limits<std::int8_t>::max() comments.
The most relevant is that I am confused about the new data product compared to the old one: same name, completely different content, deceiving schema evolution.
It may be that you had already explained that, if so I can't find where.

I am of course available for helping with the resolution of the comments.

…mportant things: structs have been changed quite a bit, in addition I am now willingly saving information of the full PCA analysis and I am saving also second and third eigenValues/Vectors.
…nluca. In particular, software related to CRT Hit transformation has beend moved from the namespace dedicated to the matching, to a new one dedicated to data related tools
…few things are left to deal with: icarus_data and addressing legacy code.
…e information which were need have been added to an SBNObj (CRTT0TaggingInfo). In this way this object is not changed wrt legacy CRTTagging code. Additionally in this commit In the CRTT0TaggingInfo also the way the track was fitted is stored.
… the Stage1 definitions and included the new CRTT0Tagging one
…Ps, association of Tracks/PFP with CRTT0TaggingInfo
@PetrilloAtWork PetrilloAtWork self-requested a review January 31, 2025 22:32
… specifically for the MC truth has been created.
@leoaliaga
Copy link
Contributor

Just a follow-up on this PR needed for the Spring production. It looks like the review comments have been addressed.
@PetrilloAtWork, @gputnam: are there any remaining concerns from the reviewers?
@francescopoppi: there's currently a conflict in CMakeLists.txt that needs to be resolved.

@gputnam
Copy link
Contributor

gputnam commented Apr 23, 2025

@leoaliaga the PR looks good to me.

@francescopoppi
Copy link
Contributor Author

francescopoppi commented Apr 24, 2025

Hi @leoaliaga, this is still open but it is based off larsoft v9.
The one which should be merged in is #814.

I was waiting before closing this as it included the whole history of comments and requests from the reviewer which was then included into the other PR (814). But if you prefer I can close this.

@PetrilloAtWork PetrilloAtWork marked this pull request as draft April 24, 2025 17:55
@PetrilloAtWork
Copy link
Member

I have put this as draft to highlight that it should not be merged as is (in fact, it should not be merged at all since it's superseded by #814).

@PetrilloAtWork PetrilloAtWork removed their request for review April 24, 2025 17:56
@gputnam
Copy link
Contributor

gputnam commented May 13, 2025

Closing in favor of #814, now merged.

@gputnam gputnam closed this May 13, 2025
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

None yet

Development

Successfully merging this pull request may close these issues.

5 participants