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

Build in ECAL precision time to PFClusters in timing era #15962

Merged
merged 2 commits into from Sep 26, 2016

Conversation

lgray
Copy link
Contributor

@lgray lgray commented Sep 22, 2016

This PR uses the ECAL precision timing modules to assign times to ECAL PFClusters.

This overwrites the usual time() in ECAL PFClusters with the value derived from the precision time digitizer.

New data formats saved, and updated time all show up in wf22424.0.

@bendavid

@cmsbuild
Copy link
Contributor

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

It involves the following packages:

IOMC/RandomEngine
RecoLocalCalo/EcalRecProducers
RecoParticleFlow/Configuration
RecoParticleFlow/PFClusterProducer
RecoParticleFlow/PFSimProducer

@smuzaffar, @Dr15Jones, @cvuosalo, @cmsbuild, @slava77, @davidlange6 can you please review it and eventually sign? Thanks.
@mmarionncern, @wmtan, @rafaellopesdesa, @wddgit, @argiro, @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

@lgray
Copy link
Contributor Author

lgray commented Sep 22, 2016

@cmsbuild please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 22, 2016

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

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

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

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

  • 10424.0_TTbar_13+TTbar_13TeV_TuneCUETP8M1_2017NewFPix_GenSimFull+DigiFull_2017NewFPix+RecoFull_2017NewFPix+HARVESTFull_2017NewFPix

@slava77
Copy link
Contributor

slava77 commented Sep 23, 2016

maybe this can be re-tested after #15939 is merged
(RelMon comparisons based on DQM will show up; the reco-tools based comparisons will be lacking until I update the map file, which I'd rather do after pre12 is made)

@lgray
Copy link
Contributor Author

lgray commented Sep 23, 2016

Let's see if that PR moves reasonably quickly. If it takes too long my preference is that this PR goes in soon.

@cvuosalo
Copy link
Contributor

+1

For #15962 f3f16c5

For Phase 2, writes ECAL precision timing to ECAL PFClusters.

The code changes are satisfactory, and Jenkins tests against baseline CMSSW_8_1_X_2016-09-22-0900 show no significant differences, as expected. A test of workflow 22424.0_TTbar_13+TTbar_13TeV_TuneCUETP8M1_2023D3Timing with 100 events also shows no significant differences and no significant change in CPU time. RECO event content shows the added timing data:

-----------------------------------------------------------------
   or, B         new, B      delta, B   delta, %   deltaJ, %    branch 
-----------------------------------------------------------------
      0.0 ->       261.4        261     NEWO   0.04     floatedmValueMap_ecalBarrelClusterFastTimer_PerfectResolutionModel_RECO.
      0.0 ->        46.4         46     NEWO   0.01     floatedmValueMap_ecalBarrelClusterFastTimer_PerfectResolutionModelResolution_RECO.
   6574.5 ->      6468.1       -106     -1.6  -0.01     recoPFClusters_particleFlowClusterECAL__RECO.
-------------------------------------------------------------
   740191 ->      740404        213             0.0     ALL BRANCHES

There is also a 1.6% decrease in the size of ECAL PF clusters, which may just be a fluctuation.

@@ -178,5 +178,14 @@
initialSeed = cms.untracked.uint32(1234567),
engineName = cms.untracked.string('HepJamesRandom')) )

eras.phase2_timing.toModify(RandomNumberGeneratorService,
trackTimeValueMapProducer = cms.PSet(initialSeed = cms.untracked.uint32(1234567), engineName = cms.untracked.string('HepJamesRandom') ) )
eras.phase2_timing.toModify(
Copy link
Contributor

Choose a reason for hiding this comment

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

@lgray - does this need to be in an era at all? (eg, is there harm in defining and not using?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@davidlange6 If it is always I guess it takes up unnecessary memory (not much but still)? What would be the benefits of just having it there all the time?

Copy link
Contributor

Choose a reason for hiding this comment

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

just code simplicity..

Copy link
Contributor

Choose a reason for hiding this comment

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

If you save the states of the random engine in the event, it is about 800 bytes per engine per event when using the HepJamesRandom type of engine. 2400 bytes if you instead use TRandom.

@davidlange6 davidlange6 merged commit f66010b into cms-sw:CMSSW_8_1_X Sep 26, 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

6 participants