-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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
Integration of extended ParticleNet trainings for simultaneous jet flavor tagging, pT regression, and tau ID + reconstruction #40745
Merged
Merged
Changes from 16 commits
Commits
Show all changes
25 commits
Select commit
Hold shift + click to select a range
17471e8
add ParticleNetFromMiniAOD
30c3043
a few fixes after runtime testing
419dc21
remove commented line
5c0afbd
reformat new code
0659445
add test cfgs for ParticleNetFromMiniAOD
17457a5
first implementation of new extended ParticleNet networks in miniAOD …
06e0181
clean up saved output nodes
da0325f
fix reference to PNet miniAOD tags
7e5767b
remove DeepAK8 from miniAOD and nanoAOD, include full ParticleNet mas…
8903fcd
enable evaluation of new ParticleNet trainings in nanoAOD step
ac075a6
fix printouts
3bca26d
add PNet regression neutrino correction
57484c4
fix jet and JEC reference in nanoAOD PNet reevaluation for PUPPI
5e7f771
remove puppi weight from PNet feature evaluator and fix nano AK4 PUPP…
69a2c39
fix CvsL definition
acbdc84
add PNet to NanoAOD DQM and change name of mass-correlated tagger nodes
1ecd5da
save decommissioned taggers for older Nano eras and infer new PNet no…
71d3b02
also remove new PNet AK4 nodes for PUPPI jets eras 12_2/4
4ceeb26
remove extra space
525aadc
update DQM plots
61e61f4
don't run PNet AK4 on PUPPI for Run-2
0075526
fix DQM variables with ParticleNet
d524a88
remove duplicate variable
3c15acd
add missing commas
55daf9d
Merge tag 'CMSSW_13_1_X_2023-03-27-1100' of https://github.com/cms-sw…
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Why not adding the L1 and L2L3 here? They are used above: https://github.com/cms-sw/cmssw/blob/master/PhysicsTools/NanoAOD/python/jetsAK4_Puppi_cff.py#L12-L15
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 thought that we no longer had L1 corrections for PUPPI jets? I borrowed the same logic as used here in the miniAOD step: https://github.com/cms-sw/cmssw/blob/master/PhysicsTools/PatAlgos/python/slimming/applyDeepBtagging_cff.py#L68. I agree though that it's strange to have two different lists of JEC corrections. Maybe JME can clarify the procedure? @abenecke @anmalara
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.
True, L1 is not needed for Puppi and L2L3Res is only for data.
I'd also add a warning here. The current corrections won't be valid for the regressed jets, so why do we need them in the first place? I didn't check the code, but this should be valid for the mini step.
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.
Hi @anmalara, the idea was to make the ParticleNet regression correction relative to the raw jet pT available in the MINIAOD and NANOAOD even if you're right that the current JEC won't apply to the regressed jets. The idea was that we embed the ParticleNet outputs in the jet collections including the raw pT regression correction as something that can be studied to assess the performance gain from the resolution improvement even if one cannot use the regression as-is for an analysis for the time being. Then on the longer term we either integrate the regression as part of the JERC, as you know well, or we deploy an updated training with the regression relative to the JEC-corrected pT.
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.
This makes sense to me. I wanted to make clear (this information should be propagated carefully in twikis and it will) that the pt-regression and the pt-corrected cannot be mixed and if one wants to use the regression, one would need to reapply JEC on the fly.
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.
Since as you know the ParticleNet regression from the raw pT effectively approximates multiple aspects of the JEC corrections, I guess that if using the regression there is no good prescription for how to integrate that with the JEC other than having it as an input prior to the JERC derivation. I totally agree that we should make sure this is clear to the user - just saying that I don't think there is really much an on-the-fly solution available other than deploying a regression trained from the JEC-corrected pT. Since for now the priority is to provide the inputs for the POGs we figured this would be okay for the short term.
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.
Thanks @anmalara , perhaps the L1FastJet could be removed here then? https://github.com/cms-sw/cmssw/blob/master/PhysicsTools/NanoAOD/python/jetsAK4_Puppi_cff.py#L12 and here: https://github.com/cms-sw/cmssw/blob/master/PhysicsTools/NanoAOD/python/jetsAK8_cff.py#L11
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.
do you have a specific reason to remove this correction? The JERC group would make sure that if L1 is not needed for Puppi, a dummy file is created. Should we decide that we need L1 for Puppi, we should revert these changes. Maybe, it's safer to include this correction even in this PR.
Thoughts?
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.
No that's fine then, if JERC would rather keep the dummy L1, we can leave it in, it's up to you.