Skip to content
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

Fixing RECO in pp_on_AA era jet reconstruction #24655

Merged
merged 5 commits into from Oct 3, 2018

Conversation

stepobr
Copy link
Contributor

@stepobr stepobr commented Sep 25, 2018

This pull request is a follow-up of #24380
The issue is partly described in #24587
The ideal scenario for #24380 would be to get rid of all jet algorithms not used for pp_on_AA era. However 3 of them (ak4PF, ak4PFCHS, ak8PFCHS) were kept because otherwise it failed to pass the validation step. But in that case it was later found that the sequences crashes if stopped after RECO step due to the absence of pfNoPileUpJME used by the 3 mentioned algorithms. Trivial solution would be to add back pfNoPileUpJME. But in order to speed-up reconstruction it was replaced by a dummy EDFilter with a very high pt threshold which results in an empty input for these 3 jet modules. The filter does not prevent the modules to be run, but since they have no input objects they do nothing.

@cfmcginn @mandrenguyen

@cmsbuild
Copy link
Contributor

The code-checks are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @stepobr (Stepan Obraztsov) for master.

It involves the following packages:

RecoHI/HiJetAlgos
RecoJets/Configuration
RecoJets/JetProducers

@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

@@ -33,3 +33,6 @@
doAreaFastjet = cms.bool(True),
jetPtMin = cms.double(100.0)
)
from Configuration.Eras.Modifier_pp_on_AA_2018_cff import pp_on_AA_2018
pp_on_AA_2018.toModify(ak4PFJets,src = cms.InputTag("pfNoPileUpJMEHI"), inputEtMin = cms.double(9999))
pp_on_AA_2018.toModify(ak4PFJetsCHS,src = cms.InputTag("pfNoPileUpJMEHI"), inputEtMin = cms.double(9999))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please drop type specifications for all parameters which already exist.
This is a safer way to protect from parameter name mistakes and will also help in possible parameter migrations.

@slava77
Copy link
Contributor

slava77 commented Sep 25, 2018

@cmsbuild please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 25, 2018

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/30629/console Started: 2018/09/25 16:59

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

Comparison job queued.

@cmsbuild
Copy link
Contributor

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

Comparison Summary:

  • No significant changes to the logs found
  • Reco comparison results: 308 differences found in the comparisons
  • DQMHistoTests: Total files compared: 32
  • DQMHistoTests: Total histograms compared: 3146876
  • DQMHistoTests: Total failures: 1934
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3144745
  • 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

@stepobr
Copy link
Contributor Author

stepobr commented Sep 29, 2018

Most of your points are correct, a few comments:

  • May be that's a typo but from that list ideally I would get rid of only ak4PFJets, ak4PFJetsCHS, ak8PFJetsCHS.
  • I can't do that not because of DQM, but rather because of the validation step. For example, excluding ak4PFJets from the sequence leads to:
    An exception of category 'ProductNotFound' occurred while [0] Processing Event run: 1 lumi: 1 event: 1 stream: 3 [1] Running path 'prevalidation_step' [2] Prefetching for module MultiTrackValidator/'trackValidator' [3] Prefetching for module JetTracksAssociationToTrackRefs/'cutsRecoTracksAK4PFJets' [4] Calling method for module JetTracksAssociatorExplicit/'ak4JetTracksAssociatorExplicitAll' Exception Message: Principal::getByToken: Found zero products matching all criteria Looking for a container with elements of type: reco::Jet Looking for module label: ak4PFJets Looking for productInstanceName:
  • Yes, this is the correct logic behind using this filter.

I will update the description now.

@perrotta
Copy link
Contributor

perrotta commented Oct 2, 2018

@cmsbuild please test
(to have back the plots with differences, which have disappeared)

@cmsbuild
Copy link
Contributor

cmsbuild commented Oct 2, 2018

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/30841/console Started: 2018/10/02 10:42

@cmsbuild
Copy link
Contributor

cmsbuild commented Oct 2, 2018

@cmsbuild
Copy link
Contributor

cmsbuild commented Oct 2, 2018

Comparison job queued.

@cmsbuild
Copy link
Contributor

cmsbuild commented Oct 2, 2018

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

Comparison Summary:

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

@perrotta
Copy link
Contributor

perrotta commented Oct 2, 2018

+1

  • ak4PF, ak4PFCHS, ak8PFCHS are "sterilized" thanks to the pfNoPileUpJME selector, which feeds them with an empty set of PF objects
  • The implementation follows what is in the PR description
  • Changes in the jenkins tets outputs are only in the 2018 HI workflow, and they correspond to the expectations (empty ak4PF, ak4PFCHS, ak8PFCHS collections)

@cmsbuild
Copy link
Contributor

cmsbuild commented Oct 2, 2018

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
Copy link
Contributor

fabiocos commented Oct 3, 2018

+1

@cmsbuild cmsbuild merged commit e07a28e into cms-sw:master Oct 3, 2018
@stepobr stepobr deleted the from-CMSSW_10_3_0_pre4 branch May 21, 2020 17:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants