Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GEM unpacker: crash protection from bad FEDs #31928

Merged

Conversation

jshlee
Copy link
Contributor

@jshlee jshlee commented Oct 23, 2020

PR description:

GEM unpacker (GEMRawToDigiModule) was crashing run 337971 MWGR.
This was due to wrong DAQ setting producing invalid GEM chamber and vfat IDs.
It was fixed with #31863 and this is backporting the same fix.
Not full backport due to many commits in 11_2_X.

PR validation:

  • tested with wf 11611.0, 23211.0 and run 337971 MWGR data.

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @jshlee (Jason Lee) for CMSSW_11_1_X.

It involves the following packages:

EventFilter/GEMRawToDigi

@perrotta, @jpata, @cmsbuild, @slava77 can you please review it and eventually sign? Thanks.
@Martin-Grunewald, @watson-ij 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
Copy link
Contributor

slava77 commented Oct 23, 2020

@cmsbuild please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Oct 23, 2020

The tests are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

+1
Tested at: a7c4f7a
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-40ba2c/10258/summary.html
CMSSW: CMSSW_11_1_X_2020-10-23-1100
SCRAM_ARCH: slc7_amd64_gcc820

@cmsbuild
Copy link
Contributor

Comparison job queued.

@cmsbuild
Copy link
Contributor

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

Comparison Summary:

  • No significant changes to the logs found
  • Reco comparison results: 2 differences found in the comparisons
  • DQMHistoTests: Total files compared: 36
  • DQMHistoTests: Total histograms compared: 2784776
  • DQMHistoTests: Total failures: 1
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 2784725
  • DQMHistoTests: Total skipped: 50
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 35 files compared)
  • Checked 152 log files, 16 edm output root files, 36 DQM output files

@slava77
Copy link
Contributor

slava77 commented Oct 24, 2020

+1

for #31928 a7c4f7a

  • code changes are in line with the PR description: a part related to bad data checks is backported from GEM unpacker: bugfix and more cross checks #31863
  • jenkins tests pass and comparisons with the baseline show no (relevant) differences
  • a local test with a cosmic data from 337971 shows that a crash goes away with an expected warning message
%MSG-w GEMRawToDigiModule:  GEMRawToDigiModule:muonGEMDigis  24-Oct-2020 02:11:56 CEST Run: 337971 Event: 23852
unpacking error: unknown Chamber 1 unknown VFat 0 bad VFat 0

@cmsbuild
Copy link
Contributor

This pull request is fully signed and it will be integrated in one of the next CMSSW_11_1_X IBs (tests are also fine) and once validation in the development release cycle CMSSW_11_2_X is complete. 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
Copy link
Contributor

qliphy commented Oct 24, 2020

+1

@cmsbuild cmsbuild merged commit 3dab8f2 into cms-sw:CMSSW_11_1_X Oct 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants