Navigation Menu

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

MTD simulation: double readout of BTL crystal bars (cleaned-up version) #25470

Merged
merged 4 commits into from Dec 12, 2018

Conversation

casarsa
Copy link
Contributor

@casarsa casarsa commented Dec 11, 2018

This is a cleaned-up version of PR #25384. It addresses all comments and suggestions that were made during its review.

This pull request concerns the BTL simulation and marginally the BTL reconstruction code, which have been updated to deal with the case of the BTL bar geometry with double crystal readout. Some additional minor improvements have been introduced.
These are the main changes:

In SimFastTiming package:

- New digitizer for the BTL detector with the bar geometry:
    new BTLBarDeviceSim with readout at both sides of the crystal bar;
    BTLDeviceSim renamed to BTLTileDeviceSim;
    only the first two samples of the DataFrame are used, to store the two
    charges and times read out.
- Improved the BTL pulse shape.
- Implemented a parametrization of the BTL time resolution from the test
    beam results.

In RecoLocalFastTime package:

- The BTLUncalibratedRecHit has been updated to produce and save hits for both crystal
   readouts in the BTL bar geometry and a flag is set to keep track of different cases.
- The MTDRecHit producer has been updated to take into account all possible scenarios.

In order to have the proper digitizer selected, the Phase2_timing_layer_new era has
been replaced with Phase2_timing_layer_tile and Phase2_timing_layer_bar. For consistency,
Phase2C4_timing_layer_new has been renamed Phase2C4_timing_layer_bar.

This branch has been tested in all available MTD scenarios with SingleMuons (22407, 22807,
26607, 27007, 27407), MinBias (22440, 22840, 26640, 27040, 27440), TTbar (22434, 22834,
26634, 27034, 27434).

@cmsbuild
Copy link
Contributor

The code-checks are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @casarsa (Massimo Casarsa) for master.

It involves the following packages:

Configuration/Eras
Configuration/Geometry
Configuration/PyReleaseValidation
Configuration/StandardSequences
DataFormats/FTLDigi
DataFormats/FTLRecHit
DataFormats/ForwardDetId
Geometry/MTDCommonData
Geometry/MTDGeometryBuilder
RecoLocalFastTime/Configuration
RecoLocalFastTime/FTLCommonAlgos
RecoLocalFastTime/FTLRecProducers
SimFastTiming/Configuration
SimFastTiming/FastTimingCommon
SimGeneral/MixingModule

@perrotta, @cmsbuild, @prebello, @Dr15Jones, @cvuosalo, @civanch, @ianna, @mdhildreth, @pgunnell, @franzoni, @kpedro88, @zhenhu, @slava77, @fabiocos, @davidlange6 can you please review it and eventually sign? Thanks.
@vargasa, @makortel, @felicepantaleo, @GiacomoSguazzoni, @rovere, @VinInn, @Martin-Grunewald, @dgulhan 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

@fabiocos
Copy link
Contributor

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Dec 11, 2018

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/32119/console Started: 2018/12/11 13:27

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

Comparison job queued.

@cmsbuild
Copy link
Contributor

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

Comparison Summary:

  • No significant changes to the logs found
  • Reco comparison results: 2371 differences found in the comparisons
  • DQMHistoTests: Total files compared: 33
  • DQMHistoTests: Total histograms compared: 3136422
  • DQMHistoTests: Total failures: 1323
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3134895
  • DQMHistoTests: Total skipped: 204
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 32 files compared)
  • Checked 137 log files, 14 edm output root files, 33 DQM output files

@fabiocos
Copy link
Contributor

@kpedro88 as you see the update of the topology parameters in mtdParameters.xml has solved the issue of the warning messages. The geometry may be checked for consistency with #25471 .

@fabiocos
Copy link
Contributor

@prebello @zhenhu the changes in PyReleaseValidation are just related to the redefinition of eras to be used for the various upgrade scenarios, could you please check and sign them in case?

@casarsa
Copy link
Contributor Author

casarsa commented Dec 12, 2018

@slava77 All plots look consistent with the changes made in the MTD digitization code.
The new version has many improvements and fixes a couple of issues.

plot 1: The effect on the BTL time is due to a new reference pulse shape. The rising edge of the old
one was delayed by ~3.5 ns, squeezing the available time window and making time saturation
more frequent.

plot 2: Also the energy range extension is a consequence of the new BTL pulse and a fix in the
normalization of the reference BTL pulse.

plot 3: There should be no times below ~10 ns, the time that takes a particle to reach the endcap. In
the old code there was an issue with the filling of the DataFrame buckets: times > 25 ns were
not properly moved to the next bucket, after 25 ns had been subtracted.

plot 4: In the new code the number of ADC bits has been changed from 12 to 8. The ADC granularity
is coarser.

@fabiocos
Copy link
Contributor

@ianna @cvuosalo could you please check the few geometry related additions, and comment or sign them?

@casarsa
Copy link
Contributor Author

casarsa commented Dec 12, 2018

These are the TimeMemorySummary reports for step2 and step3 for 1000 TTbar events (WF 27434) run on an Intel Xeon E5-2640 v4 2.4 Ghz, 128 GB RAM:

step2:
MemoryReport> Peak virtual size 2763.55 Mbytes
Key events increasing vsize:
[30] run: 1 lumi: 1 event: 30 vsize = 2378.04 deltaVsize = 0.707031 rss = 1774.12 delta = 29.4102
[11] run: 1 lumi: 1 event: 11 vsize = 2377.33 deltaVsize = 0.800781 rss = 1744.71 delta = -7.21875
[419] run: 1 lumi: 1 event: 419 vsize = 2763.55 deltaVsize = 384 rss = 1842.47 delta = 97.7656
[0] run: 0 lumi: 0 event: 0 vsize = 0 deltaVsize = 0 rss = 0 delta = 0
[0] run: 0 lumi: 0 event: 0 vsize = 0 deltaVsize = 0 rss = 0 delta = 0
[421] run: 1 lumi: 1 event: 421 vsize = 2763.55 deltaVsize = 0 rss = 1831.41 delta = -11.0664
[420] run: 1 lumi: 1 event: 420 vsize = 2763.55 deltaVsize = 0 rss = 1834.85 delta = -7.62109
[419] run: 1 lumi: 1 event: 419 vsize = 2763.55 deltaVsize = 384 rss = 1842.47 delta = 14.9961
TimeReport> Time report complete in 10293.7 seconds
Time Summary:

  • Min event: 8.3538
  • Max event: 16.0874
  • Avg event: 9.88546
  • Total loop: 10232.8
  • Total init: 59.8359
  • Total job: 10293.7
  • EventSetup Lock: 9.67979e-05
  • EventSetup Get: 77.8538
    Event Throughput: 0.0977248 ev/s
    CPU Summary:
  • Total loop: 9870.63
  • Total init: 15.7726
  • Total extra: 0
  • Total job: 9887.44
    Processing Summary:
  • Number of Events: 1000
  • Number of Global Begin Lumi Calls: 1
  • Number of Global Begin Run Calls: 1

step3:
MemoryReport> Peak virtual size 4690.09 Mbytes
Key events increasing vsize:
[30] run: 1 lumi: 1 event: 30 vsize = 4049.09 deltaVsize = 0.707031 rss = 3251.82 delta = 23.4141
[11] run: 1 lumi: 1 event: 11 vsize = 4048.38 deltaVsize = 0.800781 rss = 3228.41 delta = 39.0703
[151] run: 1 lumi: 1 event: 151 vsize = 4689.79 deltaVsize = 640 rss = 3269.09 delta = 40.6875
[399] run: 1 lumi: 1 event: 399 vsize = 4690.09 deltaVsize = 0.308594 rss = 3272.18 delta = 3.08594
[0] run: 0 lumi: 0 event: 0 vsize = 0 deltaVsize = 0 rss = 0 delta = 0
[401] run: 1 lumi: 1 event: 401 vsize = 4690.09 deltaVsize = 0 rss = 3267.2 delta = -4.97656
[400] run: 1 lumi: 1 event: 400 vsize = 4690.09 deltaVsize = 0 rss = 3268.15 delta = -4.03125
[399] run: 1 lumi: 1 event: 399 vsize = 4690.09 deltaVsize = 0.308594 rss = 3272.18 delta = 3.08594
TimeReport> Time report complete in 3579.5 seconds
Time Summary:

  • Min event: 1.37755
  • Max event: 14.0361
  • Avg event: 3.22052
  • Total loop: 3478.16
  • Total init: 93.9541
  • Total job: 3579.5
  • EventSetup Lock: 0.000231743
  • EventSetup Get: 89.3455
    Event Throughput: 0.287508 ev/s
    CPU Summary:
  • Total loop: 3019.67
  • Total init: 115.667
  • Total extra: 0
  • Total job: 3142.63
    Processing Summary:
  • Number of Events: 1000
  • Number of Global Begin Lumi Calls: 1
  • Number of Global Begin Run Calls: 1

@slava77
Copy link
Contributor

slava77 commented Dec 12, 2018

+1

for #25470 6b5b020

@kpedro88
Copy link
Contributor

+upgrade

@fabiocos
Copy link
Contributor

@civanch I assume you confirm the previous signature, changes were technical fixes

@zhenhu
Copy link
Contributor

zhenhu commented Dec 12, 2018

+1

@civanch
Copy link
Contributor

civanch commented Dec 12, 2018

+1

@cmsbuild
Copy link
Contributor

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

+1

@cmsbuild cmsbuild merged commit cb25113 into cms-sw:master Dec 12, 2018
@casarsa casarsa deleted the mc-simBTL-develNEW branch December 13, 2018 09:20
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

7 participants