-
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
Bug fix in HiPuRhoProducer.cc #36489
Conversation
One-line bug fix in the jetty tower exclusion part of the code. The bug uses ieta instead of N(ieta) to calculate the number of towers.
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-36489/27362
|
A new Pull Request was created by @FHead (Yi Chen) for master. It involves the following packages:
@jpata, @cmsbuild, @clacaputo, @slava77 can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
please test |
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-52a865/21262/summary.html Comparison SummarySummary:
|
@FHead |
@@ -464,7 +464,7 @@ void HiPuRhoProducer::calculateOrphanInput(std::vector<fastjet::PseudoJet>& orph | |||
for (auto const& im : towermap_) { | |||
double dr2 = reco::deltaR2(im.eta, im.phi, jet_etaphi.first, jet_etaphi.second); | |||
if (dr2 < radiusPU_ * radiusPU_ && !excludedTowers[std::pair(im.ieta, im.iphi)] && | |||
(im.ieta - ntowersWithJets_[im.ieta]) > minimumTowersFraction_ * im.ieta) { | |||
(geomtowers_[im.ieta] - ntowersWithJets_[im.ieta]) > minimumTowersFraction_ * geomtowers_[im.ieta]) { | |||
ntowersWithJets_[im.ieta]++; |
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.
perhaps a problem here can become a good reason to consider to refactor the common code here and in RecoHI/HiJetAlgos/src/MultipleAlgoIterator.cc to be reused
type bug-fix |
+reconstruction
|
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. @perrotta, @dpiparo, @qliphy (and backports should be raised in the release meeting by the corresponding L2) |
+1 |
Hi @slava77 Yes, we are currently reclustering the jets when processing miniAOD as a workaround. I templated PFTowers to use packedPFCandates in order to run different radius jets, but due to this bug we're forced to do it for the standard R value as well. We plan a miniAOD reprocessing in mid-2022 such that this CPU-hungry reclustering at analysis level will no longer be needed afterwards. |
This is to prepare the backport to CMSSW 11_2_X of this PR cms-sw#36489
PR description:
One-line bug fix in the jetty tower exclusion part of the code. The bug uses ieta instead of N(ieta) to calculate the number of towers.
Bug report in HIN PAG meeting: https://indico.cern.ch/event/1092697/#12-bug-report-in-cs-jet
PR validation:
The fix is checked to produce desired jet subtraction outcome.
if this PR is a backport please specify the original PR and why you need to backport that PR:
This is the initial. Not a backport.
additional info
Before submitting your pull requests, make sure you followed this checklist: