Pixel local reco: reduce code duplication between SiPixelDigiErrorsFromSoA and SiPixelRawToDigi #33526

czangela · 2021-04-26T12:17:56Z

PR description:

This PR reduces code duplication between SiPixelDigiErrorsFromSoA and SiPixelRawToDigi (see #32483).

The FED errors are sorted into different collections for later usage, this part of the code was implemented by both of the classes above. Now it has been moved to PixelDataFormatter and is accessible through a member function unpackFEDErrors(...).

Also there is some (harmless ?) cleanup done in SiPixelRawToDigi and PixelDataFormatter.

PR validation:

The code compiles and runs on the workflows 136.8855 and 136.885502 (phase 1) without any error.

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

cmsbuild · 2021-04-26T12:25:10Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33526/22288

This PR adds an extra 32KB to repository

cmsbuild · 2021-04-26T12:25:37Z

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

It involves the following packages:

EventFilter/SiPixelRawToDigi

@perrotta, @jpata, @cmsbuild, @slava77 can you please review it and eventually sign? Thanks.
@OzAmram, @makortel, @VinInn, @Martin-Grunewald, @dkotlins, @ferencek, @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

slava77 · 2021-04-26T13:22:48Z

@cmsbuild please test

cmsbuild · 2021-04-26T14:33:12Z

-1

Failed Tests: RelVals RelVals-INPUT AddOn
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-fa1837/14578/summary.html
COMMIT: 1379504
CMSSW: CMSSW_12_0_X_2021-04-26-1100/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/33526/14578/install.sh to create a dev area with all the needed externals and cmssw changes.

RelVals

11634.0

----- Begin Fatal Exception 26-Apr-2021 15:56:30 CEST-----------------------
An exception of category 'Configuration' occurred while
   [0] Constructing the EventProcessor
   [1] Validating configuration of module: class=SiPixelRawToDigi label='hltSiPixelDigis'
Exception Message:
Illegal parameter found in configuration.  The parameter is named:
 'Timing'
You could be trying to use a parameter name that is not
allowed for this plugin or it could be misspelled.
----- End Fatal Exception -------------------------------------------------

12434.0

----- Begin Fatal Exception 26-Apr-2021 15:57:18 CEST-----------------------
An exception of category 'Configuration' occurred while
   [0] Constructing the EventProcessor
   [1] Validating configuration of module: class=SiPixelRawToDigi label='hltSiPixelDigis'
Exception Message:
Illegal parameter found in configuration.  The parameter is named:
 'Timing'
You could be trying to use a parameter name that is not
allowed for this plugin or it could be misspelled.
----- End Fatal Exception -------------------------------------------------

11634.911

----- Begin Fatal Exception 26-Apr-2021 15:57:35 CEST-----------------------
An exception of category 'Configuration' occurred while
   [0] Constructing the EventProcessor
   [1] Validating configuration of module: class=SiPixelRawToDigi label='hltSiPixelDigis'
Exception Message:
Illegal parameter found in configuration.  The parameter is named:
 'Timing'
You could be trying to use a parameter name that is not
allowed for this plugin or it could be misspelled.
----- End Fatal Exception -------------------------------------------------

RelVals-INPUT

158.2158.2_PhotonJets_Pt_10_13_HI+PhotonJets_Pt_10_13_HIINPUT+DIGIHI2018PPRECO+RECOHI2018PPRECO+HARVESTHI2018PPRECO/step2_PhotonJets_Pt_10_13_HI+PhotonJets_Pt_10_13_HIINPUT+DIGIHI2018PPRECO+RECOHI2018PPRECO+HARVESTHI2018PPRECO.log
158.1158.1_QCD_Pt_80_120_13_HI+QCD_Pt_80_120_13_HIINPUT+DIGIHI2018PPRECO+RECOHI2018PPRECO+HARVESTHI2018PPRECO/step2_QCD_Pt_80_120_13_HI+QCD_Pt_80_120_13_HIINPUT+DIGIHI2018PPRECO+RECOHI2018PPRECO+HARVESTHI2018PPRECO.log
158.3158.3_ZEEMM_13_HI+ZEEMM_13_HIINPUT+DIGIHI2018PPRECO+RECOHI2018PPRECO+HARVESTHI2018PPRECO/step2_ZEEMM_13_HI+ZEEMM_13_HIINPUT+DIGIHI2018PPRECO+RECOHI2018PPRECO+HARVESTHI2018PPRECO.log

Expand to see more relval errors ...

AddOn Tests

hlt_mc_PRef

----- Begin Fatal Exception 26-Apr-2021 15:41:32 CEST-----------------------
An exception of category 'Configuration' occurred while
   [0] Constructing the EventProcessor
   [1] Validating configuration of module: class=SiPixelRawToDigi label='hltSiPixelDigis'
Exception Message:
Illegal parameter found in configuration.  The parameter is named:
 'Timing'
You could be trying to use a parameter name that is not
allowed for this plugin or it could be misspelled.
----- End Fatal Exception -------------------------------------------------

hlt_data_PRef

----- Begin Fatal Exception 26-Apr-2021 15:41:19 CEST-----------------------
An exception of category 'Configuration' occurred while
   [0] Constructing the EventProcessor
   [1] Validating configuration of module: class=SiPixelRawToDigi label='hltSiPixelDigis'
Exception Message:
Illegal parameter found in configuration.  The parameter is named:
 'Timing'
You could be trying to use a parameter name that is not
allowed for this plugin or it could be misspelled.
----- End Fatal Exception -------------------------------------------------

hlt_mc_HIon

----- Begin Fatal Exception 26-Apr-2021 15:42:12 CEST-----------------------
An exception of category 'Configuration' occurred while
   [0] Constructing the EventProcessor
   [1] Validating configuration of module: class=SiPixelRawToDigi label='hltSiPixelDigis'
Exception Message:
Illegal parameter found in configuration.  The parameter is named:
 'Timing'
You could be trying to use a parameter name that is not
allowed for this plugin or it could be misspelled.
----- End Fatal Exception -------------------------------------------------

Expand to see more addon errors ...

cmsbuild · 2021-04-26T14:53:03Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33526/22291

This PR adds an extra 16KB to repository

cmsbuild · 2021-04-26T14:53:25Z

Pull request #33526 was updated. @perrotta, @jpata, @cmsbuild, @slava77 can you please check and sign again.

slava77 · 2021-04-26T15:18:41Z

enable gpu

slava77 · 2021-04-26T15:18:53Z

@cmsbuild please test

cmsbuild · 2021-04-26T16:30:22Z

-1

Failed Tests: RelVals RelVals-INPUT AddOn
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-fa1837/14583/summary.html
COMMIT: ac54098
CMSSW: CMSSW_12_0_X_2021-04-26-1100/slc7_amd64_gcc900
Additional Tests: GPU
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/33526/14583/install.sh to create a dev area with all the needed externals and cmssw changes.

RelVals

11634.0

----- Begin Fatal Exception 26-Apr-2021 17:53:22 CEST-----------------------
An exception of category 'Configuration' occurred while
   [0] Constructing the EventProcessor
   [1] Validating configuration of module: class=SiPixelRawToDigi label='hltSiPixelDigis'
Exception Message:
In the configuration, parameter "Timing" is defined as an untracked bool.
It should be tracked.
----- End Fatal Exception -------------------------------------------------

12434.0

----- Begin Fatal Exception 26-Apr-2021 17:53:55 CEST-----------------------
An exception of category 'Configuration' occurred while
   [0] Constructing the EventProcessor
   [1] Validating configuration of module: class=SiPixelRawToDigi label='hltSiPixelDigis'
Exception Message:
In the configuration, parameter "Timing" is defined as an untracked bool.
It should be tracked.
----- End Fatal Exception -------------------------------------------------

11634.911

----- Begin Fatal Exception 26-Apr-2021 17:54:11 CEST-----------------------
An exception of category 'Configuration' occurred while
   [0] Constructing the EventProcessor
   [1] Validating configuration of module: class=SiPixelRawToDigi label='hltSiPixelDigis'
Exception Message:
In the configuration, parameter "Timing" is defined as an untracked bool.
It should be tracked.
----- End Fatal Exception -------------------------------------------------

RelVals-INPUT

158.1158.1_QCD_Pt_80_120_13_HI+QCD_Pt_80_120_13_HIINPUT+DIGIHI2018PPRECO+RECOHI2018PPRECO+HARVESTHI2018PPRECO/step2_QCD_Pt_80_120_13_HI+QCD_Pt_80_120_13_HIINPUT+DIGIHI2018PPRECO+RECOHI2018PPRECO+HARVESTHI2018PPRECO.log
158.2158.2_PhotonJets_Pt_10_13_HI+PhotonJets_Pt_10_13_HIINPUT+DIGIHI2018PPRECO+RECOHI2018PPRECO+HARVESTHI2018PPRECO/step2_PhotonJets_Pt_10_13_HI+PhotonJets_Pt_10_13_HIINPUT+DIGIHI2018PPRECO+RECOHI2018PPRECO+HARVESTHI2018PPRECO.log
158.3158.3_ZEEMM_13_HI+ZEEMM_13_HIINPUT+DIGIHI2018PPRECO+RECOHI2018PPRECO+HARVESTHI2018PPRECO/step2_ZEEMM_13_HI+ZEEMM_13_HIINPUT+DIGIHI2018PPRECO+RECOHI2018PPRECO+HARVESTHI2018PPRECO.log

Expand to see more relval errors ...

AddOn Tests

hlt_mc_PRef

----- Begin Fatal Exception 26-Apr-2021 17:40:37 CEST-----------------------
An exception of category 'Configuration' occurred while
   [0] Constructing the EventProcessor
   [1] Validating configuration of module: class=SiPixelRawToDigi label='hltSiPixelDigis'
Exception Message:
In the configuration, parameter "Timing" is defined as an untracked bool.
It should be tracked.
----- End Fatal Exception -------------------------------------------------

hlt_data_PRef

----- Begin Fatal Exception 26-Apr-2021 17:37:23 CEST-----------------------
An exception of category 'Configuration' occurred while
   [0] Constructing the EventProcessor
   [1] Validating configuration of module: class=SiPixelRawToDigi label='hltSiPixelDigis'
Exception Message:
In the configuration, parameter "Timing" is defined as an untracked bool.
It should be tracked.
----- End Fatal Exception -------------------------------------------------

hlt_mc_HIon

----- Begin Fatal Exception 26-Apr-2021 17:40:42 CEST-----------------------
An exception of category 'Configuration' occurred while
   [0] Constructing the EventProcessor
   [1] Validating configuration of module: class=SiPixelRawToDigi label='hltSiPixelDigis'
Exception Message:
In the configuration, parameter "Timing" is defined as an untracked bool.
It should be tracked.
----- End Fatal Exception -------------------------------------------------

Expand to see more addon errors ...

GPU Comparison Summary

Summary:

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

czangela · 2021-04-30T11:45:30Z

@cmsbuild please test
-- had to rebase due to a conflict

cmsbuild · 2021-04-30T15:01:25Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-fa1837/14747/summary.html
COMMIT: 64d828b
CMSSW: CMSSW_12_0_X_2021-04-29-2300/slc7_amd64_gcc900
Additional Tests: GPU
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/33526/14747/install.sh to create a dev area with all the needed externals and cmssw changes.

The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic:

remove Python2 unit tests ; migrate scripts using uproot to use python3 #33560 @davidlange6: remove Python2 unit tests ; migrate scripts using uproot to use python3
Migrate module configurations in RecoLocalMuon to use automatically generated cfi default configurations #33543 @jeongeun: Migrate module configurations in RecoLocalMuon to use automatically generated cfi default configurations
Migrate most of MixingModule and PreMixingModule to EventSetup consumes #31697 @makortel: Migrate most of MixingModule and PreMixingModule to EventSetup consumes
Adopt HFSignalAsymmetry flag in HF PFRecHit cleaning #33520 @hatakeyamak: Adopt HFSignalAsymmetry flag in HF PFRecHit cleaning
EtaBToJpsiJpsi gen fragment at 14 TeV for release validation #33386 @vjmastra: EtaBToJpsiJpsi gen fragment at 14 TeV for release validation

You can see more details here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-fa1837/14747/git-recent-commits.json
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-fa1837/14747/git-merge-result

GPU Comparison Summary

Summary:

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

Comparison Summary

Summary:

No significant changes to the logs found
Reco comparison results: 1014 differences found in the comparisons
DQMHistoTests: Total files compared: 37
DQMHistoTests: Total histograms compared: 2662646
DQMHistoTests: Total failures: 1155
DQMHistoTests: Total nulls: 1
DQMHistoTests: Total successes: 2661468
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: -0.016 KiB( 36 files compared)
DQMHistoSizes: changed ( 136.874 ): -0.016 KiB JetMET/SUSYDQM
Checked 155 log files, 37 edm output root files, 37 DQM output files
TriggerResults: no differences found

slava77 · 2021-05-01T00:22:11Z

@cmsbuild please test

to get a cleaner set (even though 30-2300 is not out yet; advancing to 30-1100 should be enough to get rid of differences in reco)

cmsbuild · 2021-05-01T07:06:01Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-fa1837/14770/summary.html
COMMIT: 64d828b
CMSSW: CMSSW_12_0_X_2021-04-30-1100/slc7_amd64_gcc900
Additional Tests: GPU
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/33526/14770/install.sh to create a dev area with all the needed externals and cmssw changes.

The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic:

Migrate module configurations in RecoLocalMuon to use automatically generated cfi default configurations #33543 @jeongeun: Migrate module configurations in RecoLocalMuon to use automatically generated cfi default configurations

You can see more details here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-fa1837/14770/git-recent-commits.json
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-fa1837/14770/git-merge-result

GPU Comparison Summary

Summary:

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

Comparison Summary

Summary:

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

mmusich · 2021-05-03T10:27:13Z

It should be now good to go from the DPG perspective.

slava77 · 2021-05-03T13:12:59Z

+reconstruction

for #33526 64d828b

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

Martin-Grunewald · 2021-05-03T13:24:12Z

+1

cmsbuild · 2021-05-03T13:24:31Z

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-05-03T23:54:16Z

+1

makortel · 2021-05-04T18:12:21Z

Seems that this PR caused a test failure in the IBs, see #33623

mmusich · 2021-05-04T18:49:06Z

Issue is addressed at #33625

@czangela there some other occurrences of this parameters in other (untested) files:

https://cmssdt.cern.ch/dxr/CMSSW/search?q=siPixelDigis.Timing&case=true&redirect=true

perhaps you can clean those up to in a follow-up PR.

cmsbuild added this to the CMSSW_12_0_X milestone Apr 26, 2021

cmsbuild added code-checks-pending orp-pending pending-signatures reconstruction-pending tests-pending labels Apr 26, 2021

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

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

cmsbuild added tests-rejected code-checks-pending tests-pending and removed tests-started code-checks-approved tests-rejected labels Apr 26, 2021

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

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

cmsbuild removed the tests-started label Apr 26, 2021

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

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

cmsbuild added tests-started and removed tests-approved labels May 1, 2021

cmsbuild added tests-approved and removed tests-started labels May 1, 2021

cmsbuild added reconstruction-approved and removed reconstruction-pending labels May 3, 2021

cmsbuild added fully-signed hlt-approved and removed hlt-pending pending-signatures labels May 3, 2021

cmsbuild added orp-approved and removed orp-pending labels May 3, 2021

cmsbuild merged commit 1936aa5 into cms-sw:master May 3, 2021

makortel mentioned this pull request May 4, 2021

TestDQMOnlineClient-fed_dqm_sourceclient test fails #33623

Closed

czangela mentioned this pull request May 4, 2021

Fix leftover Timing parameters #33626

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Pixel local reco: reduce code duplication between SiPixelDigiErrorsFromSoA and SiPixelRawToDigi #33526

Pixel local reco: reduce code duplication between SiPixelDigiErrorsFromSoA and SiPixelRawToDigi #33526

czangela commented Apr 26, 2021

cmsbuild commented Apr 26, 2021

cmsbuild commented Apr 26, 2021

slava77 commented Apr 26, 2021

cmsbuild commented Apr 26, 2021

cmsbuild commented Apr 26, 2021

cmsbuild commented Apr 26, 2021

slava77 commented Apr 26, 2021

slava77 commented Apr 26, 2021

cmsbuild commented Apr 26, 2021

czangela commented Apr 30, 2021

cmsbuild commented Apr 30, 2021

slava77 commented May 1, 2021

cmsbuild commented May 1, 2021

mmusich commented May 3, 2021

slava77 commented May 3, 2021

Martin-Grunewald commented May 3, 2021

cmsbuild commented May 3, 2021

qliphy commented May 3, 2021

makortel commented May 4, 2021

mmusich commented May 4, 2021

Pixel local reco: reduce code duplication between SiPixelDigiErrorsFromSoA and SiPixelRawToDigi #33526

Pixel local reco: reduce code duplication between SiPixelDigiErrorsFromSoA and SiPixelRawToDigi #33526

Conversation

czangela commented Apr 26, 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 Apr 26, 2021

cmsbuild commented Apr 26, 2021

slava77 commented Apr 26, 2021

cmsbuild commented Apr 26, 2021

RelVals

RelVals-INPUT

AddOn Tests

cmsbuild commented Apr 26, 2021

cmsbuild commented Apr 26, 2021

slava77 commented Apr 26, 2021

slava77 commented Apr 26, 2021

cmsbuild commented Apr 26, 2021

RelVals

RelVals-INPUT

AddOn Tests

GPU Comparison Summary

czangela commented Apr 30, 2021

cmsbuild commented Apr 30, 2021

GPU Comparison Summary

Comparison Summary

slava77 commented May 1, 2021

cmsbuild commented May 1, 2021

GPU Comparison Summary

Comparison Summary

mmusich commented May 3, 2021

slava77 commented May 3, 2021

Martin-Grunewald commented May 3, 2021

cmsbuild commented May 3, 2021

qliphy commented May 3, 2021

makortel commented May 4, 2021

mmusich commented May 4, 2021