-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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
Update of Phase2 Tracker Digitizer to fix an issue in Premixing #42938
Update of Phase2 Tracker Digitizer to fix an issue in Premixing #42938
Conversation
…arge in # of Digis in inner Pixel modules when Premixing is used during Digitization together with Dual Slope signal scaling
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-42938/37085
|
A new Pull Request was created by @suchandradutta (Suchandra Dutta) for master. It involves the following packages:
@AdrianoDee, @cmsbuild, @mdhildreth, @civanch, @srimanob can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
test parameters:
|
@cmsbuild please test |
@@ -233,11 +233,17 @@ | |||
AddNoisyPixels = False, | |||
AddInefficiency = False, | |||
AddThresholdSmearing = False, | |||
Phase2ReadoutMode = -1, | |||
AdcFullScale = 255, | |||
AddXTalk = False, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would recommend documenting this here:
cmssw/SimTracker/SiPhase2Digitizer/python/phase2TrackerDigitizer_cfi.py
Lines 219 to 223 in f738fee
# For premixing stage1 | |
# - add noise as by default | |
# - do not add noisy pixels (to be done in stage2) | |
# - do not apply inefficiency (to be done in stage2) | |
# - disable threshold smearing |
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-e80085/35029/summary.html Comparison SummarySummary:
|
Hi @suchandradutta I assume the change is expected, only Phase-2 PreMix is changed with this PR. |
@srimanob can you please let me know the workflow number where the difference is seen? So that I can re-run and check. |
25034.999 |
…off X-Talk for OT layers alsoat the first step of PreMixing
@srimanob I can fully confirm the changes observed in Inner Tracker Digi Occupancy/Charge histograms exactly as expected. I have updated the configuration file with some comments and switched off x-talk simulation also fo outer tracker layers in first step of PreMixing. Test were also done with the updated configuration file. |
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-42938/37102
|
Pull request #42938 was updated. @AdrianoDee, @cmsbuild, @srimanob, @civanch, @mdhildreth can you please check and sign again. |
@cmsbuild please test |
please test |
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-e80085/35092/summary.html Comparison SummarySummary:
|
+1 only phase2 WFs are affected, which is expected |
+Upgrade Change on Phase-2 premix workflow is expected. |
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. @rappoccio, @antoniovilela, @sextonkennedy (and backports should be raised in the release meeting by the corresponding L2) |
yes, we may discuss this Friday. |
+1 |
PR description:
Updating Phase2TrackerDigitizerAlgorithm to fix the issue observed during Premixing of PU events at the Digitization. It was observed that the Digitized signal gets lowered and correspondingly # Digis are lower than expected in inner Pixel layers . It was found that this is due to the fact that the Dual-Slope signal scaling is used. It is now fixed using simple liner scaling of charge during the first step of Pre-Mixing.
@emiglior