Strip hybrid zero suppression: do less work if the APV is already zero-suppressed

[pieterdavid]

PR description:

Changed the implementation of the zero-suppression for hybrid strip tracker data, where most of the APVs are zero-suppressed in the FED, and some detected as having a non-flat or shifted baseline, and fully read out. When the software zero-suppression processes these data, the digis for APVs that were already zero-suppressed can simply be copied, but the current implementation would still recalculate the common mode (and ignore the result from that).
The time gain for a 2021 HI scenario is difficult to estimate (it depends on occupancy and the number of APVs that is not zero-suppressed by the FEDs), but should be significant, about an order of magnitude less time spent in the ZS (on the 2015 VR file I used for validation a factor 14).
With the changes above it didn't really make sense anymore to have different "convert hybrid to raw digis" and two suppressHybridData methods, so I made it one that can handle the calls from both the ZS module and the regional unpacker; I also removed the option to write the raw digis out again from the ZS for hybrid inputs (it was not used anywhere, and keeping this would have made the rest more complicated and inefficient).

PR validation:

No changes are expected. I did a digi-to-digi comparison by adding a flag to switch between the old and new implementation (removed only in the last commit), on 2018 repacked data (as in the HI benchmarks), config, and 2015 VR data with hybrid emulation, config; I also checked that the digis are the same between the full sequence and the regional unpacker (which calls the zero-suppression in a slightly different way, I think it may have been wrong before, but was not used).

Could this be tested with workflow 140.55? I think that's the only one that uses this code.

CC: @icali @CesarBernardes

[cmsbuild]

The code-checks are being triggered in jenkins.

[cmsbuild]

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-29759/15206

• This PR adds an extra 32KB to repository

[cmsbuild]

A new Pull Request was created by @pieterdavid (Pieter David) for master.

It involves the following packages:

RecoLocalTracker/SiStripClusterizer
RecoLocalTracker/SiStripZeroSuppression

@perrotta, @cmsbuild, @slava77 can you please review it and eventually sign? Thanks.
@echabert, @makortel, @robervalwalsh, @yduhm, @GiacomoSguazzoni, @gbenelli, @rovere, @VinInn, @alesaggio, @felicepantaleo, @gpetruc, @ebrondol, @threus, @mmusich this is something you requested to watch as well.
@silviodonato, @dpiparo you are the release manager for this.

cms-bot commands are listed here

[perrotta]

please test

[cmsbuild]

The tests are being triggered in jenkins.

[mmusich]

@perrotta

Could this be tested with workflow 140.55? I think that's the only one that uses this code.

Thanks!

[perrotta]

please abort
(I should read everything before launching tests...)

[cmsbuild]

Jenkins tests are aborted.

[perrotta]

please test workflow 140.55

[cmsbuild]

The tests are being triggered in jenkins.
Test Parameters:

• MATRIX_EXTRAS = 140.55
  https://cmssdt.cern.ch/jenkins/job/ib-run-pr-tests/6160/console Started: 2020/05/07 17:34

[cmsbuild]

The code-checks are being triggered in jenkins.

[cmsbuild]

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-29759/15271

• This PR adds an extra 16KB to repository

[cmsbuild]

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

[perrotta]

please test workflow 140.55

[cmsbuild]

The tests are being triggered in jenkins.
Test Parameters:

• MATRIX_EXTRAS = 140.55
  https://cmssdt.cern.ch/jenkins/job/ib-run-pr-tests/6210/console Started: 2020/05/11 10:08

[cmsbuild]

+1
Tested at: 4abf09d
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-c599a9/6210/summary.html
CMSSW: CMSSW_11_1_X_2020-05-10-2300
SCRAM_ARCH: slc7_amd64_gcc820

[cmsbuild]

Comparison job queued.

[cmsbuild]

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

Comparison Summary:

• No significant changes to the logs found
• Reco comparison results: 4 differences found in the comparisons
• DQMHistoTests: Total files compared: 34
• DQMHistoTests: Total histograms compared: 2697527
• DQMHistoTests: Total failures: 17
• DQMHistoTests: Total nulls: 0
• DQMHistoTests: Total successes: 2697191
• DQMHistoTests: Total skipped: 319
• DQMHistoTests: Total Missing objects: 0
• DQMHistoSizes: Histogram memory added: 0.0 KiB( 33 files compared)
• Checked 147 log files, 16 edm output root files, 34 DQM output files

[slava77]

+1

for #29759 4abf09d

• 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
• local test with wf 140.55 shows about a factor of 2 speedup in the siStripZeroSuppression module time (from 1.5 s/ev to 0.8 s/ev); based on the PR description this is probably still within the expected range

[cmsbuild]

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 (and backports should be raised in the release meeting by the corresponding L2)

[silviodonato]

+1