Switching default jet collection from AK5 to AK4 #2171

rappoccio · 2014-01-24T22:00:16Z

Here I have changed the default jet collection for downstream modules from AK5 to AK4. This affects :

b-tagging
tau-tagging
HLT
PFBRECO
Type1 MET
Isolation cones.

I have not self-ported changes of hard-coded cone sizes from 0.5 to 0.4, except in the Jet RECO packages. There are probably pieces in BTagging and TauTagging code that use cone sizes and must change for consistency.

… for posterity

cmsbuild · 2014-01-24T22:03:10Z

A new Pull Request was created by @rappoccio for CMSSW_7_1_X.

Switching default jet collection from AK5 to AK4

It involves the following packages:

CommonTools/ParticleFlow
CommonTools/RecoAlgos
Configuration/Skimming
DPGAnalysis/Skims
DQM/Physics
DQMOffline/Alignment
DQMOffline/JetMET
DQMOffline/PFTau
DQMOffline/RecoB
DQMOffline/Trigger
Documentation/DataFormats
ElectroWeakAnalysis/Skimming
FastSimulation/Configuration
HLTrigger/Configuration
HLTrigger/HLTanalyzers
HLTriggerOffline/Top
JetMETCorrections/Configuration
JetMETCorrections/Type1MET
PhysicsTools/JetMCAlgos
PhysicsTools/PatExamples
RecoBTag/ImpactParameter
RecoBTag/SoftLepton
RecoHI/HiJetAlgos
RecoJets/Configuration
RecoJets/JetAssociationProducers
RecoJets/JetPlusTracks
RecoMET/METFilters
RecoMET/METProducers
RecoMuon/MuonIsolationProducers
RecoTauTag/Configuration
RecoTauTag/RecoTau
RecoTauTag/TauTagTools
RecoTracker/SpecialSeedGenerators
SLHCUpgradeSimulations/Configuration
SUSYBSMAnalysis/Skimming
TopQuarkAnalysis/Configuration
Validation/RecoHI
Validation/RecoJets
Validation/RecoMET
Validation/RecoParticleFlow

The following packages do not have a category, yet:

SLHCUpgradeSimulations/Configuration

@deguio, @lveldere, @cmsdoxy, @cerminar, @Martin-Grunewald, @anton-a, @perrotta, @ojeda, @vlimant, @cmsbuild, @fwyzard, @ktf, @thspeer, @rovere, @giamman, @slava77, @vadler, @Degano, @fabiocos, @nclopezo, @danduggan, @franzoni can you please review it and eventually sign? Thanks.
@ghellwig, @TaiSakuma, @yslai, @kkrajczar, @GiacomoSguazzoni, @jazzitup, @gpetruc, @yenjie, @RylanC24, @rovere, @kurtejung, @ferencek, @cerati, @mandrenguyen, @richard-cms, @bachtis, @yetkinyilmaz this is something you requested to watch as well.
You can sign-off by replying to this message having '+1' in the first line of your reply.
You can reject by replying to this message having '-1' in the first line of your reply.
@ktf you are the release manager for this.
You can merge this pull request by typing 'merge' in the first line of your comment.

rappoccio · 2014-01-24T22:07:46Z

I should also mention that we're keeping AK5 for consistency, but nothing downstream is supposed to use it.

Martin-Grunewald · 2014-01-25T07:47:42Z

-1

TSG has not signed off on moving from ak5 to ak4 in the HLT.
In case it does sign off, the HLT changes have to go through ConfDB, no direct editing of the dumps like the above.

degrutto · 2014-01-27T10:28:04Z

Hi Martin, all

just to let you know that am confdb first trial version with the change ak5 --> ak4 is at

/users/degrutto/ak4/V6 (working in 700pre11)

Is has not been signed off by TSGm, it is true,
but a list we had a first discussion based on some studies I made based on that

https://indico.cern.ch/getFile.py/access?subContId=2&contribId=1&resId=0&materialId=slides&confId=290680

I'm afraid TSG policy to signed this off requires the validation of many POG and PAG related quantities..
and that will require some time...
Maybe we should discuss this at a higher level of management?
Please advice,

thanks

Michele

cmsbuild · 2014-01-27T10:35:41Z

-1
I ran the usual tests and I found errors in the following unit tests:

---> test runtestPhysicsToolsPatAlgos had ERRORS

you can see the results of the tests here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-2171/30/summary.html

Martin-Grunewald · 2014-01-27T10:36:25Z

Hi Michele,

Absolutely, for example in the TSG meetings, today and/or Thursday.
To proceed for now I suggest to take out the HLTrigger modifications
from this pull request. The HLT menu update can then come as a
separate pull request later.

Best regards

Martin

cmsbuild · 2014-01-27T12:24:52Z

-1
I ran the usual tests and I found errors in the following unit tests:

---> test runtestPhysicsToolsPatAlgos had ERRORS

you can see the results of the tests here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-2171/45/summary.html

rappoccio · 2014-01-27T14:34:31Z

With respect to runtestPhysicsToolsPatAlgos, this is expected because this collection is not present in the RelVal that this is pointing to, but will be in the future. @vadler can confirm.

vadler · 2014-01-28T13:13:48Z

-1
The transition looks incomplete.
PAT IB tests still fail partially, when using new RelVals as input (25.0/step3.root in this case).

rappoccio · 2014-01-28T16:39:06Z

Apologies, it looks like my script missed a group of files. I'm testing a fix now.

vadler · 2014-01-28T16:42:15Z

BTW: Also some short-matrix work flows break: 8.0, 4.22

rappoccio · 2014-01-28T18:44:31Z

Note : The PatAlgos test will still break due to the missing file but I checked with the "matrix" file 25.0/step3.root and it works.

cmsbuild · 2014-01-28T18:48:12Z

Pull request #2171 was updated. @deguio, @lveldere, @cmsdoxy, @cerminar, @Martin-Grunewald, @anton-a, @perrotta, @ojeda, @vlimant, @cmsbuild, @fwyzard, @ktf, @thspeer, @rovere, @giamman, @slava77, @vadler, @Degano, @fabiocos, @nclopezo, @danduggan, @franzoni can you please check and sign again.

vadler · 2014-01-28T19:37:33Z

I see merge conflicts in CMSSW_7_1_X_2014-01-28-1400:

error: addinfo_cache failed for path 'RecoTauTag/ImpactParameter/plugins/ImpactParameter.cc'
Auto-merging RecoTauTag/RecoTau/python/RecoTauPiZeroProducer_cfi.py
CONFLICT (content): Merge conflict in RecoTauTag/RecoTau/python/RecoTauPiZeroProducer_cfi.py
Auto-merging RecoTauTag/RecoTau/python/RecoTauJetRegionProducer_cfi.py
CONFLICT (content): Merge conflict in RecoTauTag/RecoTau/python/RecoTauJetRegionProducer_cfi.py
Auto-merging RecoTauTag/RecoTau/python/RecoTauCombinatoricProducer_cfi.py
CONFLICT (content): Merge conflict in RecoTauTag/RecoTau/python/RecoTauCombinatoricProducer_cfi.py
Auto-merging RecoTauTag/Configuration/python/RecoTauTag_EventContent_cff.py
Auto-merging RecoTauTag/Configuration/python/RecoPFTauTag_cff.py
CONFLICT (content): Merge conflict in RecoTauTag/Configuration/python/RecoPFTauTag_cff.py
Auto-merging PhysicsTools/PatAlgos/python/tools/pfTools.py
Auto-merging DQM/Physics/src/SingleTopTChannelLeptonDQM.cc
CONFLICT (content): Merge conflict in DQM/Physics/src/SingleTopTChannelLeptonDQM.cc
Auto-merging DQM/Physics/python/singleTopDQM_cfi.py
CONFLICT (content): Merge conflict in DQM/Physics/python/singleTopDQM_cfi.py
Auto-merging CommonTools/ParticleFlow/python/pfTaus_cff.py
CONFLICT (content): Merge conflict in CommonTools/ParticleFlow/python/pfTaus_cff.py
Automatic merge failed; fix conflicts and then commit the result.
Unable to merge branch refs/pull/2171/head from repository cms-sw.

Martin-Grunewald · 2014-01-29T05:54:55Z

-1

Changes in the HLT (HLTrigger package and dependent subsystems) must go via ConfDB. Please remove HLT from this PR.

cmsbuild · 2014-01-30T08:39:33Z

+1
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-2171/46/summary.html

vadler · 2014-01-30T10:53:42Z

-1
@cmsbuild : How did you get the tests running? I still see merge conflicts (CMSSW_7_1_X_2014-01-29-1400 now).

ktf · 2014-01-30T11:01:21Z

There is an issue in the script which tests pull requests. I've fixed it
and I'll follow it up with @nclopezo...

On 30 Jan 2014, at 11:53, Volker Adler wrote:

-1
@cmsbuild : How did you get the tests running? I still see merge
conflicts (CMSSW_7_1_X_2014-01-29-1400 now).

Reply to this email directly or view it on GitHub:
#2171 (comment)

perrotta · 2014-02-03T08:21:42Z

CommonTools/ParticleFlow/python/Tools/jetTools.py

    # print 'PF2PAT: selecting jet algorithm ', algo
-
+
    if algo == 'IC5':
 #allPfJets = RecoJets.JetProducers.ic5PFJets_cfi.iterativeCone5PFJets.clone()
        jetAlgo = RecoJets.JetProducers.ic5PFJets_cfi.iterativeCone5PFJets.clone()
    elif algo == 'AK5':


Sal: shouldn't this require an additional
elif algo == 'AK4'
?
Using an ak4 jet producer when algo is 'AK5' is quite prone to mistakes and misunderstandings...

rappoccio · 2014-02-03T16:38:55Z

Hi, Folks,

@perrotta : Yes, I will commit a fix for that now.

@vadler and @ktf : what is the situation with the integration build scripts? Maybe for now can you just tell me what pull request number those changes correspond to and I can fix them?

@Martin-Grunewald : I'm following the JIRA thread here :

https://its.cern.ch/jira/browse/CMSHLT-47

What should I do here? Just remove the HLT configurations or wait until the CondDB is sorted out?

cmsbuild · 2014-02-03T16:43:55Z

Pull request #2171 was updated. @deguio, @lveldere, @cmsdoxy, @Martin-Grunewald, @anton-a, @perrotta, @ojeda, @vlimant, @cmsbuild, @fwyzard, @ktf, @davidlange6, @thspeer, @rovere, @giamman, @slava77, @vadler, @Degano, @nclopezo, @danduggan, @monttj, @franzoni can you please check and sign again.

vadler · 2014-02-03T18:58:41Z

-1
Merge conflict persists (CMSSW_7_1_X_2014-02-03-0200).

rappoccio · 2014-02-03T21:22:51Z

Hi, Volker, All,

Sorry, the last commit was not supposed to fix the merge conflict. I managed to do that now. Please have a look.

Cheers,
Sal

cmsbuild · 2014-02-03T21:28:33Z

Pull request #2171 was updated. @diguida, @deguio, @lveldere, @cmsdoxy, @Martin-Grunewald, @anton-a, @thspeer, @rcastello, @bendavid, @perrotta, @civanch, @ojeda, @vlimant, @cmsbuild, @fwyzard, @ktf, @davidlange6, @vciulli, @Dr15Jones, @mdhildreth, @rovere, @giamman, @slava77, @ggovi, @vadler, @Degano, @mulhearn, @apfeiffer1, @nclopezo, @danduggan, @monttj, @franzoni can you please check and sign again.

perrotta · 2014-02-03T22:47:05Z

rappoccio notifications@github.com ha scritto:

Hi, Folks,

@perrotta : Yes, I will commit a fix for that now.

Thank you Sal.
I think there were a few more cases as such. (For instance, I guess
that the IterativeCone5 algorithms changed their parameters without
being renamed accordingly: as soon as I find some I will tell you, but
it is not easy doing it now because after the rebase the whole CMSSW
got included in this pull request...)

@vadler and @ktf : what is the situation with the integration build
scripts? Maybe for now can you just tell me what pull request number
those changes correspond to and I can fix them?

@Martin-Grunewald : I'm following the JIRA thread here :

https://its.cern.ch/jira/browse/CMSHLT-47

What should I do here? Just remove the HLT configurations or wait
until the CondDB is sorted out?

Yes, please, remove all HLT configurations for now.
We can provide you the commits with the correct configurations as soon
as your PR stabilizes.

Andrea

Reply to this email directly or view it on GitHub:
#2171 (comment)

This message was sent using IMP, the Internet Messaging Program.

Dr15Jones · 2014-03-13T16:47:28Z

The system still says 'tests-rejected'

rappoccio · 2014-03-13T16:48:25Z

Hi, Chris,

Those are the standard PAT tests that are expected to fail due to the missing product in the existing RelVal files. When run on a "matrix" file 25.0/step3.root it works.

Cheer,s
Sal

Dr15Jones · 2014-03-13T16:52:19Z

+1

apfeiffer1 · 2014-03-13T19:17:37Z

+1

On Thu, Mar 13, 2014 at 5:52 PM, Chris Jones notifications@github.comwrote:

+1

Reply to this email directly or view it on GitHubhttps://github.com//pull/2171#issuecomment-37556862
.

Thanks,
cheers, andreas

davidlange6 · 2014-03-16T20:01:30Z

@ktf - this one is at last ready for a special build = CMSSW_7_1_0_pre4 + this pull request

ktf · 2014-03-17T13:13:13Z

The code in question seems to heavily include developments which came after pre4, am I wrong? Wouldn't it make more sense to test this with pre5 rather than me undoing those changes?

rappoccio · 2014-03-17T13:15:02Z

@ktf This is exactly what we said about pre3 (to postpone until pre4). There are no developments aside from rebasing and again. This will never converge unless we just put it in.

ktf · 2014-03-17T13:20:58Z

My bad. Things are actually ok. Sorry for the noise. @nclopezo is
building it.

On 17 Mar 2014, at 14:15, rappoccio wrote:

@ktf This is exactly what we said about pre3 (to postpone until
pre4). There are no developments aside from rebasing and again. This
will never converge unless we just put it in.

Reply to this email directly or view it on GitHub:
#2171 (comment)

nclopezo · 2014-03-18T09:20:57Z

Hi All,

CMSSW_7_1_0_pre4_AK4 is now available. it is CMSSW_7_1_0_pre4 + this pull request

rappoccio · 2014-03-18T13:03:34Z

Phew! Thanks!

davidlange6 · 2014-03-18T13:08:09Z

Hi David
Please announce the release on hyper news

Thanks!

On Mar 18, 2014, at 10:20 AM, David Mendez notifications@github.com
wrote:

Hi All,

CMSSW_7_1_0_pre4_AK4 is now available. it is CMSSW_7_1_0_pre4 + this pull request

—
Reply to this email directly or view it on GitHub.

davidlange6 · 2014-03-18T13:14:50Z

Closing…(happily)

nclopezo · 2014-03-18T13:39:06Z

I announced it here:

https://hypernews.cern.ch/HyperNews/CMS/get/relAnnounce/1399.html

Merge branch 'rappoccio_AK5_To_AK4_710' of https://github.com/rappoccio/cmssw into HEAD

Update python to 2.7.11

Switching defaults from AK5 to AK4 everywhere in 710pre1. Keeping AK5…

b5bc705

… for posterity

My script forgot a bunch of files for the AK5->AK4 transition

051bd65

perrotta reviewed Feb 3, 2014
View reviewed changes

Fixing a stray AK5 to AK4 conversion

feaf612

Updating to latest IB

9df1a51

perrotta mentioned this pull request Feb 4, 2014

Replace SeedingLayersESProducer with an EDProducer #2286

Closed

rappoccio added analysis-approved labels Mar 13, 2014

davidlange6 closed this Mar 18, 2014

jpavel pushed a commit to jpavel/cmssw that referenced this pull request Apr 16, 2014

This comes from pull reqest cms-sw#2171

0f423e8

Merge branch 'rappoccio_AK5_To_AK4_710' of https://github.com/rappoccio/cmssw into HEAD

slava77 mentioned this pull request May 7, 2014

ak5 to ak4 transition #3631

Merged

rappoccio deleted the rappoccio_AK5_To_AK4_710 branch November 3, 2014 21:11

ggovi pushed a commit to ggovi/cmssw that referenced this pull request Jan 11, 2017

Merge pull request cms-sw#2171 from degano/python2711_81X

c5ecf9b

Update python to 2.7.11

Switching default jet collection from AK5 to AK4 #2171

Switching default jet collection from AK5 to AK4 #2171

Conversation

rappoccio commented Jan 24, 2014

cmsbuild commented Jan 24, 2014

rappoccio commented Jan 24, 2014

Martin-Grunewald commented Jan 25, 2014

degrutto commented Jan 27, 2014

cmsbuild commented Jan 27, 2014

Martin-Grunewald commented Jan 27, 2014

cmsbuild commented Jan 27, 2014

rappoccio commented Jan 27, 2014

vadler commented Jan 28, 2014

rappoccio commented Jan 28, 2014

vadler commented Jan 28, 2014

rappoccio commented Jan 28, 2014

cmsbuild commented Jan 28, 2014

vadler commented Jan 28, 2014

Martin-Grunewald commented Jan 29, 2014

cmsbuild commented Jan 30, 2014

vadler commented Jan 30, 2014

ktf commented Jan 30, 2014

perrotta Feb 3, 2014

Choose a reason for hiding this comment

rappoccio commented Feb 3, 2014

cmsbuild commented Feb 3, 2014

vadler commented Feb 3, 2014

rappoccio commented Feb 3, 2014

cmsbuild commented Feb 3, 2014

perrotta commented Feb 3, 2014

Dr15Jones commented Mar 13, 2014

rappoccio commented Mar 13, 2014

Dr15Jones commented Mar 13, 2014

apfeiffer1 commented Mar 13, 2014

davidlange6 commented Mar 16, 2014

ktf commented Mar 17, 2014

rappoccio commented Mar 17, 2014

ktf commented Mar 17, 2014

nclopezo commented Mar 18, 2014

rappoccio commented Mar 18, 2014

davidlange6 commented Mar 18, 2014

davidlange6 commented Mar 18, 2014

nclopezo commented Mar 18, 2014