[nT2] rechits reconstruction from digis
PR description:
In this PR we introduce the Totem (for now, new T2-only) geometry-DetId mapping condition object, and the construction of a T2 rechits collection from the digis collection recently introduced in #38886.
In order to avoid yet another duplication of pre-existing digi-rechits conversion code, the algorithmic part of this latter was transformed into a TimingRecHitProducerAlgorithm base class from which the pre-existing CTPPSDiamondRecHitProducerAlgorithm, TotemTimingRecHitProducerAlgorithm, and the newly-introduced TotemT2RecHitProducerAlgorithm inheritate.
Most of the development history has been stashed in a few minimally-working commits, and follow-up developments were rebased on top of this branch.
PR validation:
Code compiles and runs on top of a digi-level sample. Matrix tests succede.
If this PR is a backport please specify the original PR and why you need to backport that PR. If this PR will be backported please specify to which release cycle the backport is meant for: N/A
+code-checks
Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-38986/31440
- This PR adds an extra 76KB to repository
A new Pull Request was created by @forthommel (Laurent Forthomme) for master.
It involves the following packages:
- DataFormats/TotemReco (reconstruction)
- Geometry/ForwardGeometry (geometry)
- Geometry/Records (geometry)
- RecoPPS/Local (reconstruction)
@civanch, @Dr15Jones, @makortel, @ianna, @clacaputo, @cmsbuild, @bsunanda, @mdhildreth, @jpata can you please review it and eventually sign? Thanks. @bsunanda, @rovere, @fabiocos, @grzanka this is something you requested to watch as well. @perrotta, @dpiparo, @qliphy, @rappoccio you are the release manager for this.
cms-bot commands are listed here
please test
+1
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-5000cf/26688/summary.html
COMMIT: a4dc9da37399593fc432403a013473a1489809c4
CMSSW: CMSSW_12_5_X_2022-08-07-2300/el8_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/38986/26688/install.sh to create a dev area with all the needed externals and cmssw changes.
Comparison Summary
Summary:
- No significant changes to the logs found
- Reco comparison results: 4 differences found in the comparisons
- DQMHistoTests: Total files compared: 51
- DQMHistoTests: Total histograms compared: 3691612
- DQMHistoTests: Total failures: 8
- DQMHistoTests: Total nulls: 0
- DQMHistoTests: Total successes: 3691582
- DQMHistoTests: Total skipped: 22
- DQMHistoTests: Total Missing objects: 0
- DQMHistoSizes: Histogram memory added: 0.0 KiB( 50 files compared)
- Checked 212 log files, 49 edm output root files, 51 DQM output files
- TriggerResults: no differences found
Hi @forthommel , is this PR planned to be backported into 12_4_X?
Hi @clacaputo ! No, it will only used under specific pp data-taking conditions in ~october-november, and for HI runs later on. So, 12_5_X is all fine.
+code-checks
Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-38986/31512
- This PR adds an extra 32KB to repository
Pull request #38986 was updated. @civanch, @Dr15Jones, @makortel, @ianna, @clacaputo, @cmsbuild, @bsunanda, @mdhildreth, @jpata can you please check and sign again.
+code-checks
Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-38986/31513
- This PR adds an extra 36KB to repository
Pull request #38986 was updated. @civanch, @Dr15Jones, @makortel, @ianna, @clacaputo, @cmsbuild, @bsunanda, @mdhildreth, @jpata can you please check and sign again.
Ready for another batch of reviews and/or a please test.
@cmsbuild please test
Thanks, @clacaputo ! Although the tests can now be aborted, I found an obvious problem with the new TimingRecHitsProducerAlgorithm which is about to be solved in a follow-up push.
-1
Failed Tests: RelVals RelVals-INPUT AddOn
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-5000cf/26753/summary.html
COMMIT: 2e0936e8befe0652fbd391719826fb83be2b3946
CMSSW: CMSSW_12_5_X_2022-08-10-2300/el8_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/38986/26753/install.sh to create a dev area with all the needed externals and cmssw changes.
RelVals
----- Begin Fatal Exception 11-Aug-2022 12:31:16 CEST-----------------------
An exception of category 'Configuration' occurred while
[0] Constructing the EventProcessor
[1] Constructing module: class=TotemTimingRecHitProducer label='totemTimingRecHits'
Exception Message:
MissingParameter: Parameter 'timeSliceNs' not found.
----- End Fatal Exception -------------------------------------------------
----- Begin Fatal Exception 11-Aug-2022 12:32:55 CEST-----------------------
An exception of category 'Configuration' occurred while
[0] Constructing the EventProcessor
[1] Constructing module: class=TotemTimingRecHitProducer label='totemTimingRecHits'
Exception Message:
MissingParameter: Parameter 'timeSliceNs' not found.
----- End Fatal Exception -------------------------------------------------
----- Begin Fatal Exception 11-Aug-2022 12:33:17 CEST-----------------------
An exception of category 'Configuration' occurred while
[0] Constructing the EventProcessor
[1] Constructing module: class=TotemTimingRecHitProducer label='totemTimingRecHits'
Exception Message:
MissingParameter: Parameter 'timeSliceNs' not found.
----- End Fatal Exception -------------------------------------------------
Expand to see more relval errors ...
RelVals-INPUT
- 138.4
138.4_PromptCollisions+RunMinimumBias2021+ALCARECOPROMPTR3+HARVESTDPROMPTR3/step2_PromptCollisions+RunMinimumBias2021+ALCARECOPROMPTR3+HARVESTDPROMPTR3.log - 136.9
136.9_RunDoubleMuon2016C+RunDoubleMuon2016C+RAWRECOTE16+RAWRECOLHECLEANTE16+EMBEDHADTE16+EMBEDMINIAOD16/step2_RunDoubleMuon2016C+RunDoubleMuon2016C+RAWRECOTE16+RAWRECOLHECLEANTE16+EMBEDHADTE16+EMBEDMINIAOD16.log - 138.5
138.5_ExpressCollisions+RunMinimumBias2021+TIER0EXPRUN3+ALCARECOEXPR3+HARVESTDEXPR3/step2_ExpressCollisions+RunMinimumBias2021+TIER0EXPRUN3+ALCARECOEXPR3+HARVESTDEXPR3.log
Expand to see more relval errors ...
AddOn Tests
----- Begin Fatal Exception 11-Aug-2022 12:34:57 CEST-----------------------
An exception of category 'Configuration' occurred while
[0] Constructing the EventProcessor
[1] Constructing module: class=TotemTimingRecHitProducer label='totemTimingRecHits'
Exception Message:
MissingParameter: Parameter 'timeSliceNs' not found.
----- End Fatal Exception -------------------------------------------------
----- Begin Fatal Exception 11-Aug-2022 12:36:58 CEST-----------------------
An exception of category 'Configuration' occurred while
[0] Constructing the EventProcessor
[1] Constructing module: class=TotemTimingRecHitProducer label='totemTimingRecHits'
Exception Message:
MissingParameter: Parameter 'timeSliceNs' not found.
----- End Fatal Exception -------------------------------------------------
----- Begin Fatal Exception 11-Aug-2022 12:35:57 CEST-----------------------
An exception of category 'Configuration' occurred while
[0] Constructing the EventProcessor
[1] Constructing module: class=TotemTimingRecHitProducer label='totemTimingRecHits'
Exception Message:
MissingParameter: Parameter 'timeSliceNs' not found.
----- End Fatal Exception -------------------------------------------------
Expand to see more addon errors ...
+code-checks
Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-38986/31517
- This PR adds an extra 96KB to repository
Pull request #38986 was updated. @civanch, @Dr15Jones, @makortel, @ianna, @clacaputo, @cmsbuild, @bsunanda, @mdhildreth, @jpata can you please check and sign again.
@cmsbuild please test
+1
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-5000cf/26757/summary.html
COMMIT: a064d03e3052ac80c8698779ef196847b86110cf
CMSSW: CMSSW_12_5_X_2022-08-11-1100/el8_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/38986/26757/install.sh to create a dev area with all the needed externals and cmssw changes.
Comparison Summary
Summary:
- No significant changes to the logs found
- Reco comparison results: 2 differences found in the comparisons
- DQMHistoTests: Total files compared: 51
- DQMHistoTests: Total histograms compared: 3692476
- DQMHistoTests: Total failures: 8
- DQMHistoTests: Total nulls: 0
- DQMHistoTests: Total successes: 3692446
- DQMHistoTests: Total skipped: 22
- DQMHistoTests: Total Missing objects: 0
- DQMHistoSizes: Histogram memory added: 0.0 KiB( 50 files compared)
- Checked 212 log files, 49 edm output root files, 51 DQM output files
- TriggerResults: no differences found
@cms-sw/reconstruction-l2, @cms-sw/geometry-l2, any plan to tackle this PR in the forthcoming days?
Vacations affect our review turnaround time during summer, but I think it's on track for the next release.
- Which workflows does the new code run in?
- Are any reco changes expected?
- What is the CPU/mem profiling performance of the new code?
Many thanks for your fast reaction, @jpata! And indeed I know the Summer can certainly affect the various workflows, hence my kind "ping" :) No workflow currently contain this added reconstruction scheme, as the detector is not yet integrated into cDAQ and we can only rely on simulated frames. Consequently, no reco changes are expected for standard WFs, and CPU/memory profiling is relatively difficult to evaluate outside thumbrule/scaling (only 64 readout channels are to be unpacked for low-PU end-of-year conditions). However, we can surely provide a recipe to steer the newly-introduced rechits producer algorithm, and we are more than open to discuss the rechits data format if necessary.
No workflow currently contain this added reconstruction scheme, as the detector is not yet integrated into cDAQ and we can only rely on simulated frames.
It might be useful to consider some kind of regular testing (e.g. a sim-only workflow) for this.
Can you assess the impact on the data size?
@forthommel , you will also nee to squash commits at the end.
@civanch I would suggest to edit the FAQ to match with the current release managers mindset where the squashing, effectively destroying the development history of a full project, is enforced. For this particular feature development, we already collapsed 50+ commits into about 10 self-contained ones before review, but it seems that your strategy remains a mystery (at least for me).
@forthommel , I did not wrote FAQ and would not change there anything myself. What is important - initialize class members at construction as @jpata suggested.
I also guess nT2 data is relatively small, even if all scintillators will be fired.
+code-checks
Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-38986/31644
- This PR adds an extra 88KB to repository
Pull request #38986 was updated. @civanch, @Dr15Jones, @makortel, @ianna, @clacaputo, @cmsbuild, @bsunanda, @mdhildreth, @jpata, @mandrenguyen can you please check and sign again.
Thanks! I just zero-initialised the rechits members in the last push.
For the rate, based on the fact the nT2 unpacking will be made under low-intensity (PU << 1) conditions, each event will deliver at most 8 (planes) * 4 (channels) * 2 (arms) = 64 rechits, with 3 (GlobalPoint) + 3 (time, time uncertainty, tot) = 6 floats/rechit.
Hence the maximal raw (uncompressed, prior to L1/HLT) is 64 * 6 * 4 (bytes/float on 64-bit) = 1536 bytes/BX = 1.5 kB/BX.
@cmsbuild please test
+1
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-5000cf/26922/summary.html
COMMIT: 65b6a5e64a3c12485440d682d129693f2f863feb
CMSSW: CMSSW_12_5_X_2022-08-18-2300/el8_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/38986/26922/install.sh to create a dev area with all the needed externals and cmssw changes.
Comparison Summary
Summary:
- No significant changes to the logs found
- Reco comparison results: 0 differences found in the comparisons
- DQMHistoTests: Total files compared: 51
- DQMHistoTests: Total histograms compared: 3693040
- DQMHistoTests: Total failures: 2
- DQMHistoTests: Total nulls: 0
- DQMHistoTests: Total successes: 3693016
- DQMHistoTests: Total skipped: 22
- DQMHistoTests: Total Missing objects: 0
- DQMHistoSizes: Histogram memory added: 0.0 KiB( 50 files compared)
- Checked 212 log files, 49 edm output root files, 51 DQM output files
- TriggerResults: no differences found