Migrate BTV pacakges to new PoolDBOutputService #36231

panwarlsweet · 2021-11-24T09:16:22Z

PR description:

As introduced in issue cms-AlCaDB/AlCaTools#28, migrating to the new methods replacing writeOne --> writeOneIOV, createNewIOV --> createOneIOV, and appendSinceTime --> appendOneIOV.
The files containing PoolDBOutputService in BTV packages that were addressed are:
RecoBTag/PerformanceDB/plugins/PhysicsPerformanceDBWriterFromFile_WPandPayload.cc
RecoBTag/PerformanceDB/plugins/PhysicsPerformanceDBWriterFromFile_WPandPayload_IOV.cc
RecoBTag/PerformanceDB/plugins/PhysicsPerformanceDBWriterTFormula_fromfile_WPandPL.cc
RecoBTag/TrackProbability/test/CalibrationSkeleton.cc
RecoBTag/TrackProbability/test/TrackProbabilityXMLtoDB.cc

PR validation:

changes compile with CMSSW
scram b runtests works fine

if this PR is a backport please specify the original PR and why you need to backport that PR:

N/A

@soureek @johnalison

…ory; correcting createOneIOV function

cmsbuild · 2021-11-24T09:22:38Z

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-36231/26854

This PR adds an extra 28KB to repository

Code check has found code style and quality issues which could be resolved by applying following patch(s)

code-format:
https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-36231/26854/code-format.patch
e.g. curl -k https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-36231/26854/code-format.patch | patch -p1
You can also run scram build code-format to apply code format directly

…ecks

cmsbuild · 2021-11-24T09:46:30Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-36231/26855

This PR adds an extra 28KB to repository

cmsbuild · 2021-11-24T09:47:00Z

A new Pull Request was created by @panwarlsweet (Lata Panwar) for master.

It involves the following packages:

RecoBTag/PerformanceDB (reconstruction)
RecoBTag/TrackProbability (reconstruction)

@jpata, @cmsbuild, @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

tvami · 2021-11-24T16:03:40Z

assign db

cmsbuild · 2021-11-24T16:03:57Z

New categories assigned: db

@ggovi,@francescobrivio,@malbouis,@tvami you have been requested to review this Pull request/Issue and eventually sign? Thanks

tvami · 2021-11-24T16:09:32Z

RecoBTag/TrackProbability/test/CalibrationSkeleton.cc

+      calibration2d.data.push_back(entry);
+    }
+
+    mydbservice->createOneIOV(calibration, "BTagTrackProbability3DRcd");


Suggested change

mydbservice->createOneIOV(calibration, "BTagTrackProbability3DRcd");

mydbservice->createOneIOV(calibration, mydbservice->endOfTime(), "BTagTrackProbability3DRcd");

tvami · 2021-11-24T16:09:50Z

RecoBTag/TrackProbability/test/CalibrationSkeleton.cc

+
+    mydbservice->createOneIOV(calibration, "BTagTrackProbability3DRcd");
+
+    mydbservice->createOneIOV(calibration2d, "BTagTrackProbability2DRcd");


Suggested change

mydbservice->createOneIOV(calibration2d, "BTagTrackProbability2DRcd");

mydbservice->createOneIOV(calibration2d, mydbservice->endOfTime(), "BTagTrackProbability2DRcd");

tvami · 2021-11-24T16:11:17Z

RecoBTag/TrackProbability/test/CalibrationSkeleton.cc

-            else           data2d[i].second = CalibratedHistogram();
-          }
-
+    calibrationNew =


I dont see where calibrationNew is deleted. But this file is under \test so fine...

tvami · 2021-11-24T16:15:35Z

RecoBTag/TrackProbability/test/CalibrationSkeleton.cc

  edm::ESHandle<TransientTrackBuilder> builder;
-  iSetup.get<TransientTrackRecord>().get("TransientTrackBuilder",builder);
+  iSetup.get<TransientTrackRecord>().get("TransientTrackBuilder", builder);
  m_transientTrackBuilder_producer = builder.product();


Suggested change

edm::ESHandle<TransientTrackBuilder> builder;

iSetup.get<TransientTrackRecord>().get("TransientTrackBuilder",builder);

iSetup.get<TransientTrackRecord>().get("TransientTrackBuilder", builder);

m_transientTrackBuilder_producer = builder.product();

auto const m_transientTrackBuilder_producer = &iSetup.getData(transientTrackBuilderToken_)

and you'll need to define transientTrackBuilderToken_ in the header as

const edm::ESGetToken<TransientTrackBuilder, TransientTrackRecord> transientTrackBuilderToken_;

and then consume it in the constructor of this module

implemented as suggested above

tvami · 2021-11-24T16:16:59Z

RecoBTag/TrackProbability/test/TrackProbabilityXMLtoDB.cc

@@ -78,82 +74,70 @@ using namespace reco;
 //

 class TrackProbabilityXMLtoDB : public edm::EDAnalyzer {


Suggested change

class TrackProbabilityXMLtoDB : public edm::EDAnalyzer {

class TrackProbabilityXMLtoDB : public edm::one::EDAnalyzer<> {

cmsbuild · 2021-12-09T18:48:12Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-36231/27270

This PR adds an extra 16KB to repository

cmsbuild · 2021-12-09T18:48:34Z

Pull request #36231 was updated. @malbouis, @cmsbuild, @ggovi, @slava77, @jpata, @tvami, @francescobrivio can you please check and sign again.

tvami · 2021-12-09T18:49:23Z

@cmsbuild , please test

cmsbuild · 2021-12-09T22:16:21Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-4a38a1/21135/summary.html
COMMIT: ad6e3e2
CMSSW: CMSSW_12_3_X_2021-12-09-1100/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/36231/21135/install.sh to create a dev area with all the needed externals and cmssw changes.

CMS StaticAnalyzer warnings: There are 1 EventSetupRecord::get warnings. See https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-4a38a1/21135/llvm-analysis/esrget-sa.txt for details.
CMS StaticAnalyzer warnings: There are 4 inherits from legacy modules warnings. See https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-4a38a1/21135/llvm-analysis/legacy-mod-sa.txt for details.
CMS deprecated warnings: 3 CMS deprecated warnings found, see summary page for details.

Comparison Summary

Summary:

No significant changes to the logs found
Reco comparison results: 4 differences found in the comparisons
DQMHistoTests: Total files compared: 42
DQMHistoTests: Total histograms compared: 3250704
DQMHistoTests: Total failures: 5
DQMHistoTests: Total nulls: 1
DQMHistoTests: Total successes: 3250676
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: -0.004 KiB( 41 files compared)
DQMHistoSizes: changed ( 312.0 ): -0.004 KiB MessageLogger/Warnings
Checked 177 log files, 37 edm output root files, 42 DQM output files
TriggerResults: no differences found

tvami · 2021-12-09T22:49:17Z

+db

there is more to do with these packages (move them to a correct directory, migrate away from es get()) but it will be done in a follow-up PR
Jenkins tests pass

slava77 · 2021-12-10T22:19:00Z

+reconstruction

for #36231 ad6e3e2

code changes are technical, specific to the PoolDB interface update
jenkins tests pass
I expect that the warnings related to es.get will be covered in a separate PR

cmsbuild · 2021-12-10T22:19:23Z

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-11T01:50:42Z

+1

panwarlsweet added 6 commits November 18, 2021 20:26

PoolDBOutputService migration for btagging packages

288984e

fixing pointers

90fbd90

fixing functions according to pointers

c0cafa9

reverting back BTagCalibrationDbCreator.cc changes

a8f1ddf

replacing cout with LogInfo as suggested by alcadb for plugins direct…

bf0d447

…ory; correcting createOneIOV function

corrected syntax of edm::LogInfo

b17e09a

cmsbuild added this to the CMSSW_12_2_X milestone Nov 24, 2021

cmsbuild added code-checks-pending orp-pending pending-signatures reconstruction-pending tests-pending labels Nov 24, 2021

cmsbuild added code-checks-rejected and removed code-checks-pending labels Nov 24, 2021

changes after running scram build code-format and scram build code-ch…

22d4add

…ecks

cmsbuild added code-checks-pending and removed code-checks-rejected labels Nov 24, 2021

cmsbuild added code-checks-approved and removed code-checks-pending labels Nov 24, 2021

cmsbuild added the db-pending label Nov 24, 2021

tvami reviewed Nov 24, 2021

View reviewed changes

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

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

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

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

cmsbuild added db-approved and removed db-pending labels Dec 9, 2021

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

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

cmsbuild merged commit e713ae1 into cms-sw:master Dec 11, 2021

This was referenced Dec 11, 2021

[12.3.X] Move to root 6.24 cms-sw/cmsdist#7500

Merged

[ROOT624] Testing root 62.4 for gcc10 cms-sw/cmsdist#7505

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Migrate BTV pacakges to new PoolDBOutputService #36231

Migrate BTV pacakges to new PoolDBOutputService #36231

panwarlsweet commented Nov 24, 2021

cmsbuild commented Nov 24, 2021

cmsbuild commented Nov 24, 2021

cmsbuild commented Nov 24, 2021

tvami commented Nov 24, 2021

cmsbuild commented Nov 24, 2021

tvami Nov 24, 2021

panwarlsweet Nov 25, 2021

tvami Nov 24, 2021

panwarlsweet Nov 25, 2021

tvami Nov 24, 2021

tvami Nov 24, 2021 •

edited

panwarlsweet Nov 25, 2021

tvami Nov 24, 2021

panwarlsweet Nov 25, 2021

cmsbuild commented Dec 9, 2021

cmsbuild commented Dec 9, 2021

tvami commented Dec 9, 2021

cmsbuild commented Dec 9, 2021

tvami commented Dec 9, 2021

slava77 commented Dec 10, 2021

cmsbuild commented Dec 10, 2021

qliphy commented Dec 11, 2021

	mydbservice->createOneIOV(calibration, "BTagTrackProbability3DRcd");
	mydbservice->createOneIOV(calibration, mydbservice->endOfTime(), "BTagTrackProbability3DRcd");


		mydbservice->createOneIOV(calibration, "BTagTrackProbability3DRcd");

		mydbservice->createOneIOV(calibration2d, "BTagTrackProbability2DRcd");

		@@ -78,82 +74,70 @@ using namespace reco;
		//

		class TrackProbabilityXMLtoDB : public edm::EDAnalyzer {

Migrate BTV pacakges to new PoolDBOutputService #36231

Migrate BTV pacakges to new PoolDBOutputService #36231

Conversation

panwarlsweet commented Nov 24, 2021

PR description:

PR validation:

if this PR is a backport please specify the original PR and why you need to backport that PR:

cmsbuild commented Nov 24, 2021

cmsbuild commented Nov 24, 2021

cmsbuild commented Nov 24, 2021

tvami commented Nov 24, 2021

cmsbuild commented Nov 24, 2021

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

Choose a reason for hiding this comment

tvami Nov 24, 2021 • edited

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 Dec 9, 2021

cmsbuild commented Dec 9, 2021

tvami commented Dec 9, 2021

cmsbuild commented Dec 9, 2021

Comparison Summary

tvami commented Dec 9, 2021

slava77 commented Dec 10, 2021

cmsbuild commented Dec 10, 2021

qliphy commented Dec 11, 2021

tvami Nov 24, 2021 •

edited