Pixel cluster repair by morphing #34606

veszpv · 2021-07-23T14:07:39Z

In later years of Run 3, the pixel detector, especially Layer 1, will suffer a significant radiation damage. Studies show that due to the resulting lower charge collection efficiency, pixel hits in the bulk of clusters may be systematically lost. This leads to the multiplication of fragmented clusters, to loss of resolution when the shoulders of the original clusters are not properly identified (and paired as required by generic CPE) as well as to position biases.

A digi-to-digi producer module is proposed to be placed between the raw2digi and the clusterizer modules that performs a somewhat modified "closing" image morphing operation in each pixel module separately. Closing is the result of the subsequent application of a dilation, with 3x3 kernel [(1, 1, 1) (1, 1, 1) (1, 1, 1)], and an erosion, with 3x3 kernel [(0, 1, 0) (1, 1, 1) (0, 1, 0)]. The operation injects extra pixels in the digi collection with configurable charge (provided in units of 10 electrons). These extra pixels are used in guiding the search for contiguous clusters in the clusterizer, and also potentially to initiate seeding if the adc is configured to pass the seeding threshold. Extra pixels are deleted from the final clusters.

Some reminders:

Extra pixels are flagged during morphing using the extra bit made available by Reserve one bit for flagging individual pixels #34509.
Clusters may no longer be contiguous, consumers need to make sure this is not an issue. Specifically, a subsequent PR is expected for template reconstruction addressing this before this feature can be turned on.
No improvement in intrinsic resolution is expected; the goal is to restore the remains of the original cluster in order to be able to fully exploit generic and template CPE-s
In 2021 we start with a pristine Layer 1. Assuming the CPE and tracking are optimized for that, any changes in clustering may lead to minor losses in performance. Therefore, both would need to be readjusted. Enhancement in performance is expected around 2024 or so.
More details are here: [1], [2]
Only CPU implementation is provided. As stated in [1], this module is merely ~2 times slower than raw2digi itself. Timing of a GPU implementation should be negligible.

This feature is turned off by default.

PR validation:

The method and choice of kernels were tested in two sets of clusters: exclusively broken ones with one or more discontinuities, and fully healthy ones. One can observe that the method has no effect on healthy clusters and perfectly restores x and y lengths of broken ones (this is required so that shoulders are identified and remain intact). Cluster size and charge are not restored, since missing pixels remain missing:

The cmssw implementation was tested earlier (in CMSSW 11_2_0) on real events with realistic mix of healthy and broken clusters in Layer 1 (called original), regular cluster size, and possibilities of cluster merging.

Validation on CPE and tracking are inconclusive, see results in [1].

This PR passes tests with runTheMatrix (thanks to Tanja Susa).

Further final validations will also be provided by Tanja Susa (by turning on this feature) to ensure the desired changes take place during digi morphing. The PR is requested to be merged to ease further studies.

cmsbuild · 2021-07-23T14:15:19Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-34606/24173

This PR adds an extra 32KB to repository

cmsbuild · 2021-07-23T14:15:45Z

A new Pull Request was created by @veszpv (Viktor Veszpremi) for master.

It involves the following packages:

Configuration/ProcessModifiers (operations)
DataFormats/SiPixelDetId (simulation)
RecoLocalTracker/Configuration (reconstruction)
RecoLocalTracker/SiPixelClusterizer (reconstruction)
RecoLocalTracker/SiPixelDigiMorphing (****)

The following packages do not have a category, yet:

RecoLocalTracker/SiPixelDigiMorphing
Please create a PR for https://github.com/cms-sw/cms-bot/blob/master/categories_map.py to assign category

@perrotta, @civanch, @silviodonato, @mdhildreth, @cmsbuild, @slava77, @jpata, @qliphy, @fabiocos, @davidlange6 can you please review it and eventually sign? Thanks.
@echabert, @felicepantaleo, @pieterdavid, @robervalwalsh, @Martin-Grunewald, @OzAmram, @threus, @makortel, @JanFSchulte, @ferencek, @yduhm, @GiacomoSguazzoni, @rovere, @VinInn, @alesaggio, @mmusich, @mtosi, @fabiocos, @gbenelli, @dkotlins, @gpetruc, @tvami this is something you requested to watch as well.
@silviodonato, @dpiparo, @qliphy, @perrotta you are the release manager for this.

cms-bot commands are listed here

tvami · 2021-07-23T14:24:18Z

@cmsbuild , please test

slava77

do we expect a significant growth of the code base in RecoLocalTracker/SiPixelDigiMorphing ?
an existing alternative could be a conveniently generically named RecoLocalTracker/SubCollectionProducers

(this is not a complete review, just a few quick notes before a more careful look)

slava77 · 2021-07-23T14:24:58Z

RecoLocalTracker/SiPixelDigiMorphing/plugins/SiPixelDigiMorphing.h

+#include "FWCore/ParameterSet/interface/ParameterSet.h"
+#include "FWCore/ParameterSet/interface/ConfigurationDescriptions.h"
+
+class SiPixelDigiMorphing : public edm::stream::EDProducer<> {


please merge into .cc file

slava77 · 2021-07-23T14:27:03Z

RecoLocalTracker/SiPixelDigiMorphing/plugins/SiPixelDigiMorphing.cc

+  desc.add<std::vector<int32_t>>("kernel2", {2, 7, 2});
+  desc.add<uint32_t>("fakeAdc", 100);
+
+  descriptions.add("SiPixelDigiMorphing", desc);


Suggested change

descriptions.add("SiPixelDigiMorphing", desc);

descriptions.addWithDefaultLabel(desc);

is a preferred

Or did you meanaddWithDefaultLabel()? (addDefault() does not generate cfi.py file)

Or did you meanaddWithDefaultLabel()? (addDefault() does not generate cfi.py file)

yes; thank you for noticing. I updated the suggestion

slava77 · 2021-07-23T14:27:49Z

RecoLocalTracker/SiPixelDigiMorphing/python/SiPixelDigiMorphing_cfi.py

@@ -0,0 +1,15 @@
+import FWCore.ParameterSet.Config as cms
+
+siPixelDigisMorphed = cms.EDProducer(


please replace with a clone from siPixelDigiMorphing_cfi generated with the fillDescriptions

slava77 · 2021-07-23T14:34:59Z

RecoLocalTracker/SiPixelClusterizer/python/SiPixelClusterizerPreSplitting_cfi.py

+siPixelDigiMorphing.toModify(
+    siPixelClustersPreSplitting,
+    cpu = dict(
+         src = cms.InputTag('siPixelDigisMorphed')


Suggested change

src = cms.InputTag('siPixelDigisMorphed')

src = 'siPixelDigisMorphed'

mmusich · 2021-07-23T14:42:19Z

do we expect a significant growth of the code base in RecoLocalTracker/SiPixelDigiMorphing ?
an existing alternative could be a conveniently generically named RecoLocalTracker/SubCollectionProducers

Presumably at some point it will have to host an heterogeneous implementation.
RecoLocalTracker/SubCollectionProducers might be viable but strictly speaking the output is not a subcollection, it contains all standard digis and more

veszpv · 2021-07-23T16:30:45Z

do we expect a significant growth of the code base in RecoLocalTracker/SiPixelDigiMorphing ?
an existing alternative could be a conveniently generically named RecoLocalTracker/SubCollectionProducers

Presumably at some point it will have to host an heterogeneous implementation.
RecoLocalTracker/SubCollectionProducers might be viable but strictly speaking the output is not a subcollection, it contains all standard digis and more

Apart from a potential implementation also on GPU, I have doubts if this will grow further simply because there are not many options to transform the pixel information at such an early stage of local reco. I let you decide if we want to move this to a generically named directory. Mind you, this is strictly a derived and transient collection.

cmsbuild · 2021-07-23T18:00:33Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-546425/17151/summary.html
COMMIT: 81c2cc5
CMSSW: CMSSW_12_0_X_2021-07-22-2300/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/34606/17151/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: 39
DQMHistoTests: Total histograms compared: 2998564
DQMHistoTests: Total failures: 1
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 2998541
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 38 files compared)
Checked 165 log files, 37 edm output root files, 39 DQM output files
TriggerResults: no differences found

slava77 · 2021-07-23T18:15:53Z

perhaps instead of RecoLocalTracker/SiPixelDigiMorphing a RecoLocalTracker/SiPixelDigiReProducers
would be more future-proof against similar ideas to update the digis?

cmsbuild · 2021-07-26T22:40:06Z

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-34606/24225

This PR adds an extra 32KB to repository
Found files with invalid states:
- RecoLocalTracker/SiPixelDigiMorphing/python/SiPixelDigiMorphing_cfi.py:
  - Added: 77a4088
  - Deleted: ca3f096
- RecoLocalTracker/SiPixelDigiReProducers/doc/SiPixelDigiMorphing.doc:
  - Added: ca3f096
  - Deleted: 11da61f
- RecoLocalTracker/SiPixelDigiMorphing/plugins/BuildFile.xml:
  - Added: 77a4088
  - Deleted: ca3f096
- RecoLocalTracker/SiPixelDigiMorphing/plugins/SiPixelDigiMorphing.cc:
  - Added: 77a4088
  - Deleted: ca3f096
- RecoLocalTracker/SiPixelDigiMorphing/doc/SiPixelDigiMorphing.doc:
  - Added: 77a4088
  - Deleted: ca3f096
- RecoLocalTracker/SiPixelDigiMorphing/plugins/SiPixelDigiMorphing.h:
  - Added: 77a4088
  - Deleted: ca3f096
- RecoLocalTracker/SiPixelDigiReProducers/plugins/SiPixelDigiMorphing.h:
  - Added: ca3f096
  - Deleted: 11da61f

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-34606/24225/code-format.patch
e.g. curl https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-34606/24225/code-format.patch | patch -p1
You can also run scram build code-format to apply code format directly

mmusich · 2021-08-07T07:35:06Z

Elaborating a bit on my comment #34606 (comment):

the Heavy Ion case is relevant only for the run1 HI reconstruction (which we are not targeting in general). That should hold true as long as the Run3 HI data is reconstructed with ppOnAA scenario.
the cosmics reconstruction could be targeted by this update as well, but needs to be checked before introducing the change there too

If there are no further comments, I think the PR can proceed, after being tested without the morphing option.

tsusa · 2021-08-07T14:55:30Z

I have run the matrix w/ and w/o modifier:
runTheMatrix.py -l limited -i all —ibeos
runTheMatrix.py -l limited -i all —ibeos --command='--procModifiers=siPixelDigiMorphing'

With modifier, there are nine failures (37 34 28 21 14 4 1 1 1 tests passed, 2 2 5 0 0 0 0 0 0 failed).
In four cases it is due to the missing changes in some of the config files (as @mmusich mentioned in the previous comment), but we would prefer to address those when the feature will be switched on. The remaining five are in the wfs that already have other customisations and those got "overwritten".

W/o modifier, there was no failure. I would propose to proceed with the tests w/o the morphing option.

mmusich · 2021-08-07T14:56:52Z

test parameters:

mmusich · 2021-08-07T14:57:07Z

@cmsbuild, please test

cmsbuild · 2021-08-07T19:43:23Z

-1

Failed Tests: RelVals-INPUT
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-546425/17630/summary.html
COMMIT: efa179c
CMSSW: CMSSW_12_1_X_2021-08-07-1100/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/34606/17630/install.sh to create a dev area with all the needed externals and cmssw changes.

RelVals-INPUT

11603.011603.0_SingleElectronPt1000+2021+SingleElectronPt1000_pythia8_GenSimINPUT+Digi+Reco+HARVEST+ALCA/step2_SingleElectronPt1000+2021+SingleElectronPt1000_pythia8_GenSimINPUT+Digi+Reco+HARVEST+ALCA.log
11601.011601.0_SingleElectronPt10+2021+SingleElectronPt10_pythia8_GenSimINPUT+Digi+Reco+HARVEST+ALCA/step2_SingleElectronPt10+2021+SingleElectronPt10_pythia8_GenSimINPUT+Digi+Reco+HARVEST+ALCA.log
11602.011602.0_SingleElectronPt35+2021+SingleElectronPt35_pythia8_GenSimINPUT+Digi+Reco+HARVEST+ALCA/step2_SingleElectronPt35+2021+SingleElectronPt35_pythia8_GenSimINPUT+Digi+Reco+HARVEST+ALCA.log

Expand to see more relval errors ...

Comparison Summary

Summary:

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

tvami · 2021-08-08T12:04:43Z

@cmsbuild , please test

cmsbuild · 2021-08-08T15:30:46Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-546425/17635/summary.html
COMMIT: efa179c
CMSSW: CMSSW_12_1_X_2021-08-08-0000/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/34606/17635/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: 39
DQMHistoTests: Total histograms compared: 2999410
DQMHistoTests: Total failures: 1
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 2999387
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 38 files compared)
Checked 165 log files, 37 edm output root files, 39 DQM output files
TriggerResults: no differences found

slava77 · 2021-08-09T18:20:03Z

+reconstruction

for #34606 efa179c

code changes are in line with the PR description and the follow up review
jenkins tests pass and comparisons with the baseline show no differences (the introduced new feature is not enabled in the standard workflows)

qliphy · 2021-08-10T02:13:26Z

+1

cmsbuild · 2021-08-10T02:13:50Z

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 be automatically merged.

cmsbuild added this to the CMSSW_12_0_X milestone Jul 23, 2021

cmsbuild added code-checks-pending new-package-pending operations-pending orp-pending pending-signatures reconstruction-pending simulation-pending tests-pending labels Jul 23, 2021

cmsbuild added code-checks-approved and removed code-checks-pending labels Jul 23, 2021

cmsbuild added tests-started and removed tests-pending labels Jul 23, 2021

veszpv mentioned this pull request Jul 23, 2021

Update categories_map.py cms-sw/cms-bot#1597

Merged

slava77 reviewed Jul 23, 2021

View reviewed changes

cmsbuild added tests-approved and removed tests-started labels Jul 23, 2021

cmsbuild added code-checks-pending tests-pending and removed code-checks-approved tests-approved labels Jul 26, 2021

cmsbuild removed the code-checks-pending label Jul 26, 2021

cmsbuild added tests-started and removed tests-pending labels Aug 7, 2021

cmsbuild added tests-rejected and removed tests-started labels Aug 7, 2021

cmsbuild added tests-started and removed tests-rejected labels Aug 8, 2021

cmsbuild added tests-approved and removed tests-started labels Aug 8, 2021

cmsbuild added reconstruction-approved and removed reconstruction-pending labels Aug 9, 2021

cmsbuild added fully-signed operations-approved orp-approved and removed operations-pending pending-signatures orp-pending labels Aug 10, 2021

cmsbuild merged commit 44ad0e8 into cms-sw:master Aug 10, 2021

This was referenced Aug 11, 2021

[GCC11] Fix "this pointer is null" in class FWRecoGeometryESProducer of Fireworks/Geometry #34824

Merged

[GCC11] Add missing #include for std::size_t definition #34825

Merged

czangela mentioned this pull request Aug 12, 2021

Pixel cluster repair by morphing backport to 12_0_X #34851

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Pixel cluster repair by morphing #34606

Pixel cluster repair by morphing #34606

veszpv commented Jul 23, 2021

cmsbuild commented Jul 23, 2021

cmsbuild commented Jul 23, 2021

tvami commented Jul 23, 2021

slava77 left a comment

slava77 Jul 23, 2021

slava77 Jul 23, 2021 •

edited

makortel Jul 23, 2021

slava77 Jul 23, 2021

slava77 Jul 23, 2021

slava77 Jul 23, 2021

mmusich commented Jul 23, 2021

veszpv commented Jul 23, 2021

cmsbuild commented Jul 23, 2021

slava77 commented Jul 23, 2021

cmsbuild commented Jul 26, 2021

mmusich commented Aug 7, 2021

tsusa commented Aug 7, 2021 •

edited

mmusich commented Aug 7, 2021

mmusich commented Aug 7, 2021

cmsbuild commented Aug 7, 2021

tvami commented Aug 8, 2021

cmsbuild commented Aug 8, 2021

slava77 commented Aug 9, 2021

qliphy commented Aug 10, 2021

cmsbuild commented Aug 10, 2021

	descriptions.add("SiPixelDigiMorphing", desc);
	descriptions.addWithDefaultLabel(desc);

		@@ -0,0 +1,15 @@
		import FWCore.ParameterSet.Config as cms

		siPixelDigisMorphed = cms.EDProducer(

	src = cms.InputTag('siPixelDigisMorphed')
	src = 'siPixelDigisMorphed'

Pixel cluster repair by morphing #34606

Pixel cluster repair by morphing #34606

Conversation

veszpv commented Jul 23, 2021

PR validation:

cmsbuild commented Jul 23, 2021

cmsbuild commented Jul 23, 2021

tvami commented Jul 23, 2021

slava77 left a comment

Choose a reason for hiding this comment

slava77 Jul 23, 2021

Choose a reason for hiding this comment

slava77 Jul 23, 2021 • edited

Choose a reason for hiding this comment

makortel Jul 23, 2021

Choose a reason for hiding this comment

slava77 Jul 23, 2021

Choose a reason for hiding this comment

slava77 Jul 23, 2021

Choose a reason for hiding this comment

slava77 Jul 23, 2021

Choose a reason for hiding this comment

mmusich commented Jul 23, 2021

veszpv commented Jul 23, 2021

cmsbuild commented Jul 23, 2021

Comparison Summary

slava77 commented Jul 23, 2021

cmsbuild commented Jul 26, 2021

mmusich commented Aug 7, 2021

tsusa commented Aug 7, 2021 • edited

mmusich commented Aug 7, 2021

mmusich commented Aug 7, 2021

cmsbuild commented Aug 7, 2021

RelVals-INPUT

Comparison Summary

tvami commented Aug 8, 2021

cmsbuild commented Aug 8, 2021

Comparison Summary

slava77 commented Aug 9, 2021

qliphy commented Aug 10, 2021

cmsbuild commented Aug 10, 2021

slava77 Jul 23, 2021 •

edited

tsusa commented Aug 7, 2021 •

edited