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

Rebase of #19222: JME python configuration cleanup for 92x #19406

Merged
merged 15 commits into from
Jun 30, 2017

Conversation

rappoccio
Copy link
Contributor

@rappoccio rappoccio commented Jun 22, 2017

Had a git merge problem that I could not fix in #19222, so re-adding a new PR. Here is the original message:

The JME python configurations had many vestiges left of previous migrations, as well as considerable "top-level" customization that is no longer relevant. I have consolidated the RecoPFJets_cff customizations into separate cfi python files in RecoJets/JetProducers.

Removed iterative cone, SIScone, and AK5 PF and Calo jets in all configurations.
Consolidated the various grooming schemes into a single file per jet radius (so, AK4 puppi is now in ak4PFJets_cfi, for instance).
Left AK4, AK8, KT4, KT6, and only AK5/7 Castor collections since those are the only ones used anywhere in the repository.
Added an example AK8 GenJet configuration with grooming applied, since this is a common question. It is not presently run anywhere, though.
EDIT: No changes should occur to any output, this is a reorganization.

@cmsbuild
Copy link
Contributor

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

It involves the following packages:

CommonTools/ParticleFlow
PhysicsTools/PatAlgos
PhysicsTools/PatUtils
PhysicsTools/TagAndProbe
RecoJets/Configuration
RecoJets/JetProducers

@perrotta, @cmsbuild, @slava77, @monttj, @davidlange6 can you please review it and eventually sign? Thanks.
@TaiSakuma, @gouskos, @imarches, @ahinzmann, @acaudron, @gpetruc, @mmarionncern, @jdolen, @nhanvtran, @JyothsnaKomaragiri, @gkasieczka, @yslai, @schoef, @ferencek, @mverzett, @mariadalfonso, @pvmulder, @seemasharmafnal, @cbernet this is something you requested to watch as well.
@davidlange6 you are the release manager for this.

cms-bot commands are listed here

@perrotta
Copy link
Contributor

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Jun 22, 2017

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/20850/console Started: 2017/06/22 17:28

@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-19406/20850/summary.html

Comparison Summary:

  • No significant changes to the logs found
  • Reco comparison results: 25 differences found in the comparisons
  • DQMHistoTests: Total files compared: 22
  • DQMHistoTests: Total histograms compared: 1802916
  • DQMHistoTests: Total failures: 14601
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 1788149
  • DQMHistoTests: Total skipped: 166
  • DQMHistoTests: Total Missing objects: 0
  • Checked 90 log files, 14 edm output root files, 22 DQM output files

@rappoccio
Copy link
Contributor Author

Looking at this, there seem to be some O(10%) differences in soft drop. I will follow up.

@@ -11,3 +11,25 @@
rParam = cms.double(0.4)
Copy link
Contributor

Choose a reason for hiding this comment

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

How about turning on "doAreaFastjet=True" for ak4PFJets (and thus all other derived ak jet algorithms) during this cleanup. This is currently done only in the PAT/MiniAOD configuration and lead to many errors in the past.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, will do.

@rappoccio
Copy link
Contributor Author

OK I figured out what the remaining problem it. It's actually technically a bug in the previous implementation. The AK8 CHS soft dropped jets were previously only requiring pt > 100 GeV, whereas it was intended to also have a pre-clustering with AK8 that has an |eta| < 2.4 cut.

@ahinzmann @jdolen do you two agree with the addition of the |eta| < 2.4 cut and the AK8 preclustering?

Our new implementation will slightly speed things up, and fix this "unintended feature". @slava77 is that acceptable?

@slava77
Copy link
Contributor

slava77 commented Jun 23, 2017 via email

@rappoccio
Copy link
Contributor Author

@slava77 understood. I will retain the old functionality.

@slava77
Copy link
Contributor

slava77 commented Jun 26, 2017

the errors appear to be unrelated to this PR

#5  std::string::_M_data (this=0x14e) at /mnt/build/davidlt/gcc530_final/b/BUILD/slc6_amd64_gcc530/external/gcc/5.3.0/gcc-tags_gcc_5_3_0_release-231255/obj/x86_64-pc-linux-gnu/libstdc++-v3/include/bits/basic_string.h:2691
#6  std::string::_M_rep (this=0x14e) at /mnt/build/davidlt/gcc530_final/b/BUILD/slc6_amd64_gcc530/external/gcc/5.3.0/gcc-tags_gcc_5_3_0_release-231255/obj/x86_64-pc-linux-gnu/libstdc++-v3/include/bits/basic_string.h:2699
#7  std::basic_string<char, std::char_traits<char>, std::allocator<char> >::basic_string (this=0x7fe98f765030, __str=...) at /mnt/build/davidlt/gcc530_final/b/BUILD/slc6_amd64_gcc530/external/gcc/5.3.0/gcc-tags_gcc_5_3_0_release-231255/obj/x86_64-pc-linux-gnu/libstdc++-v3/include/bits/basic_string.tcc:617
#8  0x00007fe991872fd5 in XrdCl::URL::GetHostId (this=<optimized out>) at /build/cmsbld/auto-builds/CMSSW_9_1_0_pre3-slc6_amd64_gcc530/build/CMSSW_9_1_0_pre3-build/BUILD/slc6_amd64_gcc530/external/xrootd/4.6.1.RC2/xrootd-4.6.1.RC2/src/./XrdCl/XrdClURL.hh:72

@Dr15Jones is this a known xrootd failure?

@Dr15Jones
Copy link
Contributor

I can't say that the failure looks familiar. @bbockelm ?

@cmsbuild
Copy link
Contributor

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

Comparison Summary:

  • You potentially added 137 lines to the logs
  • Reco comparison results: 0 differences found in the comparisons
  • DQMHistoTests: Total files compared: 22
  • DQMHistoTests: Total histograms compared: 1813798
  • DQMHistoTests: Total failures: 14580
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 1799052
  • DQMHistoTests: Total skipped: 166
  • DQMHistoTests: Total Missing objects: 0
  • Checked 90 log files, 14 edm output root files, 22 DQM output files

@perrotta
Copy link
Contributor

please test
(let see what happens with the new 93X IB)

@cmsbuild
Copy link
Contributor

cmsbuild commented Jun 27, 2017

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/20932/console Started: 2017/06/27 10:48

@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-19406/20932/summary.html

Comparison Summary:

  • No significant changes to the logs found
  • Reco comparison results: 0 differences found in the comparisons
  • DQMHistoTests: Total files compared: 22
  • DQMHistoTests: Total histograms compared: 1754263
  • DQMHistoTests: Total failures: 14560
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 1739537
  • DQMHistoTests: Total skipped: 166
  • DQMHistoTests: Total Missing objects: 0
  • Checked 90 log files, 14 edm output root files, 22 DQM output files

@perrotta
Copy link
Contributor

+1
Reorganization and clean up of jet configurations
After the latest fixes, there is no effect any more on the tested workflows

@davidlange6
Copy link
Contributor

merge

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.

8 participants