Include charge reweighting for Phase-2 IT planar sensors #40417
Conversation
|
test parameters:
|
|
-code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-40417/33545
Code check has found code style and quality issues which could be resolved by applying following patch(s)
|
tvami
force-pushed
the
IncludeChRewPhase2
branch
from
754c61f
to
f51f8c2
Compare
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-40417/33546
|
|
A new Pull Request was created by @tvami (Tamas Vami) for master. It involves the following packages:
@cmsbuild, @AdrianoDee, @srimanob, @civanch, @mdhildreth can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
|
@cmsbuild , please test |
|
-1 Failed Tests: Build ClangBuild BuildI found compilation error when building: >> Compiling edm plugin /data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_13_0_X_2023-01-02-2300/src/SimTracker/SiPixelDigitizer/plugins/PreMixingSiPixelWorker.cc
>> Compiling edm plugin /data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_13_0_X_2023-01-02-2300/src/SimTracker/SiPixelDigitizer/plugins/SealModule.cc
>> Compiling edm plugin /data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_13_0_X_2023-01-02-2300/src/SimTracker/SiPixelDigitizer/plugins/SiPixelDigitizer.cc
>> Compiling edm plugin /data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_13_0_X_2023-01-02-2300/src/SimTracker/SiPixelDigitizer/plugins/SiPixelDigitizerAlgorithm.cc
/data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_13_0_X_2023-01-02-2300/src/SimTracker/SiPixelDigitizer/plugins/SiPixelDigitizerAlgorithm.cc: In member function 'void SiPixelDigitizerAlgorithm::induce_signal(std::vector::const_iterator, std::vector::const_iterator, const PSimHit&, size_t, size_t, unsigned int, const PixelGeomDetUnit*, const std::vector&)':
/data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_13_0_X_2023-01-02-2300/src/SimTracker/SiPixelDigitizer/plugins/SiPixelDigitizerAlgorithm.cc:1567:83: error: no matching function for call to 'SiPixelChargeReweightingAlgorithm::hitSignalReweight(const PSimHit&, hit_map_type&, const size_t&, size_t&, const unsigned int&, const PixelTopology*&, uint32_t&, SiPixelDigitizerAlgorithm::signal_map_type&, short unsigned int, bool&)'
1567 | reweighted = TheNewSiPixelChargeReweightingAlgorithmClass->hitSignalReweight(
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^
1568 | hit, hit_signal, hitIndex, ReferenceIndex4CR, tofBin, topol, detID, theSignal, hit.processType(), makeDSLinks);
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from /data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_13_0_X_2023-01-02-2300/src/SimTracker/SiPixelDigitizer/plugins/SiPixelDigitizerAlgorithm.cc:87:
Clang BuildI found compilation warning while trying to compile with clang. Command used: See details on the summary page. |
|
Ok so the real issue is that I see what needs to be changed for the Phase-1 algo to work again. |
tvami
force-pushed
the
IncludeChRewPhase2
branch
from
f51f8c2
to
c1a712a
Compare
cmsbuild
added
code-checks-pending
and removed
tests-rejected
code-checks-approved
labels
|
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-dc9d1c/29848/summary.html The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic:
You can see more details here: Comparison SummarySummary:
|
Comparisons at #40417 (comment) now look good.
On the other hand we might want to re-trigger tests to have a cleaner picture. I am not sure if the profiling option would also check on the SIM step. |
|
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-dc9d1c/29876/summary.html Comparison SummarySummary:
|
Yay! |
|
type trk |
|
new-feature |
|
+1 |
|
+upgrade New feature as explained in the PR description. The default is still not to use it, so no change is expected. |
cmsbuild
added
fully-signed
upgrade-approved
and removed
pending-signatures
upgrade-pending
labels
|
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. @perrotta, @dpiparo, @rappoccio (and backports should be raised in the release meeting by the corresponding L2) |
|
+1 |
PR description:
Follow-up to #40349
Add the boolean
UseReweightingto Phase-2 Tracker digitizers in order to simulate charge loss due to irradiation. This uses the same (now generalized) method that is use in the Phase-0/1 simulations. For now it's set to False by default. For now I didn't touch the late charge reweighting, I believe it's not (yet) relevant for Phase-2PR validation:
20834.0ran fine with the defaultFalsevalue.After the compilation issue for Phase-1, I also ran
250202.184which tests the late charge rew (this failed with some input error), and11634.0which is for realistic 2021.when
UseReweighting = True, I used the20834.0wf but modified the step2 towhere
125X_mcRun4_realistic_Candidate_2023_01_03_23_47_45contains the 2D templates needed for the charge reweighting. This runs fine as well.Run LA trees and showed the charge vs depth profiles with the
Trueoption, the outcome looks as expected.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:
Not a backport and no backport is needed.