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.

Sign up for GitHub

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

Jump to bottom

Fix use-after-move in CITKPFIsolationSumProducer(ForPUPPI) #27118

Merged
merged 2 commits into from Jun 8, 2019
Merged

Fix use-after-move in CITKPFIsolationSumProducer(ForPUPPI) #27118

merged 2 commits into from Jun 8, 2019

Conversation

makortel
Copy link
Contributor

@makortel makortel commented Jun 5, 2019 •
edited

PR description:

This PR fixes use-after-move in CITKPFIsolationSumProducer and CITKPFIsolationSumProducerForPUPPI introduced accidentally in #25914 that leads to segfault when PluginFactory returns a unique_ptr (in #27097).

PR validation:

Code compiles.

@cmsbuild cmsbuild added this to the CMSSW_11_0_X milestone Jun 5, 2019
@cmsbuild
Copy link
Contributor

cmsbuild commented Jun 5, 2019

The code-checks are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

cmsbuild commented Jun 5, 2019

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-27118/10234

  • This PR adds an extra 16KB to repository

@makortel makortel force-pushed the pluginFactoryCITKPFIsolationSumProducer branch from 673cf10 to 278f8d5 Compare June 5, 2019 14:46
@cmsbuild
Copy link
Contributor

cmsbuild commented Jun 5, 2019

The code-checks are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

cmsbuild commented Jun 5, 2019

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-27118/10235

  • This PR adds an extra 16KB to repository

@cmsbuild
Copy link
Contributor

cmsbuild commented Jun 5, 2019

A new Pull Request was created by @makortel (Matti Kortelainen) for master.

It involves the following packages:

PhysicsTools/IsolationAlgos

@cmsbuild, @santocch can you please review it and eventually sign? Thanks.
@davidlange6, @slava77, @fabiocos you are the release manager for this.

cms-bot commands are listed here

@makortel
Copy link
Contributor Author

makortel commented Jun 5, 2019

@cmsbuild, please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Jun 5, 2019 •
edited

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-run-pr-tests/785/console Started: 2019/06/05 16:54

@makortel makortel mentioned this pull request Jun 5, 2019
Merged
@cmsbuild
Copy link
Contributor

cmsbuild commented Jun 5, 2019 •
edited

The tests are being triggered in jenkins.
Tested with other pull request(s) #27097
https://cmssdt.cern.ch/jenkins/job/ib-run-pr-tests/786/console Started: 2019/06/05 16:59

@makortel
Copy link
Contributor Author

makortel commented Jun 5, 2019

I wonder why the bot thinks that if I test this PR in conjunction with other one, this one is marked to "require external"

@cmsbuild
Copy link
Contributor

cmsbuild commented Jun 5, 2019

-1

Tested at: 278f8d5

You can see the results of the tests here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-99d998/786/summary.html

I found follow errors while testing this PR

Failed tests: UnitTests RelVals

  • Unit Tests:

I found errors in the following unit tests:

---> test testTauEmbeddingProducers had ERRORS

  • RelVals:

When I ran the RelVals I found an error in the following workflows:
136.8311 step2

runTheMatrix-results/136.8311_RunJetHT2017F_reminiaod+RunJetHT2017F_reminiaod+REMINIAOD_data2017+HARVEST2017_REMINIAOD_data2017/step2_RunJetHT2017F_reminiaod+RunJetHT2017F_reminiaod+REMINIAOD_data2017+HARVEST2017_REMINIAOD_data2017.log
136.7611 step2 
runTheMatrix-results/136.7611_RunJetHT2016E_reminiaod+RunJetHT2016E_reminiaod+REMINIAOD_data2016_HIPM+HARVESTDR2_REMINIAOD_data2016_HIPM/step2_RunJetHT2016E_reminiaod+RunJetHT2016E_reminiaod+REMINIAOD_data2016_HIPM+HARVESTDR2_REMINIAOD_data2016_HIPM.log
4.53 step3 
runTheMatrix-results/4.53_RunPhoton2012B+RunPhoton2012B+HLTD+RECODR1reHLT+HARVESTDR1reHLT/step3_RunPhoton2012B+RunPhoton2012B+HLTD+RECODR1reHLT+HARVESTDR1reHLT.log
135.4 step3 
runTheMatrix-results/135.4_ZEE_13+ZEEFS_13+HARVESTUP15FS+MINIAODMCUP15FS/step3_ZEE_13+ZEEFS_13+HARVESTUP15FS+MINIAODMCUP15FS.log
136.731 step3 
runTheMatrix-results/136.731_RunSinglePh2016B+RunSinglePh2016B+HLTDR2_2016+RECODR2_2016reHLT_skimSinglePh_HIPM+HARVESTDR2/step3_RunSinglePh2016B+RunSinglePh2016B+HLTDR2_2016+RECODR2_2016reHLT_skimSinglePh_HIPM+HARVESTDR2.log
136.788 step3 
runTheMatrix-results/136.788_RunSinglePh2017B+RunSinglePh2017B+HLTDR2_2017+RECODR2_2017reHLT_skimSinglePh_Prompt+HARVEST2017/step3_RunSinglePh2017B+RunSinglePh2017B+HLTDR2_2017+RECODR2_2017reHLT_skimSinglePh_Prompt+HARVEST2017.log
1306.0 step3 
runTheMatrix-results/1306.0_SingleMuPt1_UP15+SingleMuPt1_UP15+DIGIUP15+RECOUP15+HARVESTUP15/step3_SingleMuPt1_UP15+SingleMuPt1_UP15+DIGIUP15+RECOUP15+HARVESTUP15.log
136.85 step3 
runTheMatrix-results/136.85_RunEGamma2018A+RunEGamma2018A+HLTDR2_2018+RECODR2_2018reHLT_skimEGamma_Offline_L1TEgDQM+HARVEST2018_L1TEgDQM/step3_RunEGamma2018A+RunEGamma2018A+HLTDR2_2018+RECODR2_2018reHLT_skimEGamma_Offline_L1TEgDQM+HARVEST2018_L1TEgDQM.log
9.0 step3 
runTheMatrix-results/9.0_Higgs200ChargedTaus+Higgs200ChargedTaus+DIGI+RECO+HARVEST/step3_Higgs200ChargedTaus+Higgs200ChargedTaus+DIGI+RECO+HARVEST.log
1330.0 step3 
runTheMatrix-results/1330.0_ZMM_13+ZMM_13+DIGIUP15+RECOUP15_L1TMuDQM+HARVESTUP15_L1TMuDQM/step3_ZMM_13+ZMM_13+DIGIUP15+RECOUP15_L1TMuDQM+HARVESTUP15_L1TMuDQM.log
25.0 step3 
runTheMatrix-results/25.0_TTbar+TTbar+DIGI+RECOAlCaCalo+HARVEST+ALCATT/step3_TTbar+TTbar+DIGI+RECOAlCaCalo+HARVEST+ALCATT.log
158.0 step3 
runTheMatrix-results/158.0_HydjetQ_B12_5020GeV_2018_ppReco+HydjetQ_B12_5020GeV_2018_ppReco+DIGIHI2018PPRECO+RECOHI2018PPRECO+ALCARECOHI2018PPRECO+HARVESTHI2018PPRECO/step3_HydjetQ_B12_5020GeV_2018_ppReco+HydjetQ_B12_5020GeV_2018_ppReco+DIGIHI2018PPRECO+RECOHI2018PPRECO+ALCARECOHI2018PPRECO+HARVESTHI2018PPRECO.log
10042.0 step3 
runTheMatrix-results/10042.0_ZMM_13+ZMM_13TeV_TuneCUETP8M1_2017_GenSimFull+DigiFull_2017+RecoFull_2017+HARVESTFull_2017+ALCAFull_2017/step3_ZMM_13+ZMM_13TeV_TuneCUETP8M1_2017_GenSimFull+DigiFull_2017+RecoFull_2017+HARVESTFull_2017+ALCAFull_2017.log
10024.0 step3 
runTheMatrix-results/10024.0_TTbar_13+TTbar_13TeV_TuneCUETP8M1_2017_GenSimFull+DigiFull_2017+RecoFull_2017+HARVESTFull_2017+ALCAFull_2017/step3_TTbar_13+TTbar_13TeV_TuneCUETP8M1_2017_GenSimFull+DigiFull_2017+RecoFull_2017+HARVESTFull_2017+ALCAFull_2017.log
25202.0 step3 
runTheMatrix-results/25202.0_TTbar_13+TTbar_13+DIGIUP15_PU25+RECOUP15_PU25+HARVESTUP15_PU25/step3_TTbar_13+TTbar_13+DIGIUP15_PU25+RECOUP15_PU25+HARVESTUP15_PU25.log
11624.0 step3 
runTheMatrix-results/11624.0_TTbar_13+TTbar_13TeV_TuneCUETP8M1_2021_GenSimFull+DigiFull_2021+RecoFull_2021+HARVESTFull_2021+ALCAFull_2021/step3_TTbar_13+TTbar_13TeV_TuneCUETP8M1_2021_GenSimFull+DigiFull_2021+RecoFull_2021+HARVESTFull_2021+ALCAFull_2021.log
10824.0 step3 
runTheMatrix-results/10824.0_TTbar_13+TTbar_13TeV_TuneCUETP8M1_2018_GenSimFull+DigiFull_2018+RecoFull_2018+HARVESTFull_2018+ALCAFull_2018+NanoFull_2018/step3_TTbar_13+TTbar_13TeV_TuneCUETP8M1_2018_GenSimFull+DigiFull_2018+RecoFull_2018+HARVESTFull_2018+ALCAFull_2018+NanoFull_2018.log
10224.0 step3 
runTheMatrix-results/10224.0_TTbar_13+TTbar_13TeV_TuneCUETP8M1_2017PU_GenSimFull+DigiFullPU_2017PU+RecoFullPU_2017PU+HARVESTFullPU_2017PU/step3_TTbar_13+TTbar_13TeV_TuneCUETP8M1_2017PU_GenSimFull+DigiFullPU_2017PU+RecoFullPU_2017PU+HARVESTFullPU_2017PU.log
21234.0 step3 
runTheMatrix-results/21234.0_TTbar_14TeV+TTbar_14TeV_TuneCUETP8M1_2023D21_GenSimHLBeamSpotFull14+DigiFullTrigger_2023D21+RecoFullGlobal_2023D21+HARVESTFullGlobal_2023D21/step3_TTbar_14TeV+TTbar_14TeV_TuneCUETP8M1_2023D21_GenSimHLBeamSpotFull14+DigiFullTrigger_2023D21+RecoFullGlobal_2023D21+HARVESTFullGlobal_2023D21.log
20034.0 step3 
runTheMatrix-results/20034.0_TTbar_14TeV+TTbar_14TeV_TuneCUETP8M1_2023D17_GenSimHLBeamSpotFull14+DigiFullTrigger_2023D17+RecoFullGlobal_2023D17+HARVESTFull/bin/sh:/step3_TTbar_14TeV+TTbar_14TeV_TuneCUETP8M1_2023D35_GenSimHLBeamSpotFull14+DigiFullTrigger_2023D35+RecoFullGlobal_2023D35+HARVESTFullGlobal_2023D35.log
27434.0 step3 
runTheMatrix-results/27434.0_TTbar_14TeV+TTbar_14TeV_TuneCUETP8M1_2023D35_GenSimHLBeamSpotFull14+DigiFullTrigger_2023D35+RecoFullGlobal_2023D35+HARVESTFullGlobal_2023D35/step3_TTbar_14TeV+TTbar_14TeV_TuneCUETP8M1_2023D35_GenSimHLBeamSpotFull14+DigiFullTrigger_2023D35+RecoFullGlobal_2023D35+HARVESTFullGlobal_2023D35.log
29034.0 step4 
runTheMatrix-results/29034.0_TTbar_14TeV+TTbar_14TeV_TuneCUETP8M1_2023D41_GenSimHLBeamSpotFull14+DigiFullTrigger_2023D41+RecoFullGlobal_2023D41+HARVESTFullGlobal_2023D41/step4_TTbar_13UP18+TTbar_13UP18+PREMIXUP18_PU25+DIGIPRMXLOCALUP18_PU25+RECOPRMXUP18_PU25+HARVESTUP18_PU25.log
250202.181 step4 
runTheMatrix-results/250202.181_TTbar_13UP18+TTbar_13UP18+PREMIXUP18_PU25+DIGIPRMXLOCALUP18_PU25+RECOPRMXUP18_PU25+HARVESTUP18_PU25/step4_TTbar_13UP18+TTbar_13UP18+PREMIXUP18_PU25+DIGIPRMXLOCALUP18_PU25+RECOPRMXUP18_PU25+HARVESTUP18_PU25.log

@cmsbuild
Copy link
Contributor

cmsbuild commented Jun 5, 2019

Comparison not run due to runTheMatrix errors (RelVals and Igprof tests were also skipped)

@makortel makortel force-pushed the pluginFactoryCITKPFIsolationSumProducer branch from 278f8d5 to 206d0a7 Compare June 6, 2019 02:37
@cmsbuild
Copy link
Contributor

cmsbuild commented Jun 6, 2019

Comparison is ready
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-3d6fb2/792/summary.html

Comparison Summary:

  • No significant changes to the logs found
  • Reco comparison results: 0 differences found in the comparisons
  • DQMHistoTests: Total files compared: 33
  • DQMHistoTests: Total histograms compared: 3210222
  • DQMHistoTests: Total failures: 37
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3209851
  • DQMHistoTests: Total skipped: 334
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 13105.552 KiB( 32 files compared)
  • DQMHistoSizes: changed ( 21234.0,... ): 1143.069 KiB EgammaV/ElectronMcSignalValidatorPt1000
  • DQMHistoSizes: changed ( 21234.0,... ): 1140.263 KiB EgammaV/ElectronMcSignalValidator
  • DQMHistoSizes: changed ( 21234.0,... ): 993.056 KiB EgammaV/ElectronMcFakeValidator
  • Checked 137 log files, 14 edm output root files, 33 DQM output files

@santocch
Copy link

santocch commented Jun 7, 2019

+1

@cmsbuild
Copy link
Contributor

cmsbuild commented Jun 7, 2019

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

@fabiocos
Copy link
Contributor

fabiocos commented Jun 8, 2019

please test

to have a clean comparison

@cmsbuild
Copy link
Contributor

cmsbuild commented Jun 8, 2019 •
edited

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-run-pr-tests/846/console Started: 2019/06/08 17:55

@cmsbuild
Copy link
Contributor

cmsbuild commented Jun 8, 2019

+1
Tested at: 31fff35
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-3d6fb2/846/summary.html

@cmsbuild
Copy link
Contributor

cmsbuild commented Jun 8, 2019

Comparison job queued.

@cmsbuild
Copy link
Contributor

cmsbuild commented Jun 8, 2019

Comparison is ready
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-3d6fb2/846/summary.html

Comparison Summary:

  • No significant changes to the logs found
  • Reco comparison results: 0 differences found in the comparisons
  • DQMHistoTests: Total files compared: 33
  • DQMHistoTests: Total histograms compared: 3215458
  • DQMHistoTests: Total failures: 1
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3215123
  • DQMHistoTests: Total skipped: 334
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 32 files compared)
  • Checked 137 log files, 14 edm output root files, 33 DQM output files

@fabiocos
Copy link
Contributor

fabiocos commented Jun 8, 2019

+1

@cmsbuild cmsbuild merged commit f16f9f4 into cms-sw:master Jun 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
analysis-approved code-checks-approved comparison-available fully-signed orp-approved tests-approved
Projects
None yet
Milestone
CMSSW_11_0_X
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@makortel @cmsbuild @santocch @fabiocos