-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Custom Run 3 PFScouting NanoAOD #40438
Conversation
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-40438/33578
|
A new Pull Request was created by @alintulu (Adelina Lintuluoto) for master. It involves the following packages:
@cmsbuild, @mandrenguyen, @clacaputo, @swertz, @vlimant can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
Hi @alintulu , is there a workflow where we can test this code? |
Hi @clacaputo! Since my PR to RecoBTag-Combined (cms-data/RecoBTag-Combined#49) has not been merged yet I created another branch which is a clone of this PR with the exception of storing the ONNX model files locally. You can create a custom PFScouting NanoAOD from that branch using these commands. Is this enough? My understanding is that a PFScouting NanoDQM would be great (see #39000 (comment)), but unfortunately I've failed to create one so far. @mariadalfonso would you happen to know how to best achieve this? |
Hi @alintulu , it would be better to test the code using |
Hi @clacaputo - that should be possible. Do you have an example that we could start from that's ideally close to what we need here? I don't have any experience with implementing |
Hi @clelange , you can find an examples in #40553. Sorry for the late reply |
Thank you @clacaputo! I was able to create a test, however it fails as the RecoBTag-Combined PR (cms-data/RecoBTag-Combined#49) has yet to be merged. I will ping them. |
test parameters: |
please test @alintulu , we can test cms-data/RecoBTag-Combined#49 directly here :-) |
@alintulu you can also run your own local tests by cloning your cms-data PR into your working area:
|
@cms-sw/orp-l2 is there something holding up merging this in master ? |
unhold |
This pull request is fully signed and it will be integrated in one of the next master IBs (tests are also fine). This pull request will now be reviewed by the release team before it's merged. @sextonkennedy, @rappoccio, @antoniovilela (and backports should be raised in the release meeting by the corresponding L2) |
test parameters: pull_request = cms-data/RecoBTag-Combined#49 |
please test
|
-1 Failed Tests: UnitTests The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic:
You can see more details here: Unit TestsI found 4 errors in the following unit tests: ---> test TestDQMOnlineClient-ecalgpu_dqm_sourceclient had ERRORS ---> test TestDQMOnlineClient-hcalgpu_dqm_sourceclient had ERRORS ---> test TestDQMOnlineClient-pixelgpu_dqm_sourceclient had ERRORS and more ... Comparison SummarySummary:
NANO Comparison SummarySummary:
Nano size comparison Summary:
|
I think unit tests here failed because the external update necessary for #42953 wasn't captured yet in the IB used for tests. |
@cmsbuild, please test |
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-023d2d/35113/summary.html The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic:
You can see more details here: Comparison SummarySummary:
NANO Comparison SummarySummary:
Nano size comparison Summary:
|
+1 |
as a follow up here, please include a PR to comply with the autoNano syntax #42238 and we should figure out a way to modify the workflow for a MINI input that already has the scouting content, instead of using the two file solutions |
'--geometry' : 'DB:Extended', | ||
'--datatier':'NANOAOD', | ||
'--eventcontent':'NANOAOD', | ||
'--filein':'/store/mc/Run3Summer22MiniAODv3/BulkGravitonToHH_MX1120_MH121_TuneCP5_13p6TeV_madgraph-pythia8/MINIAODSIM/124X_mcRun3_2022_realistic_v12-v3/2810000/f9cdd76c-faac-4f24-bf0c-2496c8fffe54.root', |
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.
I missed this in the first place ; we should not be using the --filein
and --secondfilein
directly but use datasets instead as for the data workflow
PR description:
This PR adds a custom NanoAOD for Run 3 PFScouting. To achieve this we had to add a couple of new files and amend a few more. I will now try to describe the reason behind each addition/amendment:
custom_run3scouting_cff.py
, which was modeled fromcustom_jme_cff.py
. All Run 3 PFScouting objects (with the exception of jets and PFCandidates) are added with the help ofSimpleFlatTableProducer
.PhysicsTools/NanoAOD/plugins/Run3ScoutingParticleToRecoPFCandidateProducer.cc
(I was not sure where to put this file, please let me know if it should be moved). The conversion allows us to cluster AK4 and AK8 jets from the recoPFCandidates. There are a list of Run3ScoutingParticle values which are not available in recoPFCandidates, and these are therefore tracked with ValueMaps.DeepBoostedJetTagInfoProducer.cc
andBoostedJetONNXJetTagsProducer.cc
had to be amended. The former was amended to allow for the specific PFScouting inputs and the latter to produce ValueMaps instead of JetTags. Since the PFScouting jets are RECO and not PAT I did not find a way of associating the output score to the jet using a JetTag (but was able with a ValueMap). I've already opened a PR with the ONNX models necessary for running inference with CMSSW (Add PartcileNet ONNX models for Run 3 PFScouting cms-data/RecoBTag-Combined#49)JetDeltaRValueMapProducer.cc
was amended to return the index instead of the value of the matched jet.DST_Run3_PFScoutingPixelTracking_v
andDST_HLTMuon_Run3_PFScoutingPixelTracking_v
. I added a boolean (isPFScouting
) by amendingNanoAODOutputModule.cc
,TriggerOutputBranches.cc
andTriggerOutputBranches.h
. By settingisPFScouting
to True, information regarding if the event is part of one of these datasets is added.PR validation:
The PR passed the tests listed at https://cms-sw.github.io/PRWorkflow.html.
With everything included, the event size is 8.4 kB/event (when ran over 5000 events from
/store/data/Run2022F/ScoutingPFRun3/RAW/v1/000/361/303/00000/37f1ab1d-94f9-4177-91e5-db46490bc69a.root
). However the ScoutingParticles account for 5.3 kB/event. It is not clear how useful they are in the NanoAOD and could perhaps be completely or partly removed (to be discussed).A pie-chart showing the most up to date event size distribution can be found here.
If this PR is a backport please specify the original PR and why you need to backport that PR. If this PR will be backported please specify to which release cycle the backport is meant for:
N/A