Thread safe fitting in EcalUncalibRecHitRecAnalFitAlgo #39716

Dr15Jones · 2022-10-12T18:04:10Z

PR description:

Switch to ROOT API's that are thread safe as this class is used in a stream module.
This was found by the IB static analysis information.

PR validation:

Code compiles

cmsbuild · 2022-10-12T18:12:17Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-39716/32548

This PR adds an extra 12KB to repository

cmsbuild · 2022-10-12T18:12:42Z

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

It involves the following packages:

RecoLocalCalo/EcalRecAlgos (reconstruction)

@cmsbuild, @mandrenguyen, @clacaputo can you please review it and eventually sign? Thanks.
@youyingli, @apsallid, @rchatter, @argiro, @missirol, @thomreis, @simonepigazzini this is something you requested to watch as well.
@perrotta, @dpiparo, @rappoccio you are the release manager for this.

cms-bot commands are listed here

RecoLocalCalo/EcalRecAlgos/interface/EcalUncalibRecHitRecAnalFitAlgo.h

cmsbuild · 2022-10-13T14:21:34Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-39716/32561

This PR adds an extra 12KB to repository

cmsbuild · 2022-10-13T14:22:01Z

Pull request #39716 was updated. @cmsbuild, @mandrenguyen, @clacaputo can you please check and sign again.

Dr15Jones · 2022-10-13T14:42:01Z

please test

cmsbuild · 2022-10-13T14:50:08Z

-1

Failed Tests: Build HeaderConsistency
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-1de857/28233/summary.html
COMMIT: 78dd13c
CMSSW: CMSSW_12_6_X_2022-10-13-1100/el8_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/39716/28233/install.sh to create a dev area with all the needed externals and cmssw changes.

Build

I found compilation error when building:

>> Compiling edm plugin /data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_12_6_X_2022-10-13-1100/src/RecoLocalCalo/EcalRecProducers/plugins/EcalUncalibRecHitWorkerGlobal.cc
>> Compiling edm plugin /data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_12_6_X_2022-10-13-1100/src/RecoLocalCalo/EcalRecProducers/plugins/EcalUncalibRecHitWorkerMaxSample.cc
>> Compiling edm plugin /data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_12_6_X_2022-10-13-1100/src/RecoLocalCalo/EcalRecProducers/plugins/EcalUncalibRecHitWorkerMultiFit.cc
>> Compiling edm plugin /data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_12_6_X_2022-10-13-1100/src/RecoLocalCalo/EcalRecProducers/plugins/EcalUncalibRecHitWorkerRatio.cc
In file included from /data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_12_6_X_2022-10-13-1100/src/RecoLocalCalo/EcalRecProducers/plugins/EcalUncalibRecHitWorkerAnalFit.cc:28:
/data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_12_6_X_2022-10-13-1100/src/RecoLocalCalo/EcalRecAlgos/interface/EcalUncalibRecHitRecAnalFitAlgo.h:23:23: error: 'Minuit2' is not a namespace-name; did you mean 'Minuit'?
   23 | using namespace ROOT::Minuit2;
      |                       ^~~~~~~
      |                       Minuit
>> Compiling edm plugin /data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_12_6_X_2022-10-13-1100/src/RecoLocalCalo/EcalRecProducers/plugins/EcalUncalibRecHitWorkerWeights.cc
>> Compiling  /data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_12_6_X_2022-10-13-1100/src/RecoLocalCalo/EcalRecProducers/plugins/AmplitudeComputationCommonKernels.cu

Dr15Jones · 2022-10-13T15:06:25Z

please test

cmsbuild · 2022-10-13T15:15:10Z

-1

Failed Tests: Build HeaderConsistency
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-1de857/28235/summary.html
COMMIT: 78dd13c
CMSSW: CMSSW_12_6_X_2022-10-13-1100/el8_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/39716/28235/install.sh to create a dev area with all the needed externals and cmssw changes.

Build

I found compilation error when building:

>> Compiling edm plugin /data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_12_6_X_2022-10-13-1100/src/RecoLocalCalo/EcalRecProducers/plugins/EcalUncalibRecHitWorkerGlobal.cc
>> Compiling edm plugin /data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_12_6_X_2022-10-13-1100/src/RecoLocalCalo/EcalRecProducers/plugins/EcalUncalibRecHitWorkerMaxSample.cc
>> Compiling edm plugin /data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_12_6_X_2022-10-13-1100/src/RecoLocalCalo/EcalRecProducers/plugins/EcalUncalibRecHitWorkerMultiFit.cc
>> Compiling edm plugin /data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_12_6_X_2022-10-13-1100/src/RecoLocalCalo/EcalRecProducers/plugins/EcalUncalibRecHitWorkerRatio.cc
In file included from /data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_12_6_X_2022-10-13-1100/src/RecoLocalCalo/EcalRecProducers/plugins/EcalUncalibRecHitWorkerAnalFit.cc:28:
/data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_12_6_X_2022-10-13-1100/src/RecoLocalCalo/EcalRecAlgos/interface/EcalUncalibRecHitRecAnalFitAlgo.h:23:23: error: 'Minuit2' is not a namespace-name; did you mean 'Minuit'?
   23 | using namespace ROOT::Minuit2;
      |                       ^~~~~~~
      |                       Minuit
>> Compiling edm plugin /data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_12_6_X_2022-10-13-1100/src/RecoLocalCalo/EcalRecProducers/plugins/EcalUncalibRecHitWorkerWeights.cc
>> Compiling  /data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_12_6_X_2022-10-13-1100/src/RecoLocalCalo/EcalRecProducers/plugins/AmplitudeComputationCommonKernels.cu

Switch to ROOT API's that are thread safe as this class is used in a stream module.

Dr15Jones · 2022-10-13T15:44:34Z

please test

cmsbuild · 2022-10-13T15:48:43Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-39716/32564

This PR adds an extra 12KB to repository

cmsbuild · 2022-10-13T15:49:11Z

Pull request #39716 was updated. @mandrenguyen, @clacaputo can you please check and sign again.

Dr15Jones · 2022-10-13T15:51:14Z

please test

cmsbuild · 2022-10-13T19:22:28Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-1de857/28239/summary.html
COMMIT: 1adbf2a
CMSSW: CMSSW_12_6_X_2022-10-13-1100/el8_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/39716/28239/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

@slava77 comparisons for the following workflows were not done due to missing matrix map:

/data/cmsbld/jenkins/workspace/compare-root-files-short-matrix/data/PR-1de857/20834.0_TTbar_14TeV+2026D88+TTbar_14TeV_TuneCP5_GenSimHLBeamSpot14+DigiTrigger+RecoGlobal+HARVESTGlobal
/data/cmsbld/jenkins/workspace/compare-root-files-short-matrix/data/PR-1de857/20834.75_TTbar_14TeV+2026D88_HLT75e33+TTbar_14TeV_TuneCP5_GenSimHLBeamSpot14+DigiTrigger+RecoGlobal+HLT75e33+HARVESTGlobal
/data/cmsbld/jenkins/workspace/compare-root-files-short-matrix/data/PR-1de857/20896.0_CloseByPGun_CE_E_Front_120um+2026D88+CE_E_Front_120um_GenSimHLBeamSpotHGCALCloseBy+DigiTrigger+RecoGlobal+HARVESTGlobal
/data/cmsbld/jenkins/workspace/compare-root-files-short-matrix/data/PR-1de857/20900.0_CloseByPGun_CE_H_Coarse_Scint+2026D88+CE_H_Coarse_Scint_GenSimHLBeamSpotHGCALCloseBy+DigiTrigger+RecoGlobal+HARVESTGlobal
/data/cmsbld/jenkins/workspace/compare-root-files-short-matrix/data/PR-1de857/21034.999_TTbar_14TeV+2026D88PU_PMXS1S2PR+TTbar_14TeV_TuneCP5_GenSimHLBeamSpot14+PREMIX_PremixHLBeamSpot14PU+DigiTriggerPU+RecoGlobalPU+HARVESTGlobalPU
/data/cmsbld/jenkins/workspace/compare-root-files-short-matrix/data/PR-1de857/23234.0_TTbar_14TeV+2026D94+TTbar_14TeV_TuneCP5_GenSimHLBeamSpot14+DigiTrigger+RecoGlobal+HARVESTGlobal

Summary:

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

mandrenguyen · 2022-10-14T05:34:58Z

+1
technical PR

cmsbuild · 2022-10-14T05:35:21Z

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

perrotta · 2022-10-14T05:41:15Z

+1

cmsbuild added this to the CMSSW_12_6_X milestone Oct 12, 2022

cmsbuild added code-checks-pending orp-pending pending-signatures reconstruction-pending tests-pending labels Oct 12, 2022

cmsbuild added code-checks-approved and removed code-checks-pending labels Oct 12, 2022

mandrenguyen suggested changes Oct 13, 2022

View reviewed changes

Dr15Jones force-pushed the fixFitEcalUncalibRecHitRecAnalFitAlgo branch from 097d0ac to 78dd13c Compare October 13, 2022 14:14

cmsbuild added code-checks-pending and removed code-checks-approved labels Oct 13, 2022

cmsbuild added code-checks-approved and removed code-checks-pending labels Oct 13, 2022

cmsbuild added tests-started and removed tests-pending labels Oct 13, 2022

cmsbuild added tests-rejected and removed tests-started labels Oct 13, 2022

cmsbuild added tests-started and removed tests-rejected labels Oct 13, 2022

cmsbuild added tests-rejected and removed tests-started labels Oct 13, 2022

Thread safe fitting in EcalUncalibRecHitRecAnalFitAlgo

1adbf2a

Switch to ROOT API's that are thread safe as this class is used in a stream module.

Dr15Jones force-pushed the fixFitEcalUncalibRecHitRecAnalFitAlgo branch from 78dd13c to 1adbf2a Compare October 13, 2022 15:41

cmsbuild added code-checks-pending tests-pending and removed tests-rejected code-checks-approved labels Oct 13, 2022

cmsbuild added tests-started and removed tests-pending labels Oct 13, 2022

cmsbuild added code-checks-approved and removed code-checks-pending labels Oct 13, 2022

cmsbuild added tests-approved and removed tests-started labels Oct 13, 2022

cmsbuild added fully-signed reconstruction-approved and removed reconstruction-pending pending-signatures labels Oct 14, 2022

cmsbuild added orp-approved and removed orp-pending labels Oct 14, 2022

cmsbuild merged commit 791827c into cms-sw:master Oct 14, 2022

makortel mentioned this pull request Oct 14, 2022

TH1Merger::CheckForDuplicateLabels error in workflow 4.6 #39737

Closed

Dr15Jones deleted the fixFitEcalUncalibRecHitRecAnalFitAlgo branch October 17, 2022 20:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Thread safe fitting in EcalUncalibRecHitRecAnalFitAlgo #39716

Thread safe fitting in EcalUncalibRecHitRecAnalFitAlgo #39716

Dr15Jones commented Oct 12, 2022

cmsbuild commented Oct 12, 2022

cmsbuild commented Oct 12, 2022

cmsbuild commented Oct 13, 2022

cmsbuild commented Oct 13, 2022

Dr15Jones commented Oct 13, 2022

cmsbuild commented Oct 13, 2022

Dr15Jones commented Oct 13, 2022

cmsbuild commented Oct 13, 2022

Dr15Jones commented Oct 13, 2022

cmsbuild commented Oct 13, 2022

cmsbuild commented Oct 13, 2022

Dr15Jones commented Oct 13, 2022

cmsbuild commented Oct 13, 2022

mandrenguyen commented Oct 14, 2022

cmsbuild commented Oct 14, 2022

perrotta commented Oct 14, 2022

Thread safe fitting in EcalUncalibRecHitRecAnalFitAlgo #39716

Thread safe fitting in EcalUncalibRecHitRecAnalFitAlgo #39716

Conversation

Dr15Jones commented Oct 12, 2022

PR description:

PR validation:

cmsbuild commented Oct 12, 2022

cmsbuild commented Oct 12, 2022

cmsbuild commented Oct 13, 2022

cmsbuild commented Oct 13, 2022

Dr15Jones commented Oct 13, 2022

cmsbuild commented Oct 13, 2022

Build

Dr15Jones commented Oct 13, 2022

cmsbuild commented Oct 13, 2022

Build

Dr15Jones commented Oct 13, 2022

cmsbuild commented Oct 13, 2022

cmsbuild commented Oct 13, 2022

Dr15Jones commented Oct 13, 2022

cmsbuild commented Oct 13, 2022

Comparison Summary

mandrenguyen commented Oct 14, 2022

cmsbuild commented Oct 14, 2022

perrotta commented Oct 14, 2022