drop type specs in RecoHI {HiEgammaAlgos,HiJetAlgos,HiMuonAlgos} #32031

jeongeun · 2020-11-05T04:00:34Z

PR description:

Update the safer syntax for existing parameter :

drop type specifications where the original parameter exists.
move all parameter inside the clone

Instead of modifying parameters with full type specs, which can be interpreted as an insertion of a new parameter, it is a safer way to protect from parameter name mistakes and will also help in possible parameter migrations.
(The previous PR were PR#31162,PR#31243,PR#31332, PR#31389, PR#31523, PR#31538, PR#31748, PR#31784, PR#31881)

In this PR, total 17 files changed.

RecoHI/HiEgammaAlgos : 2 files
RecoHI/HiJetAlgos : 6 files
RecoHI/HiMuonAlgos : 9 files

PR validation:

Event Content comparison check was also done and there is no change with these updates.
Tested in CMSSW_11_2_X, the basic test all passed in the CMSSW PR instructions.

cmsbuild · 2020-11-05T04:01:03Z

The code-checks are being triggered in jenkins.

cmsbuild · 2020-11-05T04:08:20Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-32031/19580

This PR adds an extra 28KB to repository

cmsbuild · 2020-11-05T04:08:50Z

A new Pull Request was created by @jeongeun (JeongEun Lee) for master.

It involves the following packages:

RecoHI/HiEgammaAlgos
RecoHI/HiJetAlgos
RecoHI/HiMuonAlgos

@perrotta, @jpata, @cmsbuild, @slava77 can you please review it and eventually sign? Thanks.
@yslai, @MiheeJo, @jazzitup, @yenjie, @kurtejung, @mandrenguyen, @dgulhan, @yetkinyilmaz this is something you requested to watch as well.
@silviodonato, @dpiparo, @qliphy you are the release manager for this.

cms-bot commands are listed here

slava77 · 2020-11-05T05:09:11Z

@cmsbuild please test

cmsbuild · 2020-11-05T05:09:35Z

The tests are being triggered in jenkins.

CMSSW_11_2_X_2020-11-04-2300/slc7_amd64_gcc820: https://cmssdt.cern.ch/jenkins/job/ib-run-pr-tests/10519/console Started: 2020/11/05 08:55

cmsbuild · 2020-11-05T09:27:12Z

+1
Tested at: 0e1298a
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-1839eb/10519/summary.html
CMSSW: CMSSW_11_2_X_2020-11-04-2300
SCRAM_ARCH: slc7_amd64_gcc820

cmsbuild · 2020-11-05T09:27:15Z

Comparison job queued.

perrotta

Besides what listed inline:

missing etaRanges in the modifier within RecoHI/HiJetAlgos/python/hiFJRhoProducer.py

perrotta · 2020-11-05T08:26:13Z

RecoHI/HiJetAlgos/python/HiPFJetParameters_cff.py

-    jetPtMin = 10,
-    Ghost_EtaMax = 6.5,
+    jetPtMin       = 10,
+    Ghost_EtaMax   = 6.5,
    # this parameter is missing from PFJetParameters for some reason


The parameter is not missing, in fact (and you can modify it right below): therefore this comment line can be misleading. Please remove

perrotta · 2020-11-05T08:36:57Z

RecoHI/HiJetAlgos/python/HiRecoJets_cff.py

@@ -65,6 +65,7 @@
    rParam       = cms.double(0.5)
    )
 akPu5CaloJets.radiusPU = 0.5
+akPu5CaloJets.puPtMin  = cms.double(10)

 akPu7CaloJets = cms.EDProducer(


@mandrenguyen please notice that akPu7CaloJets is defined twice in this config, and the two configurations differ for the value of the parametes radiusPU. Please provide advise to @jeongeun about how to fix it.
I would remove the explicit definition of akPu7CaloJets in this L70 and stick on the one cloned from akPu5CaloJets right below: but then one has to decide which value of radiusPU. one has to keep

perrotta · 2020-11-05T08:50:37Z

RecoHI/HiJetAlgos/python/HiRecoJets_cff.py

@@ -65,6 +65,7 @@
    rParam       = cms.double(0.5)
    )
 akPu5CaloJets.radiusPU = 0.5
+akPu5CaloJets.puPtMin  = cms.double(10)


drop type spec

perrotta · 2020-11-05T08:54:04Z

RecoHI/HiJetAlgos/python/HiRecoPFJets_cff.py

+    subtractorName = "MultipleAlgoIterator",
+    src            = 'PFTowers',
+    doAreaFastjet  = False,
+    puPtMin        = cms.double(25)


drop type spec

@perrotta I got TypeError when I drop it. TypeError: puPtMin does not already exist, so it can only be set to a CMS python configuration type

perrotta · 2020-11-05T08:58:33Z

RecoHI/HiJetAlgos/python/HiRecoPFJets_cff.py

-   radiusPU = cms.double(0.5),
-   src = "hiPFCandCleanerforJets",
+   jetPtMin     = 15.0,
+   nSigmaPU     = cms.double(1.0),


The parameters nSigmaPU and radiusPU are commented out in the PFJetParameters defined in RecoJets/JetProducers/python/PFJetParameters_cfi.py because if doPUOffsetCorr "is false, these are not read".

I think that a cleaner solution would be to uncomment those two parameter in RecoJets/JetProducers/python/PFJetParameters_cfi.py (which are not used there, anyhow) so that you can drop type spec here. (Please notice that those two parameters exist anyhow even if not explicitly written in the configuration, because they are defined in the fillDescriptions method of VirtualJetProducer, with the same numerical values defined in the lines commented out in PFJetParameters_cfi.py)

perrotta · 2020-11-05T09:28:15Z

RecoHI/HiMuonAlgos/python/HiRegitMuonMixedTripletStep_cff.py

-    overrideTrkQuals      = cms.InputTag('hiRegitMuPixelPairStepSelector','hiRegitMuPixelPairStep'),
-    trackClassifier       = cms.InputTag(''),
-    TrackQuality          = cms.string('tight')
+    #oldClusterRemovalInfo = cms.InputTag("hiRegitMuPixelPairStepClusters"),


please remove commented out duplicate lines

perrotta · 2020-11-05T09:30:00Z

RecoHI/HiMuonAlgos/python/HiRegitMuonMixedTripletStep_cff.py


 # fitting: feed new-names
-hiRegitMuMixedTripletStepTracks                 = RecoTracker.IterativeTracking.MixedTripletStep_cff.mixedTripletStepTracks.clone(
+hiRegitMuMixedTripletStepTracks  = RecoTracker.IterativeTracking.MixedTripletStep_cff.mixedTripletStepTracks.clone(
    AlgorithmName = cms.string('hiRegitMuMixedTripletStep'),


drop type spec

cmsbuild · 2020-11-05T16:39:26Z

The code-checks are being triggered in jenkins.

cmsbuild · 2020-11-05T16:45:36Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-32031/19591

This PR adds an extra 32KB to repository

cmsbuild · 2020-11-06T09:55:49Z

Pull request #32031 was updated. @perrotta, @jpata, @cmsbuild, @slava77 can you please check and sign again.

perrotta · 2020-11-06T09:57:32Z

please test

cmsbuild · 2020-11-06T09:58:00Z

The tests are being triggered in jenkins.

CMSSW_11_2_X_2020-11-05-2300/slc7_amd64_gcc820: https://cmssdt.cern.ch/jenkins/job/ib-run-pr-tests/10544/console Started: 2020/11/06 11:00

cmsbuild · 2020-11-06T11:27:49Z

+1
Tested at: 3a46e5e
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-1839eb/10544/summary.html
CMSSW: CMSSW_11_2_X_2020-11-05-2300
SCRAM_ARCH: slc7_amd64_gcc820

cmsbuild · 2020-11-06T11:27:50Z

Comparison job queued.

cmsbuild · 2020-11-06T15:33:58Z

Comparison is ready
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-1839eb/10544/summary.html

Comparison Summary:

No significant changes to the logs found
Reco comparison results: 4 differences found in the comparisons
DQMHistoTests: Total files compared: 35
DQMHistoTests: Total histograms compared: 2544144
DQMHistoTests: Total failures: 9
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 2544113
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 34 files compared)
Checked 149 log files, 22 edm output root files, 35 DQM output files

perrotta · 2020-11-06T15:51:07Z

+1

Types dropped from clomed or modified configs as planned
Jenkins tests pass

cmsbuild · 2020-11-06T15:51:36Z

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. @silviodonato, @dpiparo, @qliphy (and backports should be raised in the release meeting by the corresponding L2)

silviodonato · 2020-11-06T19:48:30Z

RecoHI/HiJetAlgos/python/HiRecoPFJets_cff.py

-akCs4PFJets.jetPtMin      = cms.double(0.0)
+akCs4PFJets.src               = 'particleFlow'
+akCs4PFJets.doAreaFastjet     = True
+akCs4PFJets.jetPtMin          = 0.0
 akCs4PFJets.useExplicitGhosts = cms.bool(True)


have you tried akCs4PFJets.useExplicitGhosts = True?

have you tried akCs4PFJets.useExplicitGhosts = True?

The useExplicitGhosts parameter is missing in the akCs4PFJets config, contrarily to the other ones there which can therefore get modified without defining the type

silviodonato · 2020-11-06T20:15:01Z

+1

drop type specs in RecoHI {HiEgammaAlgos,HiJetAlgos,HiMuonAlgos}

0e1298a

cmsbuild added this to the CMSSW_11_2_X milestone Nov 5, 2020

cmsbuild added code-checks-pending comparison-pending orp-pending pending-signatures reconstruction-pending tests-pending labels Nov 5, 2020

cmsbuild added code-checks-approved and removed code-checks-pending labels Nov 5, 2020

cmsbuild added tests-started and removed tests-pending labels Nov 5, 2020

cmsbuild added tests-approved and removed tests-started labels Nov 5, 2020

perrotta reviewed Nov 5, 2020

View reviewed changes

drop type specs

b106a87

cmsbuild added code-checks-pending tests-pending and removed code-checks-approved tests-approved labels Nov 5, 2020

cmsbuild removed the code-checks-pending label Nov 5, 2020

cmsbuild added code-checks-approved and removed code-checks-pending labels Nov 6, 2020

cmsbuild added tests-started and removed tests-pending labels Nov 6, 2020

cmsbuild added tests-approved and removed tests-started labels Nov 6, 2020

cmsbuild added comparison-available and removed comparison-pending labels Nov 6, 2020

cmsbuild added fully-signed reconstruction-approved and removed pending-signatures reconstruction-pending labels Nov 6, 2020

silviodonato reviewed Nov 6, 2020

View reviewed changes

cmsbuild added orp-approved and removed orp-pending labels Nov 6, 2020

cmsbuild merged commit c707e88 into cms-sw:master Nov 6, 2020

jeongeun mentioned this pull request Nov 18, 2020

drop type specs in RecoHI {Configuration, HiTracking} #32171

Closed

This was referenced Dec 4, 2020

Drop type specs in RecoHI {Configuration, HiTracking} #32386

Merged

drop type spec in TrackingTools RecoMuon #32396

Merged

jeongeun mentioned this pull request Dec 14, 2020

drop type spec in CommonTools #32464

Merged

jeongeun mentioned this pull request Mar 3, 2021

drop type specs and move parameters inside the modifier in RecoHI #33048

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

drop type specs in RecoHI {HiEgammaAlgos,HiJetAlgos,HiMuonAlgos} #32031

drop type specs in RecoHI {HiEgammaAlgos,HiJetAlgos,HiMuonAlgos} #32031

jeongeun commented Nov 5, 2020

cmsbuild commented Nov 5, 2020

cmsbuild commented Nov 5, 2020

cmsbuild commented Nov 5, 2020

slava77 commented Nov 5, 2020

cmsbuild commented Nov 5, 2020 •

edited

cmsbuild commented Nov 5, 2020

cmsbuild commented Nov 5, 2020

perrotta left a comment

perrotta Nov 5, 2020

perrotta Nov 5, 2020

perrotta Nov 5, 2020

perrotta Nov 5, 2020

jeongeun Nov 5, 2020

perrotta Nov 5, 2020 •

edited

perrotta Nov 5, 2020

perrotta Nov 5, 2020

cmsbuild commented Nov 5, 2020

cmsbuild commented Nov 5, 2020

cmsbuild commented Nov 6, 2020

perrotta commented Nov 6, 2020

cmsbuild commented Nov 6, 2020 •

edited

cmsbuild commented Nov 6, 2020

cmsbuild commented Nov 6, 2020

cmsbuild commented Nov 6, 2020

perrotta commented Nov 6, 2020

cmsbuild commented Nov 6, 2020

silviodonato Nov 6, 2020

perrotta Nov 6, 2020

silviodonato commented Nov 6, 2020

drop type specs in RecoHI {HiEgammaAlgos,HiJetAlgos,HiMuonAlgos} #32031

drop type specs in RecoHI {HiEgammaAlgos,HiJetAlgos,HiMuonAlgos} #32031

Conversation

jeongeun commented Nov 5, 2020

PR description:

PR validation:

cmsbuild commented Nov 5, 2020

cmsbuild commented Nov 5, 2020

cmsbuild commented Nov 5, 2020

slava77 commented Nov 5, 2020

cmsbuild commented Nov 5, 2020 • edited

cmsbuild commented Nov 5, 2020

cmsbuild commented Nov 5, 2020

perrotta left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

perrotta Nov 5, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cmsbuild commented Nov 5, 2020

cmsbuild commented Nov 5, 2020

cmsbuild commented Nov 6, 2020

perrotta commented Nov 6, 2020

cmsbuild commented Nov 6, 2020 • edited

cmsbuild commented Nov 6, 2020

cmsbuild commented Nov 6, 2020

cmsbuild commented Nov 6, 2020

perrotta commented Nov 6, 2020

cmsbuild commented Nov 6, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

silviodonato commented Nov 6, 2020

cmsbuild commented Nov 5, 2020 •

edited

perrotta Nov 5, 2020 •

edited

cmsbuild commented Nov 6, 2020 •

edited