2018_pp_on_AA era HI jet sequences #24380

stepobr · 2018-08-23T15:41:11Z

This pull request is for having a proper set of heavy-ion specific jet algorithms within pp_on_AA_2018 era. Also,definition of kt4PFJetsForRho is removed from RecoJets/Configuration/RecoJetsGlobal and now kept in the RecoHI area.

Compared to the previous attempt:
#24349
This PR passes both 150 and 158 workflow tests and avoid any duplication.

@mverwe @mandrenguyen @cfmcginn @FHead

cmsbuild · 2018-08-23T15:41:33Z

The code-checks are being triggered in jenkins.

cmsbuild · 2018-08-23T15:43:39Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-24380/6145

cmsbuild · 2018-08-23T15:43:58Z

A new Pull Request was created by @stepobr for master.

It involves the following packages:

RecoHI/HiJetAlgos
RecoJets/Configuration

@perrotta, @cmsbuild, @slava77 can you please review it and eventually sign? Thanks.
@TaiSakuma, @jdolen, @ahinzmann, @schoef, @rappoccio, @jazzitup, @jdamgov, @yslai, @nhanvtran, @yenjie, @gkasieczka, @clelange, @kurtejung, @mariadalfonso, @mandrenguyen, @dgulhan, @seemasharmafnal, @yetkinyilmaz this is something you requested to watch as well.
@davidlange6, @slava77, @fabiocos you are the release manager for this.

cms-bot commands are listed here

perrotta · 2018-08-23T15:50:40Z

@cmsbuild please test workflow 158.0

cmsbuild · 2018-08-23T15:51:00Z

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/30025/console Started: 2018/08/23 18:02

cmsbuild · 2018-08-23T18:54:46Z

+1
Tested at: 8c0580f
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-24380/30025/summary.html

cmsbuild · 2018-08-23T18:54:49Z

Comparison job queued.

cmsbuild · 2018-08-23T20:23:43Z

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

@slava77 comparisons for the following workflows were not done due to missing matrix map:

/build/cmsbld/jenkins/workspace/compare-root-files-short-matrix/results/JR-comparison/PR-24380/158.0_HydjetQ_B12_5020GeV_2018+HydjetQ_B12_5020GeV_2018+DIGIHI2018PPRECO+RECOHI2018PPRECO+HARVESTHI2018PPRECO

Comparison Summary:

No significant changes to the logs found
Reco comparison results: 0 differences found in the comparisons
DQMHistoTests: Total files compared: 31
DQMHistoTests: Total histograms compared: 2987459
DQMHistoTests: Total failures: 1
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 2987268
DQMHistoTests: Total skipped: 190
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 30 files compared)
Checked 129 log files, 14 edm output root files, 31 DQM output files

perrotta · 2018-08-24T06:43:56Z

RecoHI/HiJetAlgos/python/HiRecoPFJets_cff.py

-                                 etaRanges = cms.vdouble(-5., -3., -2.1, -1.3, 1.3, 2.1, 3., 5.)
-)
+from RecoHI.HiJetAlgos.hiFJRhoProducer import hiFJRhoProducer
+hiFJRhoProducer.etaRanges = cms.vdouble(-5., -3., -2.1, -1.3, 1.3, 2.1, 3., 5.)


Modifying modules parameters outside their own configuration file can be dangerous, as those changes can be propagated also to configurations in which you don't want to do so. As you are doing it now, the etaRanges are also going to be modified for the pA_2016 era, while they were previously the "default"
etaRanges = cms.vdouble(-5., -3., -2., -1.5, -1., 1., 1.5, 2., 3., 5.)

My suggestion in order to retain the previous etaRanges for the pA_2016 era and update them for the AA runs is the following:

Remove the modification here (and if I am not wrong you don't even need to import hiFJRhoProducer here)

Update the default etaRanges to the new ones in RecoHI/HiJetAlgos/python/hiFJRhoProducer.py

Add a modifier in RecoHI/HiJetAlgos/python/hiFJRhoProducer.py for pA_2016 and put back with it the previous etaRanges only for that era

As such the modifier is applied in the file which defines the hiFJRhoProducer module, and it will be far much easier to maintain (and the pA_2016 reconstruction won't be affected)

perrotta · 2018-08-24T06:55:26Z

RecoJets/Configuration/python/RecoJetsGlobal_cff.py

@@ -13,15 +13,11 @@

 from Configuration.Eras.Modifier_pA_2016_cff import pA_2016
 #HI-specific algorithms needed in pp scenario special configurations
-from RecoHI.HiJetAlgos.hiFJRhoProducer import hiFJRhoProducer
-
 from RecoHI.HiJetAlgos.hiFJGridEmptyAreaCalculator_cff import hiFJGridEmptyAreaCalculator
 pA_2016.toModify(hiFJGridEmptyAreaCalculator, doCentrality = False)


Since you are going to clean the configs, you can even move this modifier for the pA_2016 era into RecoHI.HiJetAlgos.hiFJGridEmptyAreaCalculator_cff

perrotta · 2018-08-24T06:55:53Z

RecoJets/Configuration/python/RecoJets_cff.py

+recoJetsHI =cms.Sequence(fixedGridRhoFastjetAllCalo+
+                         fixedGridRhoFastjetCentralCalo+
+                         ak4CaloJets+
+			 caloTowersRec+


please indent the same

perrotta · 2018-08-24T06:57:37Z

RecoJets/Configuration/python/RecoPFJets_cff.py

+                           fixedGridRhoFastjetCentral,
+                           fixedGridRhoFastjetCentralChargedPileUp,
+                           fixedGridRhoFastjetCentralNeutral,
+			   ak4PFJets,


please indent

cmsbuild · 2018-08-31T18:12:28Z

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

slava77 · 2018-08-31T18:33:54Z

@cmsbuild please test

cmsbuild · 2018-08-31T18:34:13Z

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/30182/console Started: 2018/08/31 20:35

cmsbuild · 2018-08-31T20:36:33Z

+1
Tested at: eac8530
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-24380/30182/summary.html

cmsbuild · 2018-08-31T20:36:36Z

Comparison job queued.

cmsbuild · 2018-08-31T21:53:41Z

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

Comparison Summary:

No significant changes to the logs found
Reco comparison results: 4 differences found in the comparisons
DQMHistoTests: Total files compared: 32
DQMHistoTests: Total histograms compared: 3143879
DQMHistoTests: Total failures: 2
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3143680
DQMHistoTests: Total skipped: 197
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 31 files compared)
Checked 133 log files, 14 edm output root files, 32 DQM output files

perrotta · 2018-09-02T16:29:20Z

@stepobr : did you measure the performance with the updated PR?
Could please post here your findings, if so?

stepobr · 2018-09-03T11:38:56Z

@perrotta I wasn't able to completely reproduce this kind of table, but if subtract the removed algos from your values it should be now less than 8% increase in timing and around 0.6% increase in the event size.

perrotta · 2018-09-03T14:51:44Z

Ok, I had the tools ready and I remade the comparisons for the timing.
The relevant differences are:

  delta/mean delta/orJob     original                   new       module name
  ---------- ------------     --------                  ----       ------------
       added      +0.67%         0.00 ms/ev ->        30.89 ms/ev akPu3PFJets
       added      +0.74%         0.00 ms/ev ->        34.13 ms/ev akPu4CaloJets
       added      +0.00%         0.00 ms/ev ->         0.19 ms/ev hiFJRhoProducer
     removed      -0.01%         0.58 ms/ev ->         0.00 ms/ev trackWithVertexRefSelector
       added      +0.02%         0.00 ms/ev ->         0.74 ms/ev caloTowers
     removed      -0.00%         0.01 ms/ev ->         0.00 ms/ev cmsTopTagPFJetsCHS
     removed      -0.01%         0.49 ms/ev ->         0.00 ms/ev trackRefsForJets
       added      +0.69%         0.00 ms/ev ->        31.66 ms/ev akPu4PFJets
     removed      -0.03%         1.43 ms/ev ->         0.00 ms/ev JetPlusTrackZSPCorJetAntiKt4
     removed      -0.04%         1.69 ms/ev ->         0.00 ms/ev ak4TrackJets
       added      +3.68%         0.00 ms/ev ->       169.65 ms/ev akCs4PFJets
     removed      -0.01%         0.33 ms/ev ->         0.00 ms/ev ak4JetTracksAssociatorExplicit
       added      +1.15%         0.00 ms/ev ->        52.92 ms/ev kt4PFJetsForRho
     removed      -0.09%         4.03 ms/ev ->         0.00 ms/ev ak4JetTracksAssociatorAtVertexJPT
     removed      -0.00%         0.02 ms/ev ->         0.00 ms/ev JPTeidTight
       added      +0.07%         0.00 ms/ev ->         3.16 ms/ev PFTowers
  ---------- ------------     --------                  ----       ------------
Job total:  4.60667 s/ev ==> 5.05665 s/ev

and the overall budget gets increased by less than 10% (probably even less, given the unavoidable approximations)

perrotta · 2018-09-03T15:02:20Z

+1

Jet reco sequence gets customized for HI needs
With the updates integrated during the review the configurations are cleaned up with all unneeded duplications removed, and unnecessary modules are removed from the sequence itself: this brings down the time increase with respect to the baseline to less than 10% (measured on the 158.0 wf), and the event size increase by some 0.6%
No other differences spotted from the jenkins tests

cmsbuild · 2018-09-03T15:02:41Z

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. @davidlange6, @slava77, @smuzaffar, @fabiocos (and backports should be raised in the release meeting by the corresponding L2)

fabiocos · 2018-09-04T16:16:13Z

+1

slava77 · 2018-09-18T17:38:34Z

RecoJets/Configuration/python/RecoPFJets_cff.py

+                           fixedGridRhoFastjetCentralNeutral,
+                           ak4PFJets,
+                           ak4PFJetsCHS,
+                           ak8PFJetsCHS,


CHS requires pfNoPileUpJMETask.
Please add it to this task

I think that this should resolve what was brought up in #24587 (comment)

@stepobr @mandrenguyen
do we have a fix to this already?

@slava77 I'm testing the fix right now, will soon make a pull request.

stepobr added 4 commits August 23, 2018 16:56

updating for pp_on_AA_2018

3f5865f

keep HI jets in pp_on_AA_2018

4415840

moving definition of kt4PFJetsForRho to RecoHI/JetProducers

0bffbc5

HI jet sequences for 2018_pp_on_AA

8c0580f

cmsbuild added this to the CMSSW_10_3_X milestone Aug 23, 2018

cmsbuild added code-checks-pending comparison-pending orp-pending pending-signatures reconstruction-pending tests-pending labels Aug 23, 2018

cmsbuild added code-checks-approved and removed code-checks-pending labels Aug 23, 2018

cmsbuild added tests-started and removed tests-pending labels Aug 23, 2018

cmsbuild added tests-approved and removed tests-started labels Aug 23, 2018

cmsbuild added comparison-available and removed comparison-pending labels Aug 23, 2018

perrotta reviewed Aug 24, 2018

View reviewed changes

update default values, add modifier for pa_2016

0d04517

cmsbuild added the code-checks-approved label Aug 31, 2018

cmsbuild added tests-started and removed tests-pending labels Aug 31, 2018

cmsbuild added tests-approved and removed tests-started labels Aug 31, 2018

cmsbuild added comparison-available and removed comparison-pending labels Aug 31, 2018

cmsbuild added fully-signed reconstruction-approved and removed pending-signatures reconstruction-pending labels Sep 3, 2018

cmsbuild added orp-approved and removed orp-pending labels Sep 4, 2018

cmsbuild merged commit a37b6db into cms-sw:master Sep 4, 2018

slava77 mentioned this pull request Sep 18, 2018

make a production-like workflow for HI 2018 #24587

Closed

slava77 reviewed Sep 18, 2018

View reviewed changes

stepobr mentioned this pull request Sep 25, 2018

Fixing RECO in pp_on_AA era jet reconstruction #24655

Merged

stepobr deleted the 2018_pp_on_AA_HIJets branch May 21, 2020 17:40

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

2018_pp_on_AA era HI jet sequences #24380

2018_pp_on_AA era HI jet sequences #24380

stepobr commented Aug 23, 2018

cmsbuild commented Aug 23, 2018

cmsbuild commented Aug 23, 2018

cmsbuild commented Aug 23, 2018

perrotta commented Aug 23, 2018

cmsbuild commented Aug 23, 2018 •

edited

Loading

cmsbuild commented Aug 23, 2018

cmsbuild commented Aug 23, 2018

cmsbuild commented Aug 23, 2018

perrotta Aug 24, 2018

perrotta Aug 24, 2018

perrotta Aug 24, 2018

perrotta Aug 24, 2018

cmsbuild commented Aug 31, 2018

slava77 commented Aug 31, 2018

cmsbuild commented Aug 31, 2018 •

edited

Loading

cmsbuild commented Aug 31, 2018

cmsbuild commented Aug 31, 2018

cmsbuild commented Aug 31, 2018

perrotta commented Sep 2, 2018

stepobr commented Sep 3, 2018

perrotta commented Sep 3, 2018

perrotta commented Sep 3, 2018

cmsbuild commented Sep 3, 2018

fabiocos commented Sep 4, 2018

slava77 Sep 18, 2018

slava77 Sep 18, 2018

slava77 Sep 21, 2018

stepobr Sep 21, 2018

2018_pp_on_AA era HI jet sequences #24380

2018_pp_on_AA era HI jet sequences #24380

Conversation

stepobr commented Aug 23, 2018

cmsbuild commented Aug 23, 2018

cmsbuild commented Aug 23, 2018

cmsbuild commented Aug 23, 2018

perrotta commented Aug 23, 2018

cmsbuild commented Aug 23, 2018 • edited Loading

cmsbuild commented Aug 23, 2018

cmsbuild commented Aug 23, 2018

cmsbuild commented Aug 23, 2018

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

cmsbuild commented Aug 31, 2018

slava77 commented Aug 31, 2018

cmsbuild commented Aug 31, 2018 • edited Loading

cmsbuild commented Aug 31, 2018

cmsbuild commented Aug 31, 2018

cmsbuild commented Aug 31, 2018

perrotta commented Sep 2, 2018

stepobr commented Sep 3, 2018

perrotta commented Sep 3, 2018

perrotta commented Sep 3, 2018

cmsbuild commented Sep 3, 2018

fabiocos commented Sep 4, 2018

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

cmsbuild commented Aug 23, 2018 •

edited

Loading

cmsbuild commented Aug 31, 2018 •

edited

Loading