EDProducer/EDAnalyzer and consumes migration for some DT classes #35255

namapane · 2021-09-13T14:57:41Z

Migrate some DT classes according to issue cms-AlCaDB/AlCaTools#25 and comment to #35208
#35208 (comment)

fixes cms-AlCaDB/AlCaTools#25

cmsbuild · 2021-09-13T15:06:09Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-35255/25235

This PR adds an extra 28KB to repository
There are other open Pull requests which might conflict with changes you have proposed:
- File CalibMuon/DTCalibration/plugins/DTTPAnalyzer.cc modified in PR(s): Added esConsumes calls to code in RecoLocalMuon #35184
- File CalibMuon/DTCalibration/plugins/DTTTrigCalibration.h modified in PR(s): Added esConsumes calls to code in RecoLocalMuon #35184
- File CalibMuon/DTCalibration/plugins/DTVDriftCalibration.h modified in PR(s): Added esConsumes calls to code in RecoLocalMuon #35184
- File RecoLocalMuon/DTSegment/src/DTRecSegment2DExtendedProducer.h modified in PR(s): Added esConsumes calls to code in RecoLocalMuon #35184

cmsbuild · 2021-09-13T15:07:04Z

A new Pull Request was created by @namapane (Nicola Amapane) for master.

It involves the following packages:

CalibMuon/DTCalibration (alca)
RecoLocalMuon/DTSegment (reconstruction)

@malbouis, @yuanchao, @cmsbuild, @slava77, @jpata, @francescobrivio, @tvami can you please review it and eventually sign? Thanks.
@battibass, @abbiendi, @jhgoh, @bellan, @tocheng, @mmusich 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-09-13T15:11:13Z

@namapane thanks for taking care of this. Can you please add fixes https://github.com/cms-AlCaDB/AlCaTools/issues/25 to the description? This is a github hook that will automatically close the issue when this PR is merged. Thanks!

tvami · 2021-09-13T15:13:33Z

@cmsbuild , please test

cmsbuild · 2021-09-13T18:47:22Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-a0c9c1/18561/summary.html
COMMIT: 04bbb3d
CMSSW: CMSSW_12_1_X_2021-09-13-1100/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/35255/18561/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-a0c9c1/18561/llvm-analysis/esrget-sa.txt for details.

Comparison Summary

Summary:

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

tvami · 2021-09-13T18:52:03Z

CMS StaticAnalyzer warnings: There are 1 EventSetupRecord::get warnings. See https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-a0c9c1/18561/llvm-analysis/esrget-sa.txt for details.

/data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_12_1_X_2021-09-13-1100/src/CalibMuon/DTCalibration/plugins/DTTPAnalyzer.cc:91:3: warning: function 'DTTPAnalyzer::beginRun' calls function 'edm::eventsetup::EventSetupRecord::get<edm::ESHandle >' with argument of type 'edm::ESHandle'. Direct call of function EventSetupRecord::get(ESHandle&) is deprecated and should be replaced with a call to EventSetup::getHandle(ESGetToken&). To use ESGetToken see https://twiki.cern.ch/twiki/bin/view/CMSPublic/SWGuideHowToGetDataFromES#In_ED_module To get data with the token see https://twiki.cern.ch/twiki/bin/view/CMSPublic/SWGuideHowToGetDataFromES#Getting_data_from_EventSetup_wit [deprecated.ESRecordGetChecker]

namapane · 2021-09-14T10:29:09Z

CMS StaticAnalyzer warnings: There are 1 EventSetupRecord::get warnings. See https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-a0c9c1/18561/llvm-analysis/esrget-sa.txt for details.

/data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_12_1_X_2021-09-13-1100/src/CalibMuon/DTCalibration/plugins/DTTPAnalyzer.cc:91:3: warning: function 'DTTPAnalyzer::beginRun' calls function 'edm::eventsetup::EventSetupRecord::get<edm::ESHandle >' with argument of type 'edm::ESHandle'. Direct call of function EventSetupRecord::get(ESHandle&) is deprecated and should be replaced with a call to EventSetup::getHandle(ESGetToken&). To use ESGetToken see https://twiki.cern.ch/twiki/bin/view/CMSPublic/SWGuideHowToGetDataFromES#In_ED_module To get data with the token see https://twiki.cern.ch/twiki/bin/view/CMSPublic/SWGuideHowToGetDataFromES#Getting_data_from_EventSetup_wit [deprecated.ESRecordGetChecker]

This one is being migrated in #35184 (which resulted in a conflict - I should have fixed it now)

cmsbuild · 2021-09-14T15:10:37Z

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-35255/25257

ERROR: Build errors found during clang-tidy run.

CalibMuon/DTCalibration/plugins/DTTPAnalyzer.cc:77:52: error: too many arguments to function call, expected 2, have 3 [clang-diagnostic-error]
                                                   consumesCollector());
                                                   ^
/cvmfs/cms-ib.cern.ch/nweek-02698/slc7_amd64_gcc900/cms/cmssw-patch/CMSSW_12_1_X_2021-09-13-2300/src/FWCore/PluginManager/interface/PluginFactory.h:63:24: note: 'create' declared here
--
gmake: *** [config/SCRAM/GMake/Makefile.coderules:129: code-checks] Error 2
gmake: *** [There are compilation/build errors. Please see the detail log above.] Error 2

namapane · 2021-09-14T15:14:44Z

Ops, I made the merge but now that has to be tested on top of #35184...

tvami · 2021-09-14T16:46:20Z

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-35255/25257

ERROR: Build errors found during clang-tidy run.

CalibMuon/DTCalibration/plugins/DTTPAnalyzer.cc:77:52: error: too many arguments to function call, expected 2, have 3 [clang-diagnostic-error]
                                                   consumesCollector());
                                                   ^
/cvmfs/cms-ib.cern.ch/nweek-02698/slc7_amd64_gcc900/cms/cmssw-patch/CMSSW_12_1_X_2021-09-13-2300/src/FWCore/PluginManager/interface/PluginFactory.h:63:24: note: 'create' declared here
--
gmake: *** [config/SCRAM/GMake/Makefile.coderules:129: code-checks] Error 2
gmake: *** [There are compilation/build errors. Please see the detail log above.] Error 2

Hi @namapane have you seen this code check failure above?

namapane · 2021-09-15T09:13:20Z

Hi @namapane have you seen this code check failure above?

That's what I was referring to in the comment above:
The merge of #35184 caused a conflict in this file, which I tried to resolve with the latest commit (merge+resolve conflict)
I do not understand what went wrong as the error message refers to modifications of #35184
It looks like the test code check did not pick the updates of that PR in CalibMuon/DTDigiSync but only in the package involved with this PR.

I wil try to rebase my branch from scratch...

tvami · 2021-09-15T15:43:05Z

Hi @namapane can you please rebase to the latest IB?
i.e. something like

cmsrel CMSSW_12_1_X_2021-09-15-1100
cd CMSSW_12_1_X_2021-09-15-1100/src
cmsenv
git cms-init
git checkout DT_migrations01
git add-pkg CalibMuon/DTCalibration
git add-pkg RecoLocalMuon/DTSegment
git rebase -i CMSSW_12_1_X_2021-09-15-1100

....
there might be conflicts to be merged by hand

git rebase --continue
git push my-cmssw +HEAD:DT_migrations01

I hope this will resolve this issue

cmsbuild · 2021-09-16T09:11:26Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-35255/25317

This PR adds an extra 8KB to repository

cmsbuild · 2021-09-16T09:11:52Z

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

tvami · 2021-09-16T12:38:03Z

@cmsbuild , please test

cmsbuild · 2021-09-16T15:57:05Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-a0c9c1/18662/summary.html
COMMIT: 44e249b
CMSSW: CMSSW_12_1_X_2021-09-16-1100/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/35255/18662/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: 3 differences found in the comparisons
DQMHistoTests: Total files compared: 39
DQMHistoTests: Total histograms compared: 3000833
DQMHistoTests: Total failures: 5
DQMHistoTests: Total nulls: 1
DQMHistoTests: Total successes: 3000805
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.004 KiB( 38 files compared)
DQMHistoSizes: changed ( 312.0 ): 0.004 KiB MessageLogger/Warnings
Checked 165 log files, 37 edm output root files, 39 DQM output files
TriggerResults: no differences found

tvami · 2021-09-16T16:06:05Z

+alca

comparison diffs are only in the msg logger
there is still a lot of SA warnings, but that's for another PR:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-a0c9c1/18662/llvm-analysis/

slava77 · 2021-09-16T17:40:53Z

+reconstruction

for #35255 44e249b

code changes in reco are somewhat unrelated to the main topic of the PR; here only RecoLocalMuon/DTSegment/src/DTRecSegment2DExtendedProducer.h is updated to use edm::stream, which is OK
jenkins tests pass

cmsbuild · 2021-09-16T17:41:12Z

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-09-17T01:22:53Z

+1

cmsbuild added this to the CMSSW_12_1_X milestone Sep 13, 2021

cmsbuild added alca-pending code-checks-pending orp-pending pending-signatures reconstruction-pending tests-pending labels Sep 13, 2021

cmsbuild added code-checks-approved and removed code-checks-pending labels Sep 13, 2021

cmsbuild added tests-started and removed tests-pending labels Sep 13, 2021

cmsbuild added tests-approved and removed tests-started labels Sep 13, 2021

cmsbuild added code-checks-pending tests-pending and removed tests-approved code-checks-approved labels Sep 14, 2021

cmsbuild added code-checks-rejected and removed code-checks-pending labels Sep 14, 2021

namapane added 2 commits September 16, 2021 10:58

Migrate a few EDProducers/EDAnalyzers

5d16286

consumes migration for DumpDBToFile te

44e249b

namapane force-pushed the DT_migrations01 branch from dba798b to 44e249b Compare September 16, 2021 09:03

cmsbuild added code-checks-pending and removed code-checks-rejected labels Sep 16, 2021

cmsbuild added code-checks-approved and removed code-checks-pending labels Sep 16, 2021

cmsbuild added tests-started and removed tests-pending labels Sep 16, 2021

cmsbuild added tests-approved and removed tests-started labels Sep 16, 2021

cmsbuild added alca-approved and removed alca-pending labels Sep 16, 2021

cmsbuild added fully-signed reconstruction-approved and removed reconstruction-pending pending-signatures labels Sep 16, 2021

cmsbuild added orp-approved and removed orp-pending labels Sep 17, 2021

cmsbuild merged commit 90faddb into cms-sw:master Sep 17, 2021

cmsbuild mentioned this pull request Sep 20, 2021

Update Geant4 to 10.7.2 cms-sw/cmsdist#7316

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

EDProducer/EDAnalyzer and consumes migration for some DT classes #35255

EDProducer/EDAnalyzer and consumes migration for some DT classes #35255

namapane commented Sep 13, 2021 •

edited

Loading

cmsbuild commented Sep 13, 2021

cmsbuild commented Sep 13, 2021

tvami commented Sep 13, 2021

tvami commented Sep 13, 2021

cmsbuild commented Sep 13, 2021

tvami commented Sep 13, 2021

namapane commented Sep 14, 2021 •

edited

Loading

cmsbuild commented Sep 14, 2021

namapane commented Sep 14, 2021

tvami commented Sep 14, 2021

namapane commented Sep 15, 2021 •

edited

Loading

tvami commented Sep 15, 2021

cmsbuild commented Sep 16, 2021

cmsbuild commented Sep 16, 2021

tvami commented Sep 16, 2021

cmsbuild commented Sep 16, 2021

tvami commented Sep 16, 2021

slava77 commented Sep 16, 2021

cmsbuild commented Sep 16, 2021

qliphy commented Sep 17, 2021

EDProducer/EDAnalyzer and consumes migration for some DT classes #35255

EDProducer/EDAnalyzer and consumes migration for some DT classes #35255

Conversation

namapane commented Sep 13, 2021 • edited Loading

cmsbuild commented Sep 13, 2021

cmsbuild commented Sep 13, 2021

tvami commented Sep 13, 2021

tvami commented Sep 13, 2021

cmsbuild commented Sep 13, 2021

Comparison Summary

tvami commented Sep 13, 2021

namapane commented Sep 14, 2021 • edited Loading

cmsbuild commented Sep 14, 2021

namapane commented Sep 14, 2021

tvami commented Sep 14, 2021

namapane commented Sep 15, 2021 • edited Loading

tvami commented Sep 15, 2021

cmsbuild commented Sep 16, 2021

cmsbuild commented Sep 16, 2021

tvami commented Sep 16, 2021

cmsbuild commented Sep 16, 2021

Comparison Summary

tvami commented Sep 16, 2021

slava77 commented Sep 16, 2021

cmsbuild commented Sep 16, 2021

qliphy commented Sep 17, 2021

namapane commented Sep 13, 2021 •

edited

Loading

namapane commented Sep 14, 2021 •

edited

Loading

namapane commented Sep 15, 2021 •

edited

Loading