-
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
Switch to heavy-ion jets in HI miniAOD #30898
Conversation
The code-checks are being triggered in jenkins. |
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-30898/17291
|
A new Pull Request was created by @mandrenguyen (Matthew Nguyen) for master. It involves the following packages: PhysicsTools/PatAlgos @perrotta, @jpata, @cmsbuild, @santocch, @slava77 can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
@cmsbuild please test |
The tests are being triggered in jenkins.
|
+1 |
Comparison job queued. |
Comparison is ready Comparison Summary:
|
Comparison is ready Comparison Summary:
|
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 was looking at the results in wf 140.5611. Most of the changes are as expected, however there are still some top level oddities or points of concern
- slimmedJets_tagInfos take up 25% of the file size ( increases the total file size by 18%) ; are they really worth it? Note that the cost in less busy events of wf 158.01 is only 4% of the disk size. Some preselection may be useful, likely for a follow up PR.
- puppi is still running ; see the comment inline for patPhotons
addPuppiIsolation
- isolatedTracks module is a bit heavy (9% CPU, second slowest module, after akCs4PFJets); with only 1% disk use though. Note that in 158.01 it's pretty low on the list (0.4% of CPU). So, it's probably more in the land of general CPU optimization with a bit more interest/motivation for HI needs.
Additionally, in wf 158.0 the pfJetsEI are now filled (these run but are not written in AOD). The reason is in the ak4PFJets (see a comment for that).
I think that most of these if not more can be followed up in #31647
@@ -120,20 +124,25 @@ def miniAOD_customizeCommon(process): | |||
from Configuration.Eras.Modifier_phase2_muon_cff import phase2_muon |
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.
around line 93 I suggest to add
_hiGeneral.toModify(process.patPhotons, addPuppiIsolation = False)
it looks like the only reason why "puppi" module is still running
pp_on_AA_2018.toModify(ak4PFJets,src = "pfNoPileUpJMEHI", inputEtMin = 9999) | ||
pp_on_AA_2018.toModify(ak4PFJetsCHS,src = "pfNoPileUpJMEHI", inputEtMin = 9999) | ||
from Configuration.Eras.Modifier_pp_on_PbPb_run3_cff import pp_on_PbPb_run3 | ||
(pp_on_AA_2018 | pp_on_PbPb_run3).toModify(ak4PFJets, src = "pfEmptyCollection") |
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.
(pp_on_AA_2018 | pp_on_PbPb_run3).toModify(ak4PFJets, src = "pfEmptyCollection") | |
(pp_on_AA_2018 | pp_on_PbPb_run3).toModify(ak4PFJets, src = "pfEmptyCollection", inputEtMin = 9999) |
I think that the threshold should still be applied; better yet, a more thorough cleanup is needed in CommonTools/ParticleFlow/python/EITopPAG_cff.py
+1
A few items that somewhat extend beyond the scope of just having the jets configured for miniAOD can be followed up in #31647 |
@silviodonato Can you merge this one, please? There are other PRs I need to make on top of it that are needed for the HI-miniAOD in 11_2_X. |
merge |
+1 |
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 be automatically merged. |
Apparently this PR broke workflows 140.0, 145.0, and 150.0 with
|
@makortel I tracked down the issue, which is a naming conflict with a simple fix. |
Yes, please make a new PR. |
This PR appears to also cause many HI workflows to crash (see #31670). I hope the fix will fix those as well. |
@makortel Ah, there's another issue affecting 158.1, but I can see the fix for that as well. |
PR description:
Replaces pp jets in HI miniAOD with HI jets, using the Cs UE subtraction algorithm (as updated in #30057 ).
PR validation:
Tested with workflows 158.01 and 140.5611
if this PR is a backport please specify the original PR and why you need to backport that PR:
Before submitting your pull requests, make sure you followed this checklist: