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.

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

Fix use of std::unique_ptr in HadronizerFilter #14293

Merged
merged 1 commit into from Apr 29, 2016

Conversation

Dr15Jones
Copy link
Contributor

When the HadronizerFilter was changed to use unique_ptr from auto_ptr a
key difference between the two was missed. the method 'reset' on an auto_ptr
appears to check to see if the new value is the same as the old value and if it
is it does nothing. This does not appear to be the case for unique_ptr.
Changing the code to explicitly look for that case fixed the bug seen.

When the HadronizerFilter was changed to use unique_ptr from auto_ptr a
key difference between the two was missed. the method 'reset' on an auto_ptr
appears to check to see if the new value is the same as the old value and if it
is it does nothing. This does not appear to be the case for unique_ptr.
Changing the code to explicitly look for that case fixed the bug seen.
@cmsbuild
Copy link
Contributor

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

It involves the following packages:

GeneratorInterface/Core

@covarell, @perrozzi, @thuer, @cmsbuild, @davidlange6, @govoni can you please review it and eventually sign? Thanks.
@agrohsje, @mkirsano this is something you requested to watch as well.
@slava77, @Degano, @smuzaffar you are the release manager for this.

cms-bot commands are list here #13028

@Dr15Jones
Copy link
Contributor Author

This will fix (at least some) of the crashes in the IB. This was not tested to see if it fixes them all.

@Dr15Jones
Copy link
Contributor Author

@wmtan I think I have diagnosed the difference between std::unique_ptr and std::auto_ptr which caused the problem. I only reviewed HadronizerFilter.h for the problem so could you check over the other files you changed to see if they also might have a problem with reset?

@Dr15Jones
Copy link
Contributor Author

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 28, 2016

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/12708/console

@cmsbuild
Copy link
Contributor

-1

Tested at: 920d3e8

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

I found follow errors while testing this PR

Failed tests: RelVals AddOn

  • RelVals:

When I ran the RelVals I found an error in the following worklfows:
134.911 step2

runTheMatrix-results/134.911_RunSinglePh2015D+RunSinglePh2015D+HLTDR2_2016+RECODR2_2016reHLT+HARVESTDR2/step2_RunSinglePh2015D+RunSinglePh2015D+HLTDR2_2016+RECODR2_2016reHLT+HARVESTDR2.log
1306.0 step2
runTheMatrix-results/1306.0_SingleMuPt1_UP15+SingleMuPt1_UP15+DIGIUP15+RECOUP15+HARVESTUP15/step2_SingleMuPt1_UP15+SingleMuPt1_UP15+DIGIUP15+RECOUP15+HARVESTUP15.log
135.4 step1
runTheMatrix-results/135.4_ZEE_13+ZEEFS_13+HARVESTUP15FS+MINIAODMCUP15FS/step1_ZEE_13+ZEEFS_13+HARVESTUP15FS+MINIAODMCUP15FS.log
1330.0 step2
runTheMatrix-results/1330.0_ZMM_13+ZMM_13+DIGIUP15+RECOUP15+HARVESTUP15/step2_ZMM_13+ZMM_13+DIGIUP15+RECOUP15+HARVESTUP15.log
10021.0 step2
runTheMatrix-results/10021.0_TenMuE_0_200+TenMuE_0_200_pythia8_2017_GenSimFull+DigiFull_2017+RecoFull_2017+HARVESTFull_2017/step2_TenMuE_0_200+TenMuE_0_200_pythia8_2017_GenSimFull+DigiFull_2017+RecoFull_2017+HARVESTFull_2017.log
25202.0 step2
runTheMatrix-results/25202.0_TTbar_13+TTbar_13+DIGIUP15_PU25+RECOUP15_PU25+HARVESTUP15_PU25/step2_TTbar_13+TTbar_13+DIGIUP15_PU25+RECOUP15_PU25+HARVESTUP15_PU25.log
10024.0 step2
runTheMatrix-results/10024.0_TTbar_13+TTbar_13TeV_TuneCUETP8M1_2017_GenSimFull+DigiFull_2017+RecoFull_2017+HARVESTFull_2017/step2_TTbar_13+TTbar_13TeV_TuneCUETP8M1_2017_GenSimFull+DigiFull_2017+RecoFull_2017+HARVESTFull_2017.log
  • AddOn:

I found errors in the following addon tests:

cmsDriver.py RelVal -s L1REPACK:Full --data --scenario=pp -n 10 --conditions auto:run2_hlt_PRef --relval 9000,50 --datatier "RAW" --customise=HLTrigger/Configuration/CustomConfigs.L1T --era Run2_2016 --magField 38T_PostLS1 --eventcontent RAW --fileout file:RelVal_Raw_PRef_DATA.root --filein /store/data/Run2015D/MuonEG/RAW/v1/000/256/677/00000/80950A90-745D-E511-92FD-02163E011C5D.root : FAILED - time: date Fri Apr 29 00:47:51 2016-date Fri Apr 29 00:41:29 2016 s - exit: 16896
cmsDriver.py RelVal -s L1REPACK:Full --data --scenario=HeavyIons -n 10 --conditions auto:run2_hlt_HIon --relval 9000,50 --datatier "RAW" --eventcontent RAW --customise=HLTrigger/Configuration/CustomConfigs.L1T --era Run2_2016,Run2_HI --magField 38T_PostLS1 --fileout file:RelVal_Raw_HIon_DATA.root --filein /store/hidata/HIRun2015/HIHardProbes/RAW-RECO/HighPtJet-PromptReco-v1/000/263/689/00000/1802CD9A-DDB8-E511-9CF9-02163E0138CA.root : FAILED - time: date Fri Apr 29 00:47:17 2016-date Fri Apr 29 00:41:30 2016 s - exit: 16896
cmsDriver.py TTbar_Tauola_13TeV_cfi -s GEN,SIM,DIGI,L1,DIGI2RAW --mc --scenario=pp -n 10 --conditions auto:run2_mc_GRun --relval 9000,50 --datatier "GEN-SIM-RAW" --eventcontent RAWSIM --customise=HLTrigger/Configuration/CustomConfigs.L1T --era Run2_2016 --magField 38T_PostLS1 --fileout file:RelVal_Raw_GRun_MC.root : FAILED - time: date Fri Apr 29 00:52:46 2016-date Fri Apr 29 00:41:33 2016 s - exit: 16896
cmsDriver.py TTbar_Tauola_13TeV_cfi -s GEN,SIM,DIGI,L1,DIGI2RAW --mc --scenario=pp -n 10 --conditions auto:run2_mc_PRef --relval 9000,50 --datatier "GEN-SIM-RAW" --eventcontent RAWSIM --customise=HLTrigger/Configuration/CustomConfigs.L1T --era Run2_2016 --magField 38T_PostLS1 --fileout file:RelVal_Raw_PRef_MC.root : FAILED - time: date Fri Apr 29 00:50:09 2016-date Fri Apr 29 00:41:38 2016 s - exit: 16896
cmsDriver.py TTbar_13TeV_TuneCUETP8M1_cfi --conditions auto:run2_mc --fast -n 100 --eventcontent AODSIM,DQM --relval 100000,1000 -s GEN,SIM,RECOBEFMIX,DIGI:pdigi_valid,L1,DIGI2RAW,L1Reco,RECO,EI,HLT:@relval2016,VALIDATION --customise=HLTrigger/Configuration/CustomConfigs.L1THLT --datatier GEN-SIM-DIGI-RECO,DQMIO --beamspot NominalCollision2015 --era Run2_2016 --magField 38T_PostLS1 : FAILED - time: date Fri Apr 29 00:50:53 2016-date Fri Apr 29 00:41:39 2016 s - exit: 16896
cmsDriver.py RelVal -s L1REPACK:Full --data --scenario=pp -n 10 --conditions auto:run2_hlt_PIon --relval 9000,50 --datatier "RAW" --customise=HLTrigger/Configuration/CustomConfigs.L1T --era Run2_2016 --magField 38T_PostLS1 --eventcontent RAW --fileout file:RelVal_Raw_PIon_DATA.root --filein /store/data/Run2015D/MuonEG/RAW/v1/000/256/677/00000/80950A90-745D-E511-92FD-02163E011C5D.root : FAILED - time: date Fri Apr 29 00:47:29 2016-date Fri Apr 29 00:42:40 2016 s - exit: 16896
cmsDriver.py TTbar_Tauola_13TeV_cfi -s GEN,SIM,DIGI,L1,DIGI2RAW --mc --scenario=HeavyIons -n 10 --conditions auto:run2_mc_HIon --relval 9000,50 --datatier "GEN-SIM-RAW" --eventcontent RAWSIM --customise=HLTrigger/Configuration/CustomConfigs.L1T --era Run2_2016,Run2_HI --magField 38T_PostLS1 --fileout file:RelVal_Raw_HIon_MC.root : FAILED - time: date Fri Apr 29 00:57:29 2016-date Fri Apr 29 00:47:37 2016 s - exit: 16896
cmsDriver.py RelVal -s L1REPACK:Full --data --scenario=pp -n 10 --conditions auto:run2_hlt_GRun --relval 9000,50 --datatier "RAW" --eventcontent RAW --customise=HLTrigger/Configuration/CustomConfigs.L1T --era Run2_2016 --magField 38T_PostLS1 --fileout file:RelVal_Raw_GRun_DATA.root --filein /store/data/Run2015D/MuonEG/RAW/v1/000/256/677/00000/80950A90-745D-E511-92FD-02163E011C5D.root : FAILED - time: date Fri Apr 29 01:00:27 2016-date Fri Apr 29 00:50:45 2016 s - exit: 16896
cmsDriver.py TTbar_Tauola_13TeV_cfi -s GEN,SIM,DIGI,L1,DIGI2RAW --mc --scenario=pp -n 10 --conditions auto:run2_mc_PIon --relval 9000,50 --datatier "GEN-SIM-RAW" --eventcontent RAWSIM --customise=HLTrigger/Configuration/CustomConfigs.L1T --era Run2_2016 --magField 38T_PostLS1 --fileout file:RelVal_Raw_PIon_MC.root : FAILED - time: date Fri Apr 29 00:59:35 2016-date Fri Apr 29 00:51:18 2016 s - exit: 16896

@wmtan
Copy link
Contributor

wmtan commented Apr 29, 2016

@Dr15Jones I now fully understand the problem. You were completely right. All calls to unique_pt::reset() where the old ptr value and the new ptr value may be identical and non null must be protected against this or converted bac to auto_ptr.

@davidlange6 davidlange6 merged commit 6eb471a into cms-sw:CMSSW_8_1_X Apr 29, 2016
@Dr15Jones
Copy link
Contributor Author

@wmtan let's not go back to auto_ptr.

@Dr15Jones Dr15Jones deleted the fixHadronizerFilter branch April 29, 2016 11:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants