New option in CSC DigiToRaw to pack strip digis by CFEB in each chamber.

[dildick]

PR description:

• New option in CSC DigiToRaw step to pack strip digis by CFEB in each chamber (called packByCFEB). This is how it was done in the (digital)CFEBs in Run-1 and Run-2 (and how it will be done in Run-3), but it was not implemented until now. The option is still turned off though.
• I also added a unit test to check if the DAQ conditions implemented in the CSC packer correctly pack the CSC wiregroups, comparators and strip digis. 10 dimuon events are generated with 0.9 < |eta| < 2.4 and 2 < pT < 1000 GeV. The whole simulation is done up to RawToDigi, followed by the test. In each event, check each chamber (540 CSC chambers) if the DAQ conditions are met. They are
  • Pack wire digis if there is an ALCT in that chamber (and a L1A)
  • Pack comparator digis if there is CLCT in that chamber (and a L1A)
  • Pack strip digis in a CFEB (strip sector in a chamber) if there is CLCT pretrigger in that CFEB (and a L1A)
• The DAQ conditions were moved to a static class CSCDigiToRawAccept, so that I can use the same functions in the unit test.

PR validation:

Because packByCFEB is set to False there should be no changes in any workflow.

Tested with WF 11634.0

Also tested with testPackingUnpackingPreTriggers_cfg.py. To run the test, do cmsRun EventFilter/CSCRawToDigi/test/testPackingUnpackingPreTriggers_cfg.py with either packByCFEB in cscPacker_cfi True or False.

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

N/A

Before submitting your pull requests, make sure you followed this checklist:

• verify that the PR is really intended for the chosen branch
• verify that changes follow CMS Naming, Coding, And Style Rules
• verify that the PR passes the basic test procedure suggested in the CMSSW PR instructions

@barvic @ptcox @tahuang1991

[cmsbuild]

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-34783/24458

• This PR adds an extra 40KB to repository

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

[cmsbuild]

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-34783/24459

• This PR adds an extra 44KB to repository

[cmsbuild]

A new Pull Request was created by @dildick (Sven Dildick) for master.

It involves the following packages:

• DataFormats/MuonDetId (simulation)
• EventFilter/CSCRawToDigi (reconstruction)

@perrotta, @civanch, @mdhildreth, @cmsbuild, @slava77, @jpata can you please review it and eventually sign? Thanks.
@barvic, @HuguesBrun, @battibass, @abbiendi, @Fedespring, @Martin-Grunewald, @calderona, @sscruz, @jhgoh, @ptcox, @rovere, @trocino, @cericeci 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

[slava77]

@dildick
please add a link to slides if this was already presented in a DPG meeting.

@cms-sw/csc-dpg-l2
please clarify if we should expect a DPG code review here or if this was already checked before the PR was made.

Thank you.

[slava77]

@cmsbuild please test

[dildick]

@slava77: Tim Cox brought this up in a CSC DPG meeting some time ago. See https://indico.cern.ch/event/1056361/contributions/4443982/attachments/2277853/3869912/210707_csc_dpg_for_csc_meeting.pdf, page 8. It was on the CSC DPG wish list IIRC. Tim might be busy all week with CRUZET data certification though.

[cmsbuild]

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-bc8790/17551/summary.html
COMMIT: 5b640f2
CMSSW: CMSSW_12_1_X_2021-08-04-1100/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/34783/17551/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: 5 differences found in the comparisons
• DQMHistoTests: Total files compared: 39
• DQMHistoTests: Total histograms compared: 2998564
• DQMHistoTests: Total failures: 12
• DQMHistoTests: Total nulls: 1
• DQMHistoTests: Total successes: 2998529
• DQMHistoTests: Total skipped: 22
• DQMHistoTests: Total Missing objects: 0
• DQMHistoSizes: Histogram memory added: 0.004 KiB( 38 files compared)
• DQMHistoSizes: changed ( 312.0 ): 0.004 KiB MessageLogger/Warnings
• Checked 165 log files, 37 edm output root files, 39 DQM output files
• TriggerResults: no differences found

[dildick]

As expected, no differences for CSC in tests.

@slava77 Should we reinitiate the tests after setting packByCFEB to True? We don't have to merge it in CMSSW that way, but it would help us understand the effects on RECO muons and PF candidate reconstruction and identification in MC simulation. And I could use that information in slides to present to CSC DPG.

[ptcox]

Victor and I will surely take a look at the code, and we would like Sven to advertise his work on this at a CSC meeting. Slava Krutelyov wrote on 8/5/21 02:25:

…

@dildick <https://github.com/dildick> please add a link to slides if this was already presented in a DPG meeting. @cms-sw/csc-dpg-l2 <https://github.com/orgs/cms-sw/teams/csc-dpg-l2> please clarify if we should expect a DPG code review here or if this was already checked before the PR was made. Thank you. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#34783 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABGYLHT7NXJNUDGOPC4RMVDT3HLA7ANCNFSM5BSCZ3HQ>. Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&utm_campaign=notification-email>.

[civanch]

+1

[jpata]

Should we reinitiate the tests after setting packByCFEB to True?

As far as I know, we typically don't use the bot tests for this type of validation of new features. For this feature, an offline validation of step3 in a suitable workflow could be sufficient.

I left some stylistic code comments, let's wait for the DPG's comments and a link to the presentation as well.

[cmsbuild]

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-34783/24827

• This PR adds an extra 44KB to repository

[cmsbuild]

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

[dildick]

@jpata Can you restart the tests, please?

[perrotta]

@cmsbuild please test

[cmsbuild]

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-bc8790/17982/summary.html
COMMIT: 45a1877
CMSSW: CMSSW_12_1_X_2021-08-23-1100/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/34783/17982/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: 39
• DQMHistoTests: Total histograms compared: 3000352
• DQMHistoTests: Total failures: 5
• DQMHistoTests: Total nulls: 1
• DQMHistoTests: Total successes: 3000324
• DQMHistoTests: Total skipped: 22
• DQMHistoTests: Total Missing objects: 0
• DQMHistoSizes: Histogram memory added: -0.004 KiB( 38 files compared)
• DQMHistoSizes: changed ( 312.0 ): -0.004 KiB MessageLogger/Warnings
• Checked 165 log files, 37 edm output root files, 39 DQM output files
• TriggerResults: no differences found

[jpata]

+reconstruction

• new option for CSC strip digi packing (off by default)
• consequently, no reco changes expected or observed

[civanch]

+1

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

[perrotta]

+1

• new option for CSC strip digi packing, kept off by now but still testable with the included unit test