Remove CMS deprecation warnings from RecoBTag/PerformanceDB #36576

Dr15Jones · 2021-12-22T18:32:23Z

PR description:

changed legacy module types to thread-friendly types
added esConsumes

PR validation:

Code compiles.

cmsbuild · 2021-12-22T18:38:34Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-36576/27531

This PR adds an extra 24KB to repository

cmsbuild · 2021-12-22T18:38:54Z

A new Pull Request was created by @Dr15Jones (Chris Jones) for master.

It involves the following packages:

RecoBTag/PerformanceDB (reconstruction)

@jpata, @cmsbuild, @clacaputo, @slava77 can you please review it and eventually sign? Thanks.
@JyothsnaKomaragiri, @emilbols, @demuller, @andrzejnovak, @AlexDeMoor this is something you requested to watch as well.
@perrotta, @dpiparo, @qliphy you are the release manager for this.

cms-bot commands are listed here

Dr15Jones · 2021-12-22T18:42:09Z

please test

Dr15Jones · 2021-12-22T18:43:16Z

NOTE: this package appears to exist solely to deliver the EventSetup data product BtagPerformance which is not used anywhere else in CMSSW. It appears to me like this whole package could be removed.

cmsbuild · 2021-12-22T22:34:45Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-2ab928/21442/summary.html
COMMIT: 62b85c8
CMSSW: CMSSW_12_3_X_2021-12-21-2300/slc7_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/36576/21442/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: 0 differences found in the comparisons
DQMHistoTests: Total files compared: 43
DQMHistoTests: Total histograms compared: 3461692
DQMHistoTests: Total failures: 0
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3461670
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 42 files compared)
Checked 181 log files, 42 edm output root files, 43 DQM output files
TriggerResults: no differences found

slava77 · 2021-12-23T17:25:02Z

RecoBTag/PerformanceDB/test/TestBtagPayloads.cc

+  for (auto const& n : measureName_) {
+    if (lastName != n) {
+      lastName = n;
+      lastToken = esConsumes(edm::ESInputTag("", n));


is there some cost from calling esConsumes multiple times and more importantly, is there a change in logic if it's called multiple times?

If there is no harm, then this optimization to skip is ok,

if the logic changes, then this check seems rather weak (assuming that measureName_ is sorted, as in the current hardcoded implementation) and some more thorough check is needed (perhaps the easiest is to make a sorted temporary)

There is only a minor performance hit.

needs more memory to store the consumes info

each identical consumes will create an additional entry in the WaitingTaskHolder for the item which takes more memory and a small bit of time.

Mostly, this bugs me aesthetically.

slava77 · 2021-12-23T17:30:26Z

RecoBTag/PerformanceDB/test/TestBtagPayloads.cc

-  //Possible algorithms: TTBARWPBTAGCSVL,  TTBARWPBTAGCSVM,   TTBARWPBTAGCSVT
-  //                     TTBARWPBTAGJPL,   TTBARWPBTAGJPM,    TTBARWPBTAGJPT
-  //                                                          TTBARWPBTAGTCHPT


Suggested change

//Possible algorithms: TTBARWPBTAGCSVL, TTBARWPBTAGCSVM, TTBARWPBTAGCSVT

// TTBARWPBTAGJPL, TTBARWPBTAGJPM, TTBARWPBTAGJPT

// TTBARWPBTAGTCHPT

//Possible algorithms: TTBARWPBTAGCSVL, TTBARWPBTAGCSVM, TTBARWPBTAGCSVT

// TTBARWPBTAGJPL, TTBARWPBTAGJPM, TTBARWPBTAGJPT

// TTBARWPBTAGTCHPT

this comment is lost in full; perhaps instead of recovering here, better copy this up above before filling the measureName_

Good catch.

cmsbuild · 2021-12-23T18:27:24Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-36576/27542

This PR adds an extra 12KB to repository

cmsbuild · 2021-12-23T18:27:49Z

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

Dr15Jones · 2021-12-23T18:51:42Z

please test

- Restored original comment - Made reduced esConsumes call algorithm more robust.

cmsbuild · 2021-12-23T19:25:54Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-36576/27543

This PR adds an extra 12KB to repository

cmsbuild · 2021-12-23T19:26:15Z

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

Dr15Jones · 2021-12-23T19:26:40Z

please test

cmsbuild · 2021-12-23T23:18:18Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-2ab928/21457/summary.html
COMMIT: 48a4603
CMSSW: CMSSW_12_3_X_2021-12-23-1100/slc7_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/36576/21457/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: 0 differences found in the comparisons
DQMHistoTests: Total files compared: 43
DQMHistoTests: Total histograms compared: 3461692
DQMHistoTests: Total failures: 0
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3461670
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 42 files compared)
Checked 181 log files, 42 edm output root files, 43 DQM output files
TriggerResults: no differences found

slava77 · 2021-12-24T16:12:51Z

+reconstruction

for #36576 48a4603

code changes are technical, in line with the PR description and the follow up review. IIUC, most of the code is not exercised in the PR tests.
jenkins tests pass

cmsbuild · 2021-12-24T16:13:14Z

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

qliphy · 2021-12-26T01:39:57Z

+1

Remove CMS deprecation warnings from RecoBTag/PerformanceDB

62b85c8

cmsbuild added this to the CMSSW_12_3_X milestone Dec 22, 2021

cmsbuild added code-checks-pending orp-pending pending-signatures reconstruction-pending tests-pending labels Dec 22, 2021

cmsbuild added code-checks-approved and removed code-checks-pending labels Dec 22, 2021

cmsbuild added tests-started and removed tests-pending labels Dec 22, 2021

makortel mentioned this pull request Dec 22, 2021

Various esConsumes cms-sw/framework-team#339

Closed

5 tasks

cmsbuild added tests-approved and removed tests-started labels Dec 22, 2021

slava77 reviewed Dec 23, 2021

View reviewed changes

cmsbuild added code-checks-pending tests-pending and removed tests-approved code-checks-approved labels Dec 23, 2021

cmsbuild added code-checks-approved and removed code-checks-pending labels Dec 23, 2021

cmsbuild removed the tests-pending label Dec 23, 2021

cmsbuild added tests-rejected and removed tests-started labels Dec 23, 2021

Changes based on review comments

48a4603

- Restored original comment - Made reduced esConsumes call algorithm more robust.

Dr15Jones force-pushed the deprecated_RecoBTagPerformanceDB branch from db19db4 to 48a4603 Compare December 23, 2021 19:18

cmsbuild added code-checks-pending tests-pending and removed tests-rejected code-checks-approved labels Dec 23, 2021

cmsbuild added code-checks-approved and removed code-checks-pending labels Dec 23, 2021

cmsbuild added tests-started and removed tests-pending labels Dec 23, 2021

cmsbuild added tests-approved and removed tests-started labels Dec 23, 2021

cmsbuild added fully-signed reconstruction-approved and removed reconstruction-pending pending-signatures labels Dec 24, 2021

cmsbuild added orp-approved and removed orp-pending labels Dec 26, 2021

cmsbuild merged commit 8368caf into cms-sw:master Dec 26, 2021

cmsbuild mentioned this pull request Dec 28, 2021

Drop -fsanitize=pointer-compare from ASAN IB cms-sw/cmsdist#7516

Merged

Dr15Jones deleted the deprecated_RecoBTagPerformanceDB branch January 6, 2022 16:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove CMS deprecation warnings from RecoBTag/PerformanceDB #36576

Remove CMS deprecation warnings from RecoBTag/PerformanceDB #36576

Dr15Jones commented Dec 22, 2021

cmsbuild commented Dec 22, 2021

cmsbuild commented Dec 22, 2021

Dr15Jones commented Dec 22, 2021

Dr15Jones commented Dec 22, 2021

cmsbuild commented Dec 22, 2021

slava77 Dec 23, 2021

Dr15Jones Dec 23, 2021

slava77 Dec 23, 2021

Dr15Jones Dec 23, 2021

cmsbuild commented Dec 23, 2021

cmsbuild commented Dec 23, 2021

Dr15Jones commented Dec 23, 2021

cmsbuild commented Dec 23, 2021

cmsbuild commented Dec 23, 2021

Dr15Jones commented Dec 23, 2021

cmsbuild commented Dec 23, 2021

slava77 commented Dec 24, 2021

cmsbuild commented Dec 24, 2021

qliphy commented Dec 26, 2021

Remove CMS deprecation warnings from RecoBTag/PerformanceDB #36576

Remove CMS deprecation warnings from RecoBTag/PerformanceDB #36576

Conversation

Dr15Jones commented Dec 22, 2021

PR description:

PR validation:

cmsbuild commented Dec 22, 2021

cmsbuild commented Dec 22, 2021

Dr15Jones commented Dec 22, 2021

Dr15Jones commented Dec 22, 2021

cmsbuild commented Dec 22, 2021

Comparison Summary

slava77 Dec 23, 2021

Choose a reason for hiding this comment

Dr15Jones Dec 23, 2021

Choose a reason for hiding this comment

slava77 Dec 23, 2021

Choose a reason for hiding this comment

Dr15Jones Dec 23, 2021

Choose a reason for hiding this comment

cmsbuild commented Dec 23, 2021

cmsbuild commented Dec 23, 2021

Dr15Jones commented Dec 23, 2021

cmsbuild commented Dec 23, 2021

cmsbuild commented Dec 23, 2021

Dr15Jones commented Dec 23, 2021

cmsbuild commented Dec 23, 2021

Comparison Summary

slava77 commented Dec 24, 2021

cmsbuild commented Dec 24, 2021

qliphy commented Dec 26, 2021