[Upgrade Phase-2 Pixel 3D Digitizer] Fix misevaluation of charge when migrating due to diffusion #33300

claralasa · 2021-03-30T15:50:53Z

PR description:

Before this fix, more than half of the carriers were migrated due to diffusion. This made the residuals to change their sign in the region enabled for charge migration. In addition, a factor in the evaluation of migrated charge was missing.

Nevertheless, these fixes are barely noticeable due to the small allowed charge migration region (0.4 um) and so no changes in the plots are expected. In order to visualize the effect, such a region has been tested increasing up to 5 um. Just used for this study, the value in the code stays as 0.4 um. In the plot below, the charge per digi is shown. As can be observed, it does not change after the fix with the baseline of 0.4um.

There are also some changes in the provisional validator used to cross-check the digitizer (which is not included within the validation framework).

This PR only concerns Upgrade (@duartej , @emiglior , @skinnari )

PR validation:

Passed the runTheMatrix.py -l limited -i all --ibeos tests, don't know if they are relevant because I'm not sure if they were tested against 2026D65 geometry (or the current geometry with 3D pixels in barrel).

if this PR is a backport please specify the original PR and why you need to backport that PR:

No.

The rectangle formed by the entry and exit points of the PSimHit has been extended in order to consider all the pixels illuminated due to diffusion (charge sharing)

cmsbuild · 2021-03-30T15:56:51Z

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33300/21824

This PR adds an extra 32KB to repository
There are other open Pull requests which might conflict with changes you have proposed:
- File SimTracker/SiPhase2Digitizer/plugins/Pixel3DDigitizerAlgorithm.cc modified in PR(s): Migrate most of MixingModule and PreMixingModule to EventSetup consumes #31697

Code check has found code style and quality issues which could be resolved by applying following patch(s)

code-format:
https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33300/21824/code-format.patch
e.g. curl https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33300/21824/code-format.patch | patch -p1
You can also run scram build code-format to apply code format directly

cmsbuild · 2021-03-30T17:25:35Z

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33300/21829

This PR adds an extra 32KB to repository
There are other open Pull requests which might conflict with changes you have proposed:
- File SimTracker/SiPhase2Digitizer/plugins/Pixel3DDigitizerAlgorithm.cc modified in PR(s): Migrate most of MixingModule and PreMixingModule to EventSetup consumes #31697

Code check has found code style and quality issues which could be resolved by applying following patch(s)

code-format:
https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33300/21829/code-format.patch
e.g. curl https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33300/21829/code-format.patch | patch -p1
You can also run scram build code-format to apply code format directly

cmsbuild · 2021-03-30T17:31:06Z

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33300/21830

This PR adds an extra 36KB to repository
There are other open Pull requests which might conflict with changes you have proposed:
- File SimTracker/SiPhase2Digitizer/plugins/Pixel3DDigitizerAlgorithm.cc modified in PR(s): Migrate most of MixingModule and PreMixingModule to EventSetup consumes #31697

Code check has found code style and quality issues which could be resolved by applying following patch(s)

code-format:
https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33300/21830/code-format.patch
e.g. curl https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33300/21830/code-format.patch | patch -p1
You can also run scram build code-format to apply code format directly

cmsbuild · 2021-04-14T15:43:56Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-d62f29/14232/summary.html
COMMIT: a1a2810
CMSSW: CMSSW_11_3_X_2021-04-14-1100/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/33300/14232/install.sh to create a dev area with all the needed externals and cmssw changes.

CMS Clang-Tidy warnings: There are Clang-Tidy warnings. See https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-d62f29/14232/llvm-analysis/cmsclangtidy.txt for details.

Comparison Summary

Summary:

No significant changes to the logs found
Reco comparison results: 4 differences found in the comparisons
DQMHistoTests: Total files compared: 38
DQMHistoTests: Total histograms compared: 2864426
DQMHistoTests: Total failures: 6
DQMHistoTests: Total nulls: 1
DQMHistoTests: Total successes: 2864397
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: -0.004 KiB( 37 files compared)
DQMHistoSizes: changed ( 312.0 ): -0.004 KiB MessageLogger/Warnings
Checked 160 log files, 37 edm output root files, 38 DQM output files
TriggerResults: no differences found

civanch · 2021-04-15T19:46:08Z

+1

srimanob · 2021-04-17T07:12:25Z

@claralasa The short matrix does not include geometry with 3D pixel. Is T25 (D80) or T26 (D81) OK for the test? I will retrigger the test again.

By the way, as warning shown in
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-d62f29/14232/llvm-analysis/esrget-sa.txt
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-d62f29/14232/llvm-analysis/cmsclangtidy.txt
Do you have plan for migration?

FYI @mmusich

mmusich · 2021-04-17T08:03:36Z

@srimanob

The short matrix does not include geometry with 3D pixel. Is T25 (D80) or T26 (D81) OK for the test?

Yes, these are OK, but T23 (D79, wf: 35834.0) is even better (it has 3D also in the endcaps).

Do you have plan for migration?

Well, for the getByToken warnings in clang-tidy as I understand it's only "suggestion" for the moment, and will be processed in due time. For the esConsumes these are treated "centrally" already by the core group in PR #31697. What's the plan about that one?

srimanob · 2021-04-17T08:09:18Z

Please test workflow 35834.0

srimanob · 2021-04-17T08:11:18Z

The short matrix does not include geometry with 3D pixel. Is T25 (D80) or T26 (D81) OK for the test?

Yes, these are OK, but T23 (D79, wf: 35834.0) is even better (it has 3D also in the endcaps).
Thanks, I trigger the test with 35834.0.

Do you have plan for migration?

Well, for the getByToken warnings in clang-tidy as I understand it's only "suggestion" for the moment, and will be processed in due time. For the esConsumes these are treated "centrally" already by the core group in PR #31697. What's the plan about that one?

OK, thanks. Then I will ignore the warning for now, and will follow up with the core team.

cmsbuild · 2021-04-17T12:33:50Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-d62f29/14299/summary.html
COMMIT: a1a2810
CMSSW: CMSSW_12_0_X_2021-04-16-2300/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/33300/14299/install.sh to create a dev area with all the needed externals and cmssw changes.

CMS Clang-Tidy warnings: There are Clang-Tidy warnings. See https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-d62f29/14299/llvm-analysis/cmsclangtidy.txt for details.

Comparison Summary

@slava77 comparisons for the following workflows were not done due to missing matrix map:

/data/cmsbld/jenkins/workspace/compare-root-files-short-matrix/data/PR-d62f29/35834.0_TTbar_14TeV+2026D79+TTbar_14TeV_TuneCP5_GenSimHLBeamSpot14+DigiTrigger+RecoGlobal+HARVESTGlobal

Summary:

No significant changes to the logs found
Reco comparison results: 3 differences found in the comparisons
DQMHistoTests: Total files compared: 38
DQMHistoTests: Total histograms compared: 2877046
DQMHistoTests: Total failures: 7
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 2877017
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 37 files compared)
Checked 160 log files, 37 edm output root files, 38 DQM output files
TriggerResults: no differences found

srimanob · 2021-04-20T09:04:59Z

+Upgrade

cmsbuild · 2021-04-20T09:05:20Z

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. @silviodonato, @dpiparo, @qliphy (and backports should be raised in the release meeting by the corresponding L2)

qliphy · 2021-04-20T10:00:47Z

+1

claralasa added 5 commits March 12, 2021 17:08

Add 2D residual plot to the validator

fe49e6c

Cluster position is estimated with calibrated pixels

5f7cfe7

Extend PSimHit rectangle shadow for cluster matching

75de818

The rectangle formed by the entry and exit points of the PSimHit has been extended in order to consider all the pixels illuminated due to diffusion (charge sharing)

Correct diffusion loop to avoid unphysical migration

3751725

Fix number of migrated carriers misevaluation

6c50d89

cmsbuild added this to the CMSSW_11_3_X milestone Mar 30, 2021

cmsbuild added code-checks-pending orp-pending pending-signatures simulation-pending tests-pending upgrade-pending labels Mar 30, 2021

cmsbuild added code-checks-rejected and removed code-checks-pending labels Mar 30, 2021

Fix coding style

f15b047

cmsbuild added code-checks-pending and removed code-checks-rejected labels Mar 30, 2021

cmsbuild added code-checks-rejected and removed code-checks-pending labels Mar 30, 2021

Fix coding style

e857ba9

cmsbuild added code-checks-pending and removed code-checks-rejected labels Mar 30, 2021

cmsbuild added code-checks-rejected and removed code-checks-pending labels Mar 30, 2021

Fix coding style

71ada19

cmsbuild added code-checks-pending and removed code-checks-rejected labels Mar 30, 2021

cmsbuild added tests-started and removed tests-pending labels Apr 14, 2021

cmsbuild added tests-approved and removed tests-started labels Apr 14, 2021

cmsbuild modified the milestones: CMSSW_11_3_X, CMSSW_12_0_X Apr 15, 2021

cmsbuild added simulation-approved and removed simulation-pending labels Apr 15, 2021

cmsbuild added tests-started and removed tests-approved labels Apr 17, 2021

cmsbuild added tests-approved and removed tests-started labels Apr 17, 2021

cmsbuild mentioned this pull request Apr 19, 2021

Migrate most of MixingModule and PreMixingModule to EventSetup consumes #31697

Merged

cmsbuild added fully-signed upgrade-approved and removed pending-signatures upgrade-pending labels Apr 20, 2021

cmsbuild added orp-approved and removed orp-pending labels Apr 20, 2021

cmsbuild merged commit dafb55d into cms-sw:master Apr 20, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Upgrade Phase-2 Pixel 3D Digitizer] Fix misevaluation of charge when migrating due to diffusion #33300

[Upgrade Phase-2 Pixel 3D Digitizer] Fix misevaluation of charge when migrating due to diffusion #33300

claralasa commented Mar 30, 2021

cmsbuild commented Mar 30, 2021

cmsbuild commented Mar 30, 2021

cmsbuild commented Mar 30, 2021

cmsbuild commented Apr 14, 2021

civanch commented Apr 15, 2021

srimanob commented Apr 17, 2021

mmusich commented Apr 17, 2021

srimanob commented Apr 17, 2021

srimanob commented Apr 17, 2021

cmsbuild commented Apr 17, 2021

srimanob commented Apr 20, 2021

cmsbuild commented Apr 20, 2021

qliphy commented Apr 20, 2021

[Upgrade Phase-2 Pixel 3D Digitizer] Fix misevaluation of charge when migrating due to diffusion #33300

[Upgrade Phase-2 Pixel 3D Digitizer] Fix misevaluation of charge when migrating due to diffusion #33300

Conversation

claralasa commented Mar 30, 2021

PR description:

PR validation:

if this PR is a backport please specify the original PR and why you need to backport that PR:

cmsbuild commented Mar 30, 2021

cmsbuild commented Mar 30, 2021

cmsbuild commented Mar 30, 2021

cmsbuild commented Apr 14, 2021

Comparison Summary

civanch commented Apr 15, 2021

srimanob commented Apr 17, 2021

mmusich commented Apr 17, 2021

srimanob commented Apr 17, 2021

srimanob commented Apr 17, 2021

cmsbuild commented Apr 17, 2021

Comparison Summary

srimanob commented Apr 20, 2021

cmsbuild commented Apr 20, 2021

qliphy commented Apr 20, 2021