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

First attempt of premixing for MTD #26332

Merged
merged 2 commits into from Apr 8, 2019
Merged

Conversation

makortel
Copy link
Contributor

@makortel makortel commented Apr 3, 2019

PR description:

This PR makes the first attempt to implement premixing for MTD. As the SimHit accumulation approach is similar to HGCal, the premixing approach is as well similar. A few details are different-enough though so that no code can be easily re-used (e.g. "MTD cell" indexing uses row and column indices in addition to DetId, and the exact way to fill MTDCellInfo differs from HGCCellInfo).

Basically in stage1 MTDSimHitDataAccumulator is converted to PMTDSimAccumulator by packing each "non-zero" element MTDCellInfo to 11 bits.

Elements of MTDCellInfo[0](energy) are packed with logintpack::pack16log() to the range 1e-4..1e6 yielding ~1.3 % precision. I found this range by printing out values on 200 PU, so likely there is room for further tuning (from printouts I noticed that especially on the higher side the values seem to be quantized with much larger steps than 1.3 %).

Elements of MTDCellInfo[1] (time of flight) appeared to be limited roughly to the range of 0-26, so those values are packed linearly on that range.

PR validation:

Tested in 10_4_0_mtd5 (and then rebased on 10_6_0_pre3, compiled, and limited matrix runs). I have here comparison of 4D-vertices validation on 10 ttbar+PU35 events
http://mkdev7.cern.ch:8081/dqm/relval/start?runnr=1;dataset=/RelValTTbar_14/CMSSW_10_4_0_mtd5-PU35_2023d35_10ev_classical_v1-v1/DQMIO;sampletype=offline_relval;filter=all;referencepos=ratiooverlay;referenceshow=all;referencenorm=True;referenceobj1=other%3A%3A/RelValTTbar_14/CMSSW_10_4_0_mtd5-PU35_2023d35_10ev_premixing_v2-v1/DQMIO%3A%3A;referenceobj2=none;referenceobj3=none;referenceobj4=none;search=;striptype=object;stripruns=;stripaxis=run;stripomit=none;workspace=Everything;size=M;root=Vertexing/PrimaryVertexV/offlinePrimaryVertices4D;focus=Vertexing/PrimaryVertexV/offlinePrimaryVertices4D/RecoAllAssoc2GenMatched_PullY;zoom=no;
and (e.g. reco-only) distributions look reasonable
image
image

Further testing (larger statistics, lower-level variables) would be useful,

@kpedro88 @mdhildreth @fabiocos

@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 3, 2019

The code-checks are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 3, 2019

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-26332/9040

  • This PR adds an extra 32KB to repository

@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 3, 2019

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

It involves the following packages:

DataFormats/FTLDigi
RecoLocalFastTime/FTLRecProducers
SimFastTiming/Configuration
SimFastTiming/FastTimingCommon
SimGeneral/MixingModule
SimGeneral/PreMixingModule

@perrotta, @civanch, @mdhildreth, @cmsbuild, @kpedro88, @slava77 can you please review it and eventually sign? Thanks.
@rovere this is something you requested to watch as well.
@davidlange6, @slava77, @fabiocos you are the release manager for this.

cms-bot commands are listed here

@makortel
Copy link
Contributor Author

makortel commented Apr 3, 2019

@cmsbuild, please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 3, 2019

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/33921/console Started: 2019/04/03 11:08

value = cms.vstring( 'keep *_mix_FTLBarrel_*','keep *_mix_FTLEndcap_*','keep *_mix_InitialVertices_*' )
)
# For premixing switch the sim digi collections to the ones including pileup
# Unsure what to do with InitialVertices, they don't seem to be consumed downstream?
Copy link
Contributor

Choose a reason for hiding this comment

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

@lgray @pmeridian @fabiocos @casarsa please clarify the use (if any) of InitialVertices

SimFastTiming/FastTimingCommon/interface/MTDDigitizer.h Outdated Show resolved Hide resolved
@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 3, 2019

The code-checks are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 3, 2019

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-26332/9042

  • This PR adds an extra 32KB to repository

@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 3, 2019

Pull request #26332 was updated. @perrotta, @civanch, @mdhildreth, @cmsbuild, @kpedro88, @slava77 can you please check and sign again.

@makortel makortel mentioned this pull request Apr 4, 2019
@perrotta
Copy link
Contributor

perrotta commented Apr 4, 2019

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 4, 2019

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/33974/console Started: 2019/04/04 16:56

@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 4, 2019

@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 4, 2019

Comparison job queued.

@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 4, 2019

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

Comparison Summary:

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

@kpedro88
Copy link
Contributor

kpedro88 commented Apr 5, 2019

+upgrade

@perrotta
Copy link
Contributor

perrotta commented Apr 5, 2019

+1

  • Reco touched only in RecoLocalFastTime/FTLRecProducers/python/mtdUncalibratedRecHits_cfi.py, with some minor and logical modification
  • Jenkins test pass

@civanch
Copy link
Contributor

civanch commented Apr 5, 2019

+1

@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 5, 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 Apr 7, 2019

@casarsa FYI

@fabiocos
Copy link
Contributor

fabiocos commented Apr 8, 2019

+1

@cmsbuild cmsbuild merged commit 65d5af1 into cms-sw:master Apr 8, 2019
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