Change of strategy in Pixel Charge Reweighting #34181

carolinecollard · 2021-06-18T12:56:03Z

PR description:

This PR modifies the strategy of which SimHit is used for the Charge Reweighting (CR) in Pixels.

The original strategy for the case of a non-Primary particle is to use the first hit of the collection for that detID. In general this hit has no overlap with the cluster and the CR is not applied.
The new strategy for the case of a non-Primary particle is to search for the primary hit with same trackID and use it for the CR. In case no such hit is found, then use the first hit of the collection for that detID, as in the original strategy.
The Consequence: The new strategy allows to apply CR in more cases than with the original one -> a lower averaged value of the charge and more digis in total. But only a small fraction of the hits are impacted by this change of strategy.

Latest indico presentation in Pixel offline meeting:
https://indico.cern.ch/event/1000811/contributions/4410111/attachments/2265215/3846024/20210616_PRinPreparation.pdf

PR validation:

Validation of the charge profile on 100k ttbar events :
https://indico.cern.ch/event/1000810/contributions/4389223/attachments/2256939/3829798/Pixel_Charge_profiles.pdf

Some additional validation checks have been performed as shown in the slides, and discussed in meeting:
https://indico.cern.ch/event/1000811/contributions/4410111/attachments/2265215/3846024/20210616_PRinPreparation.pdf

…hting in the case of no primary particle

… is envoked

cmsbuild · 2021-06-18T13:02:32Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-34181/23392

This PR adds an extra 44KB to repository

cmsbuild · 2021-06-18T13:02:51Z

A new Pull Request was created by @carolinecollard for master.

It involves the following packages:

SimTracker/SiPixelDigitizer

@cmsbuild, @civanch, @mdhildreth can you please review it and eventually sign? Thanks.
@makortel, @mtosi, @GiacomoSguazzoni, @JanFSchulte, @rovere, @VinInn, @OzAmram, @dkotlins, @ferencek, @threus, @dgulhan, @tvami 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 · 2021-06-18T16:37:08Z

SimTracker/SiPixelDigitizer/plugins/SiPixelDigitizerAlgorithm.cc

-  using std::cerr;
-  using std::cout;
-  using std::endl;
+  //  using std::cerr;


@carolinecollard , please, remove these lines forever, because std::cout,... allowed only in analyzers and unit tests.

civanch · 2021-06-18T17:28:38Z

SimTracker/SiPixelDigitizer/plugins/SiPixelDigitizerAlgorithm.cc


-    //     cout << " map size  " << calmap.size()  << endl;
+    //     edm::LogInfo("PixelDigitizer ") << " map size  " << calmap.size()  << "\n";
    //     map<int,CalParameters,std::less<int> >::iterator ix,it;


@carolinecollard , we keep commented lines as an exception. AN alternative LogDebug instead of edm::LogInfo - this code will not be executed unless you start debugging. For debug purpose instead of uncommenting you may substitute LogDebug by edm::LogVerbatim

mmusich · 2021-06-21T07:57:37Z

enable profiling

mmusich · 2021-06-21T07:57:48Z

@cmsbuild please test

cmsbuild · 2021-06-21T13:12:10Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-c8ab06/16127/summary.html
COMMIT: e90c092
CMSSW: CMSSW_12_0_X_2021-06-20-2300/slc7_amd64_gcc900
Additional Tests: PROFILING
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/34181/16127/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: 25284 differences found in the comparisons
DQMHistoTests: Total files compared: 38
DQMHistoTests: Total histograms compared: 2785631
DQMHistoTests: Total failures: 31936
DQMHistoTests: Total nulls: 2
DQMHistoTests: Total successes: 2753671
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.866 KiB( 37 files compared)
DQMHistoSizes: changed ( 10224.0 ): -0.308 KiB SiStrip/MechanicalView
DQMHistoSizes: changed ( 250202.181 ): 1.178 KiB SiStrip/MechanicalView
DQMHistoSizes: changed ( 312.0 ): -0.004 KiB MessageLogger/Warnings
Checked 160 log files, 37 edm output root files, 38 DQM output files
TriggerResults: found differences in 5 / 37 workflows

carolinecollard · 2021-06-22T06:49:34Z

@civanch, I have modified the code according to your comments (I hope I understood them correctly). So no more cout, cerr, endl (even if commented) and replacement of the commented printed messages by use of LogDebug.

cmsbuild · 2021-06-22T07:03:26Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-34181/23425

This PR adds an extra 36KB to repository

cmsbuild · 2021-06-22T07:03:47Z

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

civanch · 2021-06-22T10:14:59Z

please test

cmsbuild · 2021-06-22T20:35:22Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-c8ab06/16163/summary.html
COMMIT: 58a21ee
CMSSW: CMSSW_12_0_X_2021-06-22-1100/slc7_amd64_gcc900
Additional Tests: PROFILING
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/34181/16163/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: 25280 differences found in the comparisons
DQMHistoTests: Total files compared: 38
DQMHistoTests: Total histograms compared: 2785631
DQMHistoTests: Total failures: 31934
DQMHistoTests: Total nulls: 1
DQMHistoTests: Total successes: 2753674
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.87 KiB( 37 files compared)
DQMHistoSizes: changed ( 10224.0 ): -0.308 KiB SiStrip/MechanicalView
DQMHistoSizes: changed ( 250202.181 ): 1.178 KiB SiStrip/MechanicalView
Checked 160 log files, 37 edm output root files, 38 DQM output files
TriggerResults: found differences in 5 / 37 workflows

civanch · 2021-06-22T22:20:13Z

+1

cmsbuild · 2021-06-22T22:20:32Z

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-06-23T08:25:35Z

@carolinecollard Just to confirm whether all the changes brought by this PR are as expected?
https://cmssdt.cern.ch/SDT/jenkins-artifacts/baseLineComparisons/CMSSW_12_0_X_2021-06-22-1100+c8ab06/43602/validateJR.html

carolinecollard · 2021-06-24T07:09:07Z

@qliphy Yes, the changes are as expected.

qliphy · 2021-06-24T09:15:07Z

+1

carolinecollard and others added 2 commits June 7, 2021 14:32

changing the strategy in the choice of hit used for the charge reweig…

482b5bf

…hting in the case of no primary particle

reduced the number of places in the code where hitSignalReweight(...)…

e90c092

… is envoked

cmsbuild added this to the CMSSW_12_0_X milestone Jun 18, 2021

cmsbuild added code-checks-pending orp-pending pending-signatures simulation-pending tests-pending labels Jun 18, 2021

cmsbuild added code-checks-approved and removed code-checks-pending labels Jun 18, 2021

civanch reviewed Jun 18, 2021

View reviewed changes

cmsbuild added tests-started and removed tests-pending labels Jun 21, 2021

cmsbuild added tests-approved and removed tests-started labels Jun 21, 2021

use of LogDebug for debug printing messages

58a21ee

cmsbuild added code-checks-pending tests-pending and removed code-checks-approved tests-approved labels Jun 22, 2021

cmsbuild added code-checks-approved and removed code-checks-pending labels Jun 22, 2021

cmsbuild added tests-started and removed tests-pending labels Jun 22, 2021

cmsbuild added tests-approved and removed tests-started labels Jun 22, 2021

cmsbuild added fully-signed simulation-approved and removed pending-signatures simulation-pending labels Jun 22, 2021

cmsbuild added orp-approved and removed orp-pending labels Jun 24, 2021

cmsbuild merged commit 31a387e into cms-sw:master Jun 24, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Change of strategy in Pixel Charge Reweighting #34181

Change of strategy in Pixel Charge Reweighting #34181

carolinecollard commented Jun 18, 2021

cmsbuild commented Jun 18, 2021

cmsbuild commented Jun 18, 2021

civanch Jun 18, 2021

civanch Jun 18, 2021

mmusich commented Jun 21, 2021

mmusich commented Jun 21, 2021

cmsbuild commented Jun 21, 2021

carolinecollard commented Jun 22, 2021

cmsbuild commented Jun 22, 2021

cmsbuild commented Jun 22, 2021

civanch commented Jun 22, 2021

cmsbuild commented Jun 22, 2021

civanch commented Jun 22, 2021

cmsbuild commented Jun 22, 2021

qliphy commented Jun 23, 2021

carolinecollard commented Jun 24, 2021

qliphy commented Jun 24, 2021

Change of strategy in Pixel Charge Reweighting #34181

Change of strategy in Pixel Charge Reweighting #34181

Conversation

carolinecollard commented Jun 18, 2021

PR description:

PR validation:

cmsbuild commented Jun 18, 2021

cmsbuild commented Jun 18, 2021

civanch Jun 18, 2021

Choose a reason for hiding this comment

civanch Jun 18, 2021

Choose a reason for hiding this comment

mmusich commented Jun 21, 2021

mmusich commented Jun 21, 2021

cmsbuild commented Jun 21, 2021

Comparison Summary

carolinecollard commented Jun 22, 2021

cmsbuild commented Jun 22, 2021

cmsbuild commented Jun 22, 2021

civanch commented Jun 22, 2021

cmsbuild commented Jun 22, 2021

Comparison Summary

civanch commented Jun 22, 2021

cmsbuild commented Jun 22, 2021

qliphy commented Jun 23, 2021

carolinecollard commented Jun 24, 2021

qliphy commented Jun 24, 2021