Phase2 digitizer updated to exclude badchannel in Strip like detectors #37576

suchandradutta · 2022-04-15T07:33:12Z

PR description:

Implemented code to exclude BadChannels by reading information from the Condition Database for Strip-like detectors, namely PSS and SS types. Also updated the framework to be able to treat BadChannels independently for each type of subdetectors. For Inner Pixel detectors BadChannel implementation is done in PixelDigitizerAlgorithm. 3D-Pixel and Bricked-Pixel algorithms inherit from PixelDigitizerAlgorithm. Also changed the variable/method names dead->bad as the name Bad signifies both dead and noisy

Changes expected

Less Digis in PSS and SS detector and accordingly lower Occupancy. The amount what of loss depend on the fraction of bad channels introduced through the SiPhase2OTFakeBadStripsESSource

Depends on the PR #37463 (already integrated)

PR validation:

Validation is done using

runTheMatrix.py --what upgrade -ne -l 35010.0

The description and the plots are attached below

@emiglior @mmusich
Phase2Digitizer_BadChannelStudy_15042022.pdf

…he Condition Database for Strip-like detectors, namely PSS and SS types. Also updated the framework to be able to treat BadChannels independently for each type of subdetectors. For Inner Pixel detectors BadChannel implementation is done in PixelDigitizerAlgorithm. 3D-Pixel and Bricked-Pixel algorithms inherit from PixelDigitizerAlgorithm. Also changed the variable/method names dead->bad as the name Bad signifies both dead and noisy.

cmsbuild · 2022-04-15T07:40:35Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-37576/29328

This PR adds an extra 44KB to repository

cmsbuild · 2022-04-15T07:40:58Z

A new Pull Request was created by @suchandradutta (Suchandra Dutta) for master.

It involves the following packages:

SimTracker/SiPhase2Digitizer (upgrade, simulation)

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

cms-bot commands are listed here

mmusich · 2022-04-15T09:19:18Z

@cmsbuild, please test

cmsbuild · 2022-04-15T13:37:59Z

-1

Failed Tests: RelVals-INPUT
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-6adfd4/23943/summary.html
COMMIT: 56702e4
CMSSW: CMSSW_12_4_X_2022-04-14-2300/slc7_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/37576/23943/install.sh to create a dev area with all the needed externals and cmssw changes.

RelVals-INPUT

The relvals timed out after 4 hours.

Comparison Summary

Summary:

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

mmusich · 2022-04-15T19:44:45Z

@cmsbuild, please test

let's see if it was a glitch

cmsbuild · 2022-04-15T23:37:53Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-6adfd4/23958/summary.html
COMMIT: 56702e4
CMSSW: CMSSW_12_4_X_2022-04-15-1100/slc7_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/37576/23958/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: 2 differences found in the comparisons
DQMHistoTests: Total files compared: 48
DQMHistoTests: Total histograms compared: 3593159
DQMHistoTests: Total failures: 7
DQMHistoTests: Total nulls: 1
DQMHistoTests: Total successes: 3593129
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.004 KiB( 47 files compared)
DQMHistoSizes: changed ( 312.0 ): 0.004 KiB MessageLogger/Warnings
Checked 200 log files, 45 edm output root files, 48 DQM output files
TriggerResults: no differences found

civanch · 2022-04-16T10:50:24Z

SimTracker/SiPhase2Digitizer/plugins/Phase2TrackerDigitizerAlgorithm.cc

@@ -23,10 +23,10 @@
 #include "DataFormats/DetId/interface/DetId.h"
 #include "CondFormats/SiPixelObjects/interface/GlobalPixel.h"
 #include "CondFormats/SiPixelObjects/interface/SiPixelLorentzAngle.h"
-#include "CondFormats/SiPixelObjects/interface/SiPixelQuality.h"
-#include "CondFormats/SiPixelObjects/interface/PixelROC.h"
+//#include "CondFormats/SiPixelObjects/interface/SiPixelQuality.h"


It would be good not to keep commented headers.

civanch · 2022-04-16T10:53:10Z

@suchandradutta , there is a general recommendation: in plugin directory do not have separate *.h files but include them into *.cc; it may be not a strong requirement but when you already working with some modifications it may be worse implementing.

mmusich

if the PR is touched anyway here's a couple of other comments.

mmusich · 2022-04-16T11:58:44Z

SimTracker/SiPhase2Digitizer/plugins/PixelDigitizerAlgorithm.cc

+        for (auto const& p : path) {
+          const PixelROC* myroc = fedCablingMap_->findItem(p);
+          if (myroc->idInDetUnit() == j) {
+            LocalPixel::RocRowCol local = {39, 25};  //corresponding to center of ROC row, col


Is this true also for the phase2 sensors?

I do not think so. This is basically from PhaseI. Should I make it dummy for the moment? As it is the case fpr PSP sensors?

As the functionality is not used, I think it's fine as it is. Let's revisit this after the PR is merged.

These hardcoded bad modules should better get taken from a DB or configuration file.
It is easy to forget them hardcoded here otherwise.
If the experts keep agreeing on merging this PR as such, we will mergi it as it is: please at least open a github issue in that case, so that it will remain track of it.

@perrotta these are NOT hardcoded bad modules, this is just a cheap way of getting the center of the ROC in local pixel coordinates. My point was that 39,25 is the middle of the ROC for the phase-1 pixel, but not necessarily also for the phase-2 one.

Ah, ok. As such it makes more sense.
In any case, this could also get made customizable via python config, if it has to eventually support a different geometry for the Phase2 modules.

mmusich · 2022-04-16T15:31:21Z

SimTracker/SiPhase2Digitizer/plugins/PixelDigitizerAlgorithm.cc

+  for (size_t id = 0; id < disabledModules.size(); id++) {
+    if (detID == disabledModules[id].DetID) {
+      isbad = true;
+      badmodule = disabledModules[id];
+      break;
+    }
+  }


Suggested change

for (size_t id = 0; id < disabledModules.size(); id++) {

if (detID == disabledModules[id].DetID) {

isbad = true;

badmodule = disabledModules[id];

break;

}

}

for (const auto& mod : disabledModules) {

if (detID == mod.DetID) {

isbad = true;

badmodule = mod;

break;

}

}

this could be a range-based loop.

suchandradutta · 2022-04-17T04:13:53Z

@suchandradutta , there is a general recommendation: in plugin directory do not have separate *.h files but include them into *.cc; it may be not a strong requirement but when you already working with some modifications it may be worse implementing.

@civanch In the main driver class Phase2Digitizer.cc requires all the header files of individual algorithms. Also the inheritance will fail if the individual header files are removed.

cmsbuild · 2022-04-17T04:31:39Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-37576/29360

This PR adds an extra 44KB to repository

cmsbuild · 2022-04-17T04:32:03Z

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

mmusich · 2022-04-17T18:03:26Z

please test

cmsbuild · 2022-04-17T21:51:19Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-6adfd4/23977/summary.html
COMMIT: 2207283
CMSSW: CMSSW_12_4_X_2022-04-16-1100/slc7_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/37576/23977/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: 4 differences found in the comparisons
DQMHistoTests: Total files compared: 48
DQMHistoTests: Total histograms compared: 3591161
DQMHistoTests: Total failures: 7
DQMHistoTests: Total nulls: 1
DQMHistoTests: Total successes: 3591131
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: -0.004 KiB( 47 files compared)
DQMHistoSizes: changed ( 312.0 ): -0.004 KiB MessageLogger/Warnings
Checked 200 log files, 45 edm output root files, 48 DQM output files
TriggerResults: no differences found

mmusich · 2022-04-18T06:08:05Z

urgent

samples with this feature will be requested in 12_4_0_pre3

civanch · 2022-04-18T11:25:50Z

+1

srimanob · 2022-04-18T11:54:47Z

+Upgrade

This PR introduce the method to exclude bad channels using information from the DB. This PR has no effect in the current workflow, no change has been seen in PR test as expected.

cmsbuild · 2022-04-18T11:55:06Z

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

perrotta · 2022-04-18T18:58:58Z

+1

Suchandra added 2 commits April 15, 2022 08:43

cmsbuild added this to the CMSSW_12_4_X milestone Apr 15, 2022

cmsbuild added code-checks-pending orp-pending pending-signatures simulation-pending tests-pending upgrade-pending labels Apr 15, 2022

cmsbuild added code-checks-approved and removed code-checks-pending labels Apr 15, 2022

cmsbuild added tests-started and removed tests-pending labels Apr 15, 2022

mmusich mentioned this pull request Apr 15, 2022

[OT inefficiency] add 0% bad strip scenario to customization function and generate new RelVal workflows #37578

Merged

cmsbuild added tests-rejected and removed tests-started labels Apr 15, 2022

cmsbuild added tests-started and removed tests-rejected labels Apr 15, 2022

cmsbuild added tests-approved and removed tests-started labels Apr 15, 2022

civanch reviewed Apr 16, 2022

View reviewed changes

mmusich reviewed Apr 16, 2022

View reviewed changes

cmsbuild added code-checks-pending tests-pending and removed tests-approved code-checks-approved labels Apr 17, 2022

cmsbuild added code-checks-approved and removed code-checks-pending labels Apr 17, 2022

cmsbuild added tests-started and removed tests-pending labels Apr 17, 2022

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

cmsbuild added the urgent label Apr 18, 2022

cmsbuild added simulation-approved and removed simulation-pending labels Apr 18, 2022

cmsbuild added fully-signed upgrade-approved and removed pending-signatures upgrade-pending labels Apr 18, 2022

cmsbuild added orp-approved and removed orp-pending labels Apr 18, 2022

cmsbuild merged commit 4de7be8 into cms-sw:master Apr 18, 2022

cmsbuild mentioned this pull request Apr 19, 2022

Move framework test modules away from legacy types #37609

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Phase2 digitizer updated to exclude badchannel in Strip like detectors #37576

Phase2 digitizer updated to exclude badchannel in Strip like detectors #37576

suchandradutta commented Apr 15, 2022

cmsbuild commented Apr 15, 2022

cmsbuild commented Apr 15, 2022

mmusich commented Apr 15, 2022

cmsbuild commented Apr 15, 2022

mmusich commented Apr 15, 2022

cmsbuild commented Apr 15, 2022

civanch Apr 16, 2022

civanch commented Apr 16, 2022

mmusich left a comment

mmusich Apr 16, 2022

suchandradutta Apr 17, 2022

mmusich Apr 17, 2022

perrotta Apr 18, 2022

mmusich Apr 18, 2022 •

edited

Loading

perrotta Apr 18, 2022

mmusich Apr 16, 2022

suchandradutta commented Apr 17, 2022 •

edited

Loading

cmsbuild commented Apr 17, 2022

cmsbuild commented Apr 17, 2022

mmusich commented Apr 17, 2022

cmsbuild commented Apr 17, 2022

mmusich commented Apr 18, 2022

civanch commented Apr 18, 2022

srimanob commented Apr 18, 2022

cmsbuild commented Apr 18, 2022

perrotta commented Apr 18, 2022

Phase2 digitizer updated to exclude badchannel in Strip like detectors #37576

Phase2 digitizer updated to exclude badchannel in Strip like detectors #37576

Conversation

suchandradutta commented Apr 15, 2022

PR description:

Changes expected

Depends on the PR #37463 (already integrated)

PR validation:

cmsbuild commented Apr 15, 2022

cmsbuild commented Apr 15, 2022

mmusich commented Apr 15, 2022

cmsbuild commented Apr 15, 2022

RelVals-INPUT

Comparison Summary

mmusich commented Apr 15, 2022

cmsbuild commented Apr 15, 2022

Comparison Summary

civanch Apr 16, 2022

Choose a reason for hiding this comment

civanch commented Apr 16, 2022

mmusich left a comment

Choose a reason for hiding this comment

mmusich Apr 16, 2022

Choose a reason for hiding this comment

suchandradutta Apr 17, 2022

Choose a reason for hiding this comment

mmusich Apr 17, 2022

Choose a reason for hiding this comment

perrotta Apr 18, 2022

Choose a reason for hiding this comment

mmusich Apr 18, 2022 • edited Loading

Choose a reason for hiding this comment

perrotta Apr 18, 2022

Choose a reason for hiding this comment

mmusich Apr 16, 2022

Choose a reason for hiding this comment

suchandradutta commented Apr 17, 2022 • edited Loading

cmsbuild commented Apr 17, 2022

cmsbuild commented Apr 17, 2022

mmusich commented Apr 17, 2022

cmsbuild commented Apr 17, 2022

Comparison Summary

mmusich commented Apr 18, 2022

civanch commented Apr 18, 2022

srimanob commented Apr 18, 2022

cmsbuild commented Apr 18, 2022

perrotta commented Apr 18, 2022

mmusich Apr 18, 2022 •

edited

Loading

suchandradutta commented Apr 17, 2022 •

edited

Loading