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

SimClustering for HGCal #15293

Merged
merged 17 commits into from Aug 11, 2016
Merged

SimClustering for HGCal #15293

merged 17 commits into from Aug 11, 2016

Conversation

lgray
Copy link
Contributor

@lgray lgray commented Jul 26, 2016

This PR implements GEANT-based gathering of HGCal RecHits into "SimClusters".

This enables, with a full GReco era, the possibility of a full global reconstruction with the HGCal detector in place.

A "sim-particle flow" to complement this sim clusters is implemented in this PR.

The collection "particleFlowClusterHGCal" is added to the local reconstruction for HGCal.

reco::PFCandidates are now also made in simPFProducer.

Another PR will introduce the simPFProducer into the global reconstruction sequence when running the HGCal era.

@lgray
Copy link
Contributor Author

lgray commented Jul 26, 2016

@cmsbuild please test

@lgray
Copy link
Contributor Author

lgray commented Jul 26, 2016

@kpedro88

@cmsbuild
Copy link
Contributor

cmsbuild commented Jul 26, 2016

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

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @lgray (Lindsey Gray) for CMSSW_8_1_X.

It involves the following packages:

DataFormats/HGCDigi
DataFormats/HGCRecHit
Fireworks/Geometry
Geometry/HGCalCommonData
RecoLocalCalo/Configuration
RecoLocalCalo/HGCalRecProducers
RecoParticleFlow/Configuration
RecoParticleFlow/PFClusterProducer
SimCalorimetry/HGCalSimProducers
SimDataFormats/CaloAnalysis
SimGeneral/CaloAnalysis
SimGeneral/Configuration
SimGeneral/MixingModule

The following packages do not have a category, yet:

SimDataFormats/CaloAnalysis
SimGeneral/CaloAnalysis

@civanch, @Dr15Jones, @cvuosalo, @ianna, @mdhildreth, @cmsbuild, @alja, @slava77, @davidlange6 can you please review it and eventually sign? Thanks.
@ghellwig, @vandreev11, @sethzenz, @wmtan, @mmarionncern, @rafaellopesdesa, @kpedro88, @cseez, @alja, @pfs, @cbernet, @bachtis this is something you requested to watch as well.
@slava77, @smuzaffar you are the release manager for this.

cms-bot commands are list here #13028

@cmsbuild
Copy link
Contributor

-1

Tested at: 6e4d0d5

You can see the results of the tests here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-15293/14241/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:
5.1 step1

runTheMatrix-results/5.1_TTbar+TTbarFS+HARVESTFS/step1_TTbar+TTbarFS+HARVESTFS.log
135.4 step1
runTheMatrix-results/135.4_ZEE_13+ZEEFS_13+HARVESTUP15FS+MINIAODMCUP15FS/step1_ZEE_13+ZEEFS_13+HARVESTUP15FS+MINIAODMCUP15FS.log
  • AddOn:

I found errors in the following addon tests:

cmsDriver.py TTbar_8TeV_TuneCUETP8M1_cfi --conditions auto:run1_mc --fast -n 100 --eventcontent AODSIM,DQM --relval 100000,1000 -s GEN,SIM,RECOBEFMIX,DIGI:pdigi_valid,L1,DIGI2RAW,L1Reco,RECO,EI,HLT:@Fake,VALIDATION --customise=HLTrigger/Configuration/CustomConfigs.L1THLT --datatier GEN-SIM-DIGI-RECO,DQMIO --beamspot Realistic8TeVCollision : FAILED - time: date Tue Jul 26 23:40:07 2016-date Tue Jul 26 23:40:05 2016 s - exit: 256
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:@relval25ns,VALIDATION --customise=HLTrigger/Configuration/CustomConfigs.L1THLT --datatier GEN-SIM-DIGI-RECO,DQMIO --beamspot NominalCollision2015 --era Run2_25ns --magField 38T_PostLS1 : FAILED - time: date Tue Jul 26 23:40:20 2016-date Tue Jul 26 23:40:18 2016 s - exit: 256
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 Tue Jul 26 23:40:28 2016-date Tue Jul 26 23:40:26 2016 s - exit: 256

@slava77
Copy link
Contributor

slava77 commented Aug 8, 2016

Some plots to go along:
(black line histogram is for this PR; red is for baseline [where appropriate])

about 14K HGCal pf clusters per event
all_origvssign741_ttbar_13_2023lreco_pu200c_recopfclusters_particleflowclusterhgcal__reco_objat_size

all_origvssign741_ttbar_13_2023lreco_pu200c_log10recopfclusters_particleflowclusterhgcal__reco_obj_energy
about 2.5% have energy=0

I suppose these 0-energy clusters are what defines the bump in the phi distribution
all_origvssign741_ttbar_13_2023lreco_pu200c_recopfclusters_particleflowclusterhgcal__reco_obj_phi

Do we need these 0-energy clusters?

@lgray
Copy link
Contributor Author

lgray commented Aug 8, 2016

@slava77 0-energy clusters are needed to keep 1:1 mapping back to the sim particles. For downstream, these can be cut out simply while still retaining the trail of crumbs back to the simulated info.

Can you make these plots with energy() > 0 ?

@slava77
Copy link
Contributor

slava77 commented Aug 8, 2016

timing (The same PU200 running LReco):

  • step2 didn't change significantly, but the uncertainty on mix module time is large (partly driven by file IO)
  • step3 has new modules as expected
       added      +0.58%         0.00 ms/ev ->       186.42 ms/ev particleFlowRecHitHGC
       added      +2.78%         0.00 ms/ev ->       898.75 ms/ev particleFlowClusterHGCal

Event size changes are roughly as expected:

  • step2 collection names changed and two collections were added with 4.2% increase in the file size
size    % of total    branchName
          2592350        3.54     SimClusters_mix_MergedCaloTruth_HLT.
           779282        1.06     CaloParticles_mix_MergedCaloTruth_HLT.
 73296217 ->    76339261    3043043             4.2     ALL BRANCHES
  • step3 PF HGCal collections show up and increase the output size by ~60% (from 5.8 MB to 9.1MB per event; this is FEVTDEBUGHLT event content):
      3353990        57.37     recoPFClusters_particleFlowClusterHGCal__RECO.
      330        0.01     recoPFRecHits_particleFlowRecHitHGC_Cleaned_RECO.

@slava77
Copy link
Contributor

slava77 commented Aug 8, 2016

+1

for #15293 ac9836e

  • changes in the code are in line with the PR description and the follow up review
  • jenkins tests pass and comparisons with baseline show no differences (the only 2023 test in jenkins is GReco which doesn't have HGCAL).
  • tested locally with more performance tests in PU200 TTbar. See notes earlier in the thread.
    • memory increases in step2 by almost 30 MB/thread and in step3 by almost 60 MB/thread (the test was made in 16 threads; there is an off-chance that a fraction of this increase is independent of nThreads)

@kpedro88
Copy link
Contributor

kpedro88 commented Aug 8, 2016

@slava77 LReco is also in jenkins as a PR test and does have HGCal - you can at least confirm that the desired output products are there...

@kpedro88
Copy link
Contributor

kpedro88 commented Aug 8, 2016

@ianna @civanch @alja please review/sign ASAP

@slava77
Copy link
Contributor

slava77 commented Aug 8, 2016

On 8/8/16 5:06 PM, Kevin Pedro wrote:

@slava77 https://github.com/slava77 LReco is also in jenkins as a PR
test and does have HGCal - you can at least confirm that the desired
output products are there...

jenkins comparisons lost 11024.0 in the last commit tests


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#15293 (comment), or
mute the thread
https://github.com/notifications/unsubscribe-auth/AEdcbhq_OdZfSc364HdEEl2YtkLrWh7Pks5qd6hbgaJpZM4JVi3o.

@kpedro88
Copy link
Contributor

kpedro88 commented Aug 8, 2016

ah, I see, the test was run but the comparison isn't there. Oh well.

@lgray
Copy link
Contributor Author

lgray commented Aug 9, 2016

@ianna @civanch @alja please review and sign.

@mdhildreth
Copy link
Contributor

+1

@lgray
Copy link
Contributor Author

lgray commented Aug 10, 2016

@alja please review and sign.

@kpedro88
Copy link
Contributor

@davidlange6 please merge whenever possible

@alja
Copy link
Contributor

alja commented Aug 11, 2016

+1
Sorry for the delay. I'm on the vacations this week.

@cmsbuild
Copy link
Contributor

This pull request is fully signed and it will be integrated in one of the next CMSSW_8_1_X IBs (tests are also fine). This pull request requires discussion in the ORP meeting before it's merged. @slava77, @davidlange6, @smuzaffar

@davidlange6
Copy link
Contributor

+1

@lgray @kpedro88 -if this code has much lifetime, please look at using eigen instead of TPrincipal for the PCA work. Thanks

@cmsbuild cmsbuild merged commit 5ab9bcf into cms-sw:CMSSW_8_1_X Aug 11, 2016
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

10 participants