[Pixel 3D Digitizer] Include charge generation underneath the n-column #31977

claralasa · 2020-10-28T17:17:08Z

PR description:

Improve 3D pixel digitization: include charge generation underneath the n-column in the 3D Pixel Digitizer, by allowing the n-column to penetrate to a certain extent.

Details here: https://indico.cern.ch/event/879046/contributions/3931488/attachments/2068866/3472667/20200703final.pdf

In addition, fix some issues in the provisional validator (remember it is not part of this package and will probably be removed or migrated to the Validator package once the main development is done).

Conveners of the Upgrade Simulation group, @emiglior and @skinnari , are aware and agree.
cc: @duartej

PR validation:

Following PR instructions, a basic battery of tests was successfully run. Given that this PR is only affecting 3D pixels geometry (T23), some other tests were performed with workflows 30210.0, 30410.0, 30234.0 and 30434.0. The workflows 30410.0 and 30434.0, which include Pileup, fail launching the following error:

An exception of category 'NoSecondaryFiles' occurred while
[0] Constructing the EventProcessor
[1] Constructing module: class=MixingModule label='mix'
Exception Message:
RootEmbeddedFileSequence no input files specified for secondary input source.

Pileup files are not found in the expected repository, it is probably an issue with my config at lxplus... so I ignored it.

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

It is not a backport

cmsbuild · 2020-10-28T17:17:35Z

The code-checks are being triggered in jenkins.

cmsbuild · 2020-10-28T17:25:10Z

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-31977/19441

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
- File SimTracker/SiPhase2Digitizer/plugins/Pixel3DDigitizerAlgorithm.h 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-31977/19441/code-format.patch
e.g. curl https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-31977/19441/code-format.patch | patch -p1
You can also run scram build code-format to apply code format directly

cmsbuild · 2020-10-28T17:38:30Z

The code-checks are being triggered in jenkins.

cmsbuild · 2020-10-28T17:46:11Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-31977/19445

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
- File SimTracker/SiPhase2Digitizer/plugins/Pixel3DDigitizerAlgorithm.h modified in PR(s): Migrate most of MixingModule and PreMixingModule to EventSetup consumes #31697

cmsbuild · 2020-10-28T17:46:32Z

A new Pull Request was created by @claralasa (Clara Lasaosa) for master.

It involves the following packages:

SimTracker/SiPhase2Digitizer

@cmsbuild, @civanch, @kpedro88, @mdhildreth can you please review it and eventually sign? Thanks.
@makortel, @mtosi, @GiacomoSguazzoni, @JanFSchulte, @rovere, @VinInn, @ebrondol, @threus, @dgulhan this is something you requested to watch as well.
@silviodonato, @dpiparo, @qliphy you are the release manager for this.

cms-bot commands are listed here

civanch · 2020-10-28T19:16:00Z

please test

cmsbuild · 2020-10-28T19:16:19Z

The tests are being triggered in jenkins.

CMSSW_11_2_X_2020-10-28-1100/slc7_amd64_gcc820: https://cmssdt.cern.ch/jenkins/job/ib-run-pr-tests/10369/console Started: 2020/10/28 20:17

kpedro88 · 2020-10-28T19:28:26Z

SimTracker/SiPhase2Digitizer/plugins/Pixel3DDigitizerAlgorithm.h

-  float _np_column_radius;
-  float _ohm_column_radius;
+  // Radius of Column np and ohmic
+  const float _np_column_radius;


Code rule 2.14: '_' should not be used as first character http://cms-sw.github.io/cms_coding_rules.html
though this usage precedes this PR, it would be good to correct it here (for both the functions and the variables)

kpedro88 · 2020-10-28T19:29:49Z

SimTracker/SiPhase2Digitizer/python/phase2TrackerDigitizer_cfi.py

@@ -203,6 +203,11 @@
    )
 )

+# Add some additional parameters for 3D pixels
+phase2TrackerDigitizer.Pixel3DDigitizerAlgorithm.NPColumnRadius = cms.double(4.0)


please do this inside the clone statement that defines Pixel3DDigitizerAlgorithm (L87 in this revision)

cmsbuild · 2020-10-28T20:34:22Z

+1
Tested at: 1769b9f
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-3e8404/10369/summary.html
CMSSW: CMSSW_11_2_X_2020-10-28-1100
SCRAM_ARCH: slc7_amd64_gcc820

cmsbuild · 2020-10-29T14:12:07Z

The code-checks are being triggered in jenkins.

cmsbuild · 2020-10-29T14:17:04Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-31977/19466

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
- File SimTracker/SiPhase2Digitizer/plugins/Pixel3DDigitizerAlgorithm.h modified in PR(s): Migrate most of MixingModule and PreMixingModule to EventSetup consumes #31697

cmsbuild · 2020-10-29T14:17:22Z

Pull request #31977 was updated. @cmsbuild, @civanch, @kpedro88, @mdhildreth can you please check and sign again.

cmsbuild · 2020-10-29T15:53:16Z

+1
Tested at: 6e22721
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-3e8404/10381/summary.html
CMSSW: CMSSW_11_2_X_2020-10-29-1100
SCRAM_ARCH: slc7_amd64_gcc820

cmsbuild · 2020-10-29T15:53:18Z

Comparison job queued.

cmsbuild · 2020-10-29T17:12:20Z

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

Comparison Summary:

No significant changes to the logs found
Reco comparison results: 0 differences found in the comparisons
DQMHistoTests: Total files compared: 35
DQMHistoTests: Total histograms compared: 2544092
DQMHistoTests: Total failures: 1
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 2544069
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 34 files compared)
Checked 149 log files, 22 edm output root files, 35 DQM output files

civanch · 2020-10-29T21:45:10Z

+1

kpedro88 · 2020-10-29T23:13:25Z

+upgrade

cmsbuild · 2020-10-29T23:13:48Z

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 · 2020-10-30T00:10:26Z

+1

claralasa added 4 commits October 15, 2020 17:26

Fix pixel translation bug

b361733

Generate charge underneath the n-column

cc93d7f

Add missing change in commit b361733

968575e

Improve validator logic: loop based on PSimHit

23d42ab

cmsbuild added this to the CMSSW_11_2_X milestone Oct 28, 2020

cmsbuild added code-checks-pending comparison-pending orp-pending pending-signatures simulation-pending tests-pending upgrade-pending labels Oct 28, 2020

cmsbuild added code-checks-rejected and removed code-checks-pending labels Oct 28, 2020

Fix code style

1769b9f

cmsbuild added code-checks-pending and removed code-checks-rejected labels Oct 28, 2020

cmsbuild added code-checks-approved and removed code-checks-pending labels Oct 28, 2020

cmsbuild added tests-started and removed tests-pending labels Oct 28, 2020

kpedro88 reviewed Oct 28, 2020

View reviewed changes

cmsbuild added the code-checks-rejected label Oct 29, 2020

Fix code style

6e22721

cmsbuild added code-checks-pending and removed code-checks-rejected labels Oct 29, 2020

cmsbuild added code-checks-approved and removed code-checks-pending labels Oct 29, 2020

cmsbuild added tests-approved and removed tests-pending labels Oct 29, 2020

cmsbuild added comparison-available and removed comparison-pending labels Oct 29, 2020

cmsbuild added simulation-approved and removed simulation-pending labels Oct 29, 2020

cmsbuild added fully-signed upgrade-approved and removed pending-signatures upgrade-pending labels Oct 29, 2020

cmsbuild added orp-approved and removed orp-pending labels Oct 30, 2020

cmsbuild merged commit 153e26d into cms-sw:master Oct 30, 2020

cmsbuild mentioned this pull request Nov 4, 2020

Switch to Geant4 thermal neutron physics #32009

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Pixel 3D Digitizer] Include charge generation underneath the n-column #31977

[Pixel 3D Digitizer] Include charge generation underneath the n-column #31977

claralasa commented Oct 28, 2020

cmsbuild commented Oct 28, 2020

cmsbuild commented Oct 28, 2020

cmsbuild commented Oct 28, 2020

cmsbuild commented Oct 28, 2020

cmsbuild commented Oct 28, 2020

civanch commented Oct 28, 2020

cmsbuild commented Oct 28, 2020 •

edited

Loading

kpedro88 Oct 28, 2020

kpedro88 Oct 28, 2020

cmsbuild commented Oct 28, 2020

cmsbuild commented Oct 29, 2020

cmsbuild commented Oct 29, 2020

cmsbuild commented Oct 29, 2020

cmsbuild commented Oct 29, 2020

cmsbuild commented Oct 29, 2020

cmsbuild commented Oct 29, 2020

civanch commented Oct 29, 2020

kpedro88 commented Oct 29, 2020

cmsbuild commented Oct 29, 2020

qliphy commented Oct 30, 2020

[Pixel 3D Digitizer] Include charge generation underneath the n-column #31977

[Pixel 3D Digitizer] Include charge generation underneath the n-column #31977

Conversation

claralasa commented Oct 28, 2020

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 Oct 28, 2020

cmsbuild commented Oct 28, 2020

cmsbuild commented Oct 28, 2020

cmsbuild commented Oct 28, 2020

cmsbuild commented Oct 28, 2020

civanch commented Oct 28, 2020

cmsbuild commented Oct 28, 2020 • edited Loading

kpedro88 Oct 28, 2020

Choose a reason for hiding this comment

kpedro88 Oct 28, 2020

Choose a reason for hiding this comment

cmsbuild commented Oct 28, 2020

cmsbuild commented Oct 29, 2020

cmsbuild commented Oct 29, 2020

cmsbuild commented Oct 29, 2020

cmsbuild commented Oct 29, 2020

cmsbuild commented Oct 29, 2020

cmsbuild commented Oct 29, 2020

civanch commented Oct 29, 2020

kpedro88 commented Oct 29, 2020

cmsbuild commented Oct 29, 2020

qliphy commented Oct 30, 2020

cmsbuild commented Oct 28, 2020 •

edited

Loading