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

sort or order btags in python configuration #34409

Conversation

slava77
Copy link
Contributor

@slava77 slava77 commented Jul 8, 2021

this was triggered by observed non-repeatable btag comparison plots cms-sw/cmsdist#7112 (comment)

apparently the difference originates from python set being not ordered (not repeatable either)

I found two places based on config dumps of 136.88811 and 136.874

  • setupBTagging where the patJets discriminatorSources (and related) are defined using python set
  • DeepDoubleX : here the clone parameters were using a set display { } (instead of a more common list display [ ] that we use in other python configs

@makortel
the order of the following is not repeatable; This may complicate the edmConfigDump-based comparisons:

  • canDeleteEarly
  • logErrorHarvester.excludeModules and .includeModules

@slava77
Copy link
Contributor Author

slava77 commented Jul 8, 2021

@cmsbuild please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Jul 8, 2021

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-34409/23813

  • This PR adds an extra 32KB to repository

@cmsbuild
Copy link
Contributor

cmsbuild commented Jul 8, 2021

A new Pull Request was created by @slava77 (Slava Krutelyov) for master.

It involves the following packages:

PhysicsTools/PatAlgos (reconstruction)
RecoBTag/ONNXRuntime (reconstruction)

@perrotta, @jpata, @slava77 can you please review it and eventually sign? Thanks.
@mmarionncern, @gouskos, @JyothsnaKomaragiri, @ahinzmann, @schoef, @rappoccio, @swozniewski, @jdamgov, @jdolen, @nhanvtran, @gkasieczka, @clelange, @emilbols, @hatakeyamak, @mbluj, @gpetruc, @andrzejnovak, @mariadalfonso, @seemasharmafnal this is something you requested to watch as well.
@silviodonato, @dpiparo, @qliphy, @perrotta you are the release manager for this.

cms-bot commands are listed here

@makortel
Copy link
Contributor

makortel commented Jul 8, 2021

the order of the following is not repeatable; This may complicate the edmConfigDump-based comparisons:

Thanks, I'll take a look.

@makortel
Copy link
Contributor

makortel commented Jul 8, 2021

the order of the following is not repeatable; This may complicate the edmConfigDump-based comparisons:

  • canDeleteEarly
  • logErrorHarvester.excludeModules and .includeModules

These are addressed in #34412

@cmsbuild
Copy link
Contributor

cmsbuild commented Jul 8, 2021

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-bbfde2/16620/summary.html
COMMIT: b325bb2
CMSSW: CMSSW_12_0_X_2021-07-08-1100/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/34409/16620/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

  • No significant changes to the logs found
  • Reco comparison results: 5137 differences found in the comparisons
  • DQMHistoTests: Total files compared: 38
  • DQMHistoTests: Total histograms compared: 2786260
  • DQMHistoTests: Total failures: 1
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 2786237
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 37 files compared)
  • Checked 160 log files, 37 edm output root files, 38 DQM output files
  • TriggerResults: no differences found

@slava77
Copy link
Contributor Author

slava77 commented Jul 8, 2021

+reconstruction

for #34409 b325bb2

  • code changes are technical for the physics results, but is meant to recover the simple functionality of the reco comparison scripts
  • jenkins tests pass and comparisons with the baseline show differences only in the pat jet tagging variables in reco comparison script outputs; no differences appear in the DQM plots
  • a manual check of the step2.root output of the wf 136.88811 using https://github.com/slava77/cms-reco-tools/blob/master/printFWLite.C#L90 compare_patJets_btags shows that the order of tagging variables changed to alpha-ordered and the saved values did not change
  • repeatability of configuration was checked with config dump before the PR was made

@cmsbuild
Copy link
Contributor

cmsbuild commented Jul 8, 2021

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

@qliphy
Copy link
Contributor

qliphy commented Jul 8, 2021

+1

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.

5 participants