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

SiStrip unpacker corrupted buffer fix #12385

Merged
merged 1 commit into from
Nov 12, 2015

Conversation

forthommel
Copy link
Contributor

The FEDBuffer object modified in #12157 to introduce the new FED readout modes was feeding corrupted data due to an improperly set variable (the switch between the legacy unpacker and its updated version).
Also, the packet code returned in the case of a ZS lite format was improperly set.

This commit has been tested against 4.53 and 140.53, and the baseline behaviour on the list of corrupted FED (<7_5_5) has been restored (4.53):

fedvsapvid
corruptedfeds

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @forthommel (Laurent Forthomme) for CMSSW_7_5_X.

It involves the following packages:

DataFormats/SiStripCommon
EventFilter/SiStripRawToDigi
RecoLocalTracker/SiStripClusterizer

@cmsbuild, @cvuosalo, @davidlange6, @slava77 can you please review it and eventually sign? Thanks.
@Martin-Grunewald, @makortel, @yduhm, @GiacomoSguazzoni, @gbenelli, @rovere, @VinInn, @nickmccoll, @jlagram, @gpetruc, @cerati, @threus this is something you requested to watch as well.

Following commands in first line of a comment are recognized

  • +1|approve[d]|sign[ed]: L1/L2's to approve it
  • -1|reject[ed]: L1/L2's to reject it
  • assign <category>[,<category>[,...]]: L1/L2's to request signatures from other categories
  • unassign <category>[,<category>[,...]]: L1/L2's to remove signatures from other categories
  • hold: L1/L2's to mark it as on hold
  • merge: L1 to merge this request
  • [@cmsbuild,] please test: L1/L2 and selected users to start jenkins tests
  • [@cmsbuild,] please test with cms-sw/cmsdist#<PR>: L1/L2 and selected users to start jenkins tests using externals from cmsdist

@Degano you are the release manager for this.
You can merge this pull request by typing 'merge' in the first line of your comment.

@forthommel
Copy link
Contributor Author

As an additional feature, the comments raised by @slava77 in the last commit of #12157 were addressed and implemented in this PR as well.

@slava77
Copy link
Contributor

slava77 commented Nov 11, 2015

@cmsbuild please test

@cmsbuild
Copy link
Contributor

@forthommel
Copy link
Contributor Author

Preparing to run 1003.0 RunMinBias2012A+RunMinBias2012A+RECODDQM+HARVESTDDQM
Slave went offline during the build
[...]
Finished: FAILURE

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

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

The workflows 140.53 have different files in step1_dasquery.log than the ones found in the baseline. You may want to check and retrigger the tests if necessary. You can check it in the "files" directory in the results of the comparisons

@davidlange6
Copy link
Contributor

@slava77 @cvuosalo - I will merge this - but please double check it in parallel with a patch release build. There are no differences in reco objects as expected.

davidlange6 added a commit that referenced this pull request Nov 12, 2015
SiStrip unpacker corrupted buffer fix
@davidlange6 davidlange6 merged commit 37469de into cms-sw:CMSSW_7_5_X Nov 12, 2015
@forthommel forthommel deleted the from-CMSSW_7_5_5 branch November 12, 2015 14:01
@slava77
Copy link
Contributor

slava77 commented Nov 12, 2015

+1
(post factum)

for #12385 ad3da7b

  • changes are in line with the description
  • jenkins tests passed, but are incomplete
    • comparisons for data workflows that had diffs in DQM before do not appear in the results (maybe a copy problem)
    • 140.53 HI Run1 workflow wasn't processed correctly due to DAS problems
  • tested locally in CMSSW_7_5_4 in a combination with New FED modes handled in SiStrip unpacker #12157 74e87c0: the combination is expected to give no differences. No differences were observed in the monitored quantities.

slava77 pushed a commit to slava77/cmssw that referenced this pull request Nov 13, 2015
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.

4 participants