-
Notifications
You must be signed in to change notification settings - Fork 40
CRTT0Tagging - icaruscode v10 #814
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Ok, the SBNObj PR needed for this PR was already open by @gputnam and it is this one: SBNSoftware/sbnobj#124 |
|
Hello, the required SBNObj PR has already been merged and is included in sbnobj v10_00_09. Can we proceed with reviewing this PR? Just to note: the new module CRTT0Tagging will be included when we merge [PR-774 |
I left a comment on the def file, I am not sure what you are asking for the actual stage 1 file? It seems the definition you have included will run this after pandora? |
|
icaruscode/fcl/caf/cafmaker_defs.fcl Line 91 in cd7ba42
There is one thing I did not see before. Apparently 2 years ago @gputnam added the old crt t0 producer in the cafs definitions. Since this is now done at stage1, can I remove this? edit : I did not see that in a commit few hours prior you already fixed this. thanks. |
…and removes the duplication of CRTT0Tagging at cafs level.\
|
@SFBayLaser I removed deprecated producers from the Stage1 definitions and I also removed duplicated CRTT0 production at caf levels according to the modifications that @gputnam did on the release/Mar25Production branch. |
|
@SFBayLaser, @gputnam, @PetrilloAtWork, @jzennamo: Could you please take a look at this PR. is it ready or needs further revisions? Thanks! |
SFBayLaser
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good to go for me
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The next one will become CRTTnaughtProducer?
…. The best candidates can now have up to 300 cm distance, this is done to give the analyzer more freedom of selection at CAF level. Additionally, specific cuts have been implemented for common values in East and West Cryos.
|
I included some changes in fcl parameters to do 2 things: |
|
trigger build LArSoft/lar*@LARSOFT_SUITE_v10_06_00 SBNSoftware/sbnalg@v10_06_00_01 SBNSoftware/sbncode@v10_06_00_01 SBNSoftware/icarus_signal_processing#24 |
|
✔️ CI build for LArSoft Succeeded on slf7 for e26:prof -- details available through the CI dashboard |
|
✔️ 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 ICARUS Failed at phase ci_tests ICARUS on slf7 for e26:prof -- 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 is the big Pull Request which will introduce CRTT0Tagging into Stage1. CRTT0Tagging will be used at the moment for introducing CRT Tagged Tracks into the calibration Ntuples. The core of the software was partly already reviewd by @PetrilloAtWork , since I believe this is the definitive Pull Request I am requesting an additional check.
In this code legacy CRTTagging code has been moved to Legacy Folder.
I am also requesting a review from @SFBayLaser because I would like guidance on the stage1 fcl and stage1 definitions to modify, I have a proposal here, but should be double checked.
I also included here @jzennamo and @gputnam because this PR is needed for the Spring Production project.
This require a dependance on two new SBNObj Data Products, I will add a comment here with the PR as soon as it is created.