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

Hgc digithr g4cutrange #4551

Merged
merged 13 commits into from
Jul 14, 2014

Conversation

pfs
Copy link
Contributor

@pfs pfs commented Jul 8, 2014

Updates to HGCal digitization
Changing range cut in Geant4 in Si for e/gamma to 30microns (~1/3 Si width)
@vandreev11 Can you check as well?
@bsunanda Forgot to include you to watch this PR as well.

@cmsbuild
Copy link
Contributor

cmsbuild commented Jul 8, 2014

A new Pull Request was created by @pfs (Pedro Silva) for CMSSW_6_2_X_SLHC.

Hgc digithr g4cutrange

It involves the following packages:

Geometry/HGCalSimData
SimCalorimetry/HGCSimProducers
SimGeneral/MixingModule

The following packages do not have a category, yet:

Geometry/HGCalSimData
SimCalorimetry/HGCSimProducers

@civanch, @nclopezo, @mdhildreth, @cmsbuild, @Degano, @ktf can you please review it and eventually sign? Thanks.
@ghellwig, @wmtan this is something you requested to watch as well.
You can sign-off by replying to this message having '+1' in the first line of your reply.
You can reject by replying to this message having '-1' in the first line of your reply.
@fratnikov, @mark-grimes you are the release manager for this.
You can merge this pull request by typing 'merge' in the first line of your comment.

@mark-grimes
Copy link

Testing at the moment. @pfs is this going to affect memory use? I need to run some resource checks on the HGCal digitisation (relvals failed in digi, apparantly due to memory use). Just wondering whether I should test that before or after this pull request. Looks like you're relaxing cuts so presumably memory use would increase?


//handle sim hits
int maxSimHitsAccTime_;
int bxTime_;
HGCSimHitDataAccumulator simHitAccumulator_;
HGCSimHitDataAccumulator *simHitAccumulator_;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could this be made a std::unique_ptr? Or left as a standard data member. There is no corresponding delete for the new. Having said that I'm pretty sure only one HGCDigitizer is instantiated (is it?) so the leak isn't that big.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I experienced crashes at the end of the generation of ttbar events with it declared as data member.
But I think we can make a std::unique_ptr. Per job there are 3 HGCDigitizers, one for each HGC section (EE,HEF,HEB)

@mark-grimes
Copy link

Every single test, including non HGCal, fails in step3 with:

----- Begin Fatal Exception 08-Jul-2014 16:45:06 CEST-----------------------
An exception of category 'Configuration' occurred while
   [0] Constructing the EventProcessor
   [1] Constructing module: class=HGCalRecHitProducer label='HGCalRecHit'
Exception Message:
StatusMismatch: Parameter 'HGCEEmipInKeV' is designated as untracked in the code,
but is not designated as untracked in the configuration file.
Please change the configuration file to 'untracked <type> HGCEEmipInKeV'.
----- End Fatal Exception -------------------------------------------------

@pfs
Copy link
Contributor Author

pfs commented Jul 8, 2014

My bad. I forgot to propagate the removal of untracked to the RecHitProducers.
I have updated the branch and included also the change to std::unique_ptr proposed above.

@cmsbuild
Copy link
Contributor

cmsbuild commented Jul 8, 2014

Pull request #4551 was updated. @civanch, @nclopezo, @mdhildreth, @cmsbuild, @Degano, @ktf can you please check and sign again.

@mark-grimes
Copy link

Only some Phase 1 scenarios pass. HGCal scenarios fail in step 2 with

----- Begin Fatal Exception 10-Jul-2014 15:06:58 CEST-----------------------
An exception of category 'Configuration' occurred while
   [0] Constructing the EventProcessor
   [1] Constructing module: class=MixingModule label='mix'
Exception Message:
StatusMismatch: ParameterSet 'digiCfg' is designated as tracked in the code,
but is designated as untracked in the configuration file.
Please remove 'untracked' from the configuration file for parameter 'digiCfg'.
----- End Fatal Exception -------------------------------------------------

The other Phase2 scenarios (and some Phase1) fail in step 3 with

----- Begin Fatal Exception 10-Jul-2014 14:49:21 CEST-----------------------
An exception of category 'Configuration' occurred while
   [0] Constructing the EventProcessor
   [1] Constructing module: class=HGCalRecHitProducer label='HGCalRecHit'
Exception Message:
StatusMismatch: Parameter 'HGCEEmipInKeV' is designated as tracked in the code,
but is designated as untracked in the configuration file.
Please remove 'untracked' from the configuration file for parameter 'HGCEEmipInKeV'.
----- End Fatal Exception -------------------------------------------------

Can you try a few of the workflows to make sure everything is okay? E.g. a simple set with Phase1, HGCal and Shashlik is

runTheMatrix.py -w upgrade -l 11200,12000,12400,14200

@vandreev11
Copy link
Contributor

@mark-grimes, @pfs, @bsunanda Mark, I just had a run with today's IB (07-10-0200) plus this PR
on the following workflows -l 11200, 12000, 12400, 14200, 12200. They passed all steps:
11200_FourMuPt1_200+FourMuPt_1_200_2019WithGEM_GenSimFull+DigiFull_2019WithGEM+RecoFull_2019WithGEM+HARVESTFull_2019WithGEM Step0-PASSED Step1-PASSED Step2-PASSED Step3-PASSED - time date Thu Jul 10 17:46:12 2014-date Thu Jul 10 17:34:38 2014; exit: 0 0 0 0
12000_FourMuPt1_200+FourMuPt_1_200_Extended2023_GenSimFull+DigiFull_Extended2023+RecoFull_Extended2023+HARVESTFull_Extended2023 Step0-PASSED Step1-PASSED Step2-PASSED Step3-PASSED - time date Thu Jul 10 17:46:41 2014-date Thu Jul 10 17:34:43 2014; exit: 0 0 0 0
12200_FourMuPt1_200+FourMuPt_1_200_Extended2023HGCalMuon_GenSimFull+DigiFull_Extended2023HGCalMuon+RecoFull_Extended2023HGCalMuon+HARVESTFull_Extended2023HGCalMuon Step0-PASSED Step1-PASSED Step2-PASSED Step3-PASSED - time date Thu Jul 10 17:47:12 2014-date Thu Jul 10 17:34:47 2014; exit: 0 0 0 0
12400_FourMuPt1_200+FourMuPt_1_200_Extended2023SHCal_GenSimFull+DigiFull_Extended2023SHCal+RecoFull_Extended2023SHCal+HARVESTFull_Extended2023SHCal Step0-PASSED Step1-PASSED Step2-PASSED Step3-PASSED - time date Thu Jul 10 17:47:44 2014-date Thu Jul 10 17:34:48 2014; exit: 0 0 0 0
14200_FourMuPt1_200+FourMuPt_1_200_Extended2023HGCal_GenSimFull+DigiFull_Extended2023HGCal+RecoFull_Extended2023HGCal+HARVESTFull_Extended2023HGCal Step0-PASSED Step1-PASSED Step2-PASSED Step3-PASSED - time date Thu Jul 10 17:55:21 2014-date Thu Jul 10 17:46:20 2014; exit: 0 0 0 0
5 5 5 5 tests passed, 0 0 0 0 failed

@vandreev11
Copy link
Contributor

@mark-grimes, @pfs, @bsunanda There is a crash at step2 with 14816 workflow (QCD tith V4Muon, V4MuonReco) with
some segmentation violation - looks to me from SiliconStrip detector:
#16 0x00002b8a19228224 in std::_Rb_tree<unsigned short, std::pair<unsigned short const, unsigned short>, std::_Select1st<std::pair<unsigned short const, unsigned short> >, std::less, std::allocator<std::pair<unsigned short const, unsigned short> > >::M_erase(std::Rb_tree_node<std::pair<unsigned short const, unsigned short> >) () from /afs/cern.ch/cms/sw/ReleaseCandidates/vol0/slc5_amd64_gcc472/cms/cmssw/CMSSW_6_2_X_SLHC_2014-07-10-0200/lib/slc5_amd64_gcc472/libDataFormatsStdDictionaries.so
#17 0x00002b8a1b22cdff in Reflex::FunctionMember::Invoke(Reflex::Object const&, Reflex::Object
, std::vector<void*, std::allocator<void*> > const&) const () from /afs/cern.ch/cms/sw/ReleaseCandidates/vol0/slc5_amd64_gcc472/cms/cmssw/CMSSW_6_2_X_SLHC_2014-07-10-0200/external/slc5_amd64_gcc472/lib/libReflex.so
#18 0x00002b8a1b218de8 in Reflex::Class::Destruct(void*, bool) const () from /afs/cern.ch/cms/sw/ReleaseCandidates/vol0/slc5_amd64_gcc472/cms/cmssw/CMSSW_6_2_X_SLHC_2014-07-10-0200/external/slc5_amd64_gcc472/lib/libReflex.so
#19 0x00002b8a305a8fde in boost::detail::sp_counted_base::release() () from /afs/cern.ch/cms/sw/ReleaseCandidates/vol0/slc5_amd64_gcc472/cms/cmssw/CMSSW_6_2_X_SLHC_2014-07-10-0200/lib/slc5_amd64_gcc472/pluginCondCoreSiStripPlugins.so
#20 0x00002b8a305a9850 in cond::PayloadProxy::~PayloadProxy() () from /afs/cern.ch/cms/sw/ReleaseCandidates/vol0/slc5_amd64_gcc472/cms/cmssw/CMSSW_6_2_X_SLHC_2014-07-10-0200/lib/slc5_amd64_gcc472/pluginCondCoreSiStripPlugins.so
#21 0x00002b8a305a8fde in boost::detail::sp_counted_base::release() () from /afs/cern.ch/cms/sw/ReleaseCandidates/vol0/slc5_amd64_gcc472/cms/cmssw/CMSSW_6_2_X_SLHC_2014-07-10-0200/lib/slc5_amd64_gcc472/pluginCondCoreSiStripPlugins.so
#22 0x00002b8a305a9eed in DataProxy<SiStripLorentzAngleSimRcd, SiStripLorentzAngle>::~DataProxy() () from /afs/cern.ch/cms/sw/ReleaseCandidates/vol0/slc5_amd64_gcc472/cms/cmssw/CMSSW_6_2_X_SLHC_2014-07-10-0200/lib/slc5_amd64_gcc472/pluginCondCoreSiStripPlugins.so
#23 0x00002b8a305a9f39 in DataProxy<SiStripLorentzAngleSimRcd, SiStripLorentzAngle>::~DataProxy() () from /afs/cern.ch/cms/sw/ReleaseCandidates/vol0/slc5_amd64_gcc472/cms/cmssw/CMSSW_6_2_X_SLHC_2014-07-10-0200/lib/slc5_amd64_gcc472/pluginCondCoreSiStripPlugins.so

@vandreev11
Copy link
Contributor

@mark-grimes, @pfs, @bsunanda -l 12216 (HGC v5, QCD) gives crash at step2 in HGCalDigitizer with complaint on memory allocation:
#25 0x00002ba9c2a8db9a in HGCDigitizer::~HGCDigitizer() () from /afs/cern.ch/work/v/vandreev/private/HGC/PullRequests/CMSSW_6_2_X_SLHC_2014-07-10-0200/lib/slc5_amd64_gcc472/libSimCalorimetryHGCSimProducers.so
#26 0x00002ba9c29c5af8 in HGCDigiProducer::~HGCDigiProducer() () from /afs/cern.ch/work/v/vandreev/private/HGC/PullRequests/CMSSW_6_2_X_SLHC_2014-07-10-0200/lib/slc5_amd64_gcc472/pluginSimCalorimetryHGCSimProducersAuto.so
#27 0x00002ba9c138cd76 in edm::MixingModule::~MixingModule() () from /afs/cern.ch/work/v/vandreev/private/HGC/PullRequests/CMSSW_6_2_X_SLHC_2014-07-10-0200/lib/slc5_amd64_gcc472/pluginSimGeneralMixingModulePlugins.so
#28 0x00002ba9c138ce99 in edm::MixingModule::~MixingModule() () from /afs/cern.ch/work/v/vandreev/private/HGC/PullRequests/CMSSW_6_2_X_SLHC_2014-07-10-0200/lib/slc5_amd64_gcc472/pluginSimGeneralMixingModulePlugins.so
#29 0x00002ba9a69f5a74 in edm::WorkerTedm::EDProducer::~WorkerT() () from /afs/cern.ch/cms/sw/ReleaseCandidates/vol0/slc5_amd64_gcc472/cms/cmssw/CMSSW_6_2_X_SLHC_2014-07-10-0200/lib/slc5_amd64_gcc472/libFWCoreFramework.so

@pfs
Copy link
Contributor Author

pfs commented Jul 11, 2014

@mark-grimes @vandreev11 @bsunanda I think i understand why it's crashing, and I can now run the QCD generation without problems. I'm doing one final cross check with igprof.

…r.cc

Moved data members to unique_ptr which are instantiated only if the digitizer is in charge of a sub-detector
(previously they where all being instantiated even if only one would be used)
@cmsbuild
Copy link
Contributor

Pull request #4551 was updated. @civanch, @nclopezo, @mdhildreth, @cmsbuild, @Degano, @ktf can you please check and sign again.

@pfs
Copy link
Contributor Author

pfs commented Jul 11, 2014

I have updated the PR. I added some changes, moving data members to std::unique_ptr but that was not the fix. It was related to this line here only

PFCal-dev@ba6bb59#diff-2f822c20cc09dbcda00cee6fb3a20892R174

I ran igprof on the QCD sample and i put the memory profile reports here, in case you want to take a look:

/afs/cern.ch/user/p/psilva/public/PR4551

@pfs pfs closed this Jul 11, 2014
@pfs pfs reopened this Jul 11, 2014
@pfs
Copy link
Contributor Author

pfs commented Jul 11, 2014

@mark-grimes @vandreev11 @bsunanda I clicked on s.th. i shouldn't have clicked... sorry for the noise. The PR is now re-opened.

@pfs
Copy link
Contributor Author

pfs commented Jul 11, 2014

@mark-grimes @lgray @vandreev11 @bsunanda this now includes the optimization of the sim hit data accumulator discussed in some private thread

@cmsbuild
Copy link
Contributor

Pull request #4551 was updated. @civanch, @nclopezo, @mdhildreth, @cmsbuild, @Degano, @ktf can you please check and sign again.

@lgray
Copy link
Contributor

lgray commented Jul 11, 2014

@pfs @mark-grimes Now that we removed whatever the issue was for the memory footprint, you can probably switch back to using doubles if you want... Doubt it is needed though.

@pfs
Copy link
Contributor Author

pfs commented Jul 11, 2014

@lgray doubles are not needed, indeed. I just compared the ADC spectra for single pions and they are similar.

@lgray
Copy link
Contributor

lgray commented Jul 11, 2014

@pfs bon

@mark-grimes
Copy link

merge

Tested with muons and pions, no problems. Haven't checked the memory yet but will do once it's in an IB.

cmsbuild added a commit that referenced this pull request Jul 14, 2014
@cmsbuild cmsbuild merged commit d78e022 into cms-sw:CMSSW_6_2_X_SLHC Jul 14, 2014
@pfs pfs deleted the hgc_digithr_g4cutrange branch November 18, 2021 21:44
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

5 participants