-
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
Run iii electron cut based #39839
Run iii electron cut based #39839
Conversation
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-39839/32720
|
A new Pull Request was created by @rgoldouz (Reza Goldouzian) 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 |
so this PR already contain #39759, then I guess that one can be closed and this one can be reviewed? is that fine? |
Yes, please do. |
Hello Experts, I just made a new commit where I have modified the address of the effective area file to use the one for Run3. I think my latest commit Will be merged. Right? Thanks, |
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-39839/32722
|
Pull request #39839 was updated. @cmsbuild, @mandrenguyen, @clacaputo, @swertz, @vlimant can you please check and sign again. |
@@ -132,8 +144,8 @@ def _get_bitmapVIDForEle_docstring(modules,WorkingPoints): | |||
relative = cms.bool(False), | |||
rho_MiniIso = cms.InputTag("fixedGridRhoFastjetAll"), | |||
rho_PFIso = cms.InputTag("fixedGridRhoFastjetAll"), | |||
EAFile_MiniIso = cms.FileInPath("RecoEgamma/ElectronIdentification/data/Fall17/effAreaElectrons_cone03_pfNeuHadronsAndPhotons_94X.txt"), | |||
EAFile_PFIso = cms.FileInPath("RecoEgamma/ElectronIdentification/data/Fall17/effAreaElectrons_cone03_pfNeuHadronsAndPhotons_94X.txt"), | |||
EAFile_MiniIso = cms.FileInPath("RecoEgamma/ElectronIdentification/data/Run3_Winter22/effAreaElectrons_cone03_pfNeuHadronsAndPhotons_122X.txt"), |
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.
Don't we want to keep the Fall17 constants for Run2 samples?
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.
yes, @rgoldouz can you fix it please? It's better to not delete anything related to Run2 IDs, but just add things related to Run3 IDs.
type egamma |
Hello @swertz , Keeping fall17 files needs more careful considerations. I am also applying another change. We would like to make run3 ID as the default cut based for run3. We also want not to add run3 ID when the code is running on Run2 datasets. These era modifications have made the code dirty and difficult to modify/debug. Do you have any plan to clean the code? I can also help if needed. Please let us know your opinion, |
enable nano |
please test |
please abort |
please test |
-1 Failed Tests: RelVals-INPUT RelVals-NANO RelVals-INPUT
Expand to see more relval errors ...RelVals-NANO
Expand to see more relval errors ...Comparison SummarySummary:
|
@rgoldouz tests on 80X and 94X samples fail because the new ID is not added here:
However, support for these old inputs is being dropped in #39796, so once that is merged you will be able to rebase your PR without having to worry about 80X and 94X. In the meanwhile you could take care of the pending item about isolation/EA? EDIT: #39796 has been merged, so you'll need to rebase... |
PR description:
Dear Experts,
Based on the the following request for adding Run3 electron cut based IDs,
#39759
I have added the run3 IDs to the nanoAOD. We would like to keep the Run2 IDs. Therefore, I have added a new branch for Run3 ID that is called "cutBasedRunIIIWinter22". So we have two branches for the electron cut based IDs, "cutBased" branch which is for Run2 IDs and "cutBasedRunIIIWinter22" which for run3 IDs. I have also added the corresponding bitmap branch.
Please let me know your comments.
Thanks,
Reza
@swagata87 @a-kapoor @DebabrataBhowmik