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 (un)packer: fixes and support for nonstandard ZS(lite) modes #23417

Merged
merged 21 commits into from Jun 18, 2018

Conversation

pieterdavid
Copy link
Contributor

This PR contains fixes for some bugs in the SiStrip unpacker for 10bit ZS(lite) readout modes (found by @erikbutz when testing the new hybrid zero suppression for heavy ion data taking later this year), and adds support for all different packing schemes (readout modes and packet codes, many were missing) to the packer (used in simulation and to repack HI data in the HLT).
The unpacker changes are limited to the code used for 10bit packed data (plus the code in the EDProducer to add cases for the modes that were not taken into account there), so there should be no effect on what is used for normal pp data. The packer changes were tested by unpacking its output and comparing the digis to the original collections, taking into account the expected differences (see this config and related code).

with @alesaggio
CC: @OlivierBondu @vidalm @mmusich @echabert

pieterdavid and others added 16 commits May 21, 2018 17:01
- rename the PACKET_CODE definitions for ZS lite to ZS
- change the packetCode implementation: default (0) for ZS lite modes,
  taken from the third byte for ZS
Reason: for ZS lite there are no packet codes, the packing scheme
is given by the readout mode; for non-lite ZS it is in the channel data
- add "PacketCode" option to SiStripDigiToRawModule
- add packetCodeFromString conversion in SiStripFEDBufferComponents
- pass the packet code on to FEDBufferPayloadCreator::fillChannelBuffer
@cmsbuild
Copy link
Contributor

cmsbuild commented Jun 1, 2018

The code-checks are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

cmsbuild commented Jun 1, 2018

@cmsbuild
Copy link
Contributor

cmsbuild commented Jun 1, 2018

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

It involves the following packages:

EventFilter/SiStripRawToDigi

@perrotta, @cmsbuild, @slava77 can you please review it and eventually sign? Thanks.
@makortel, @Martin-Grunewald, @VinInn this is something you requested to watch as well.
@davidlange6, @slava77, @fabiocos you are the release manager for this.

cms-bot commands are listed here

@mmusich
Copy link
Contributor

mmusich commented Jun 1, 2018

@icali FYI

@slava77
Copy link
Contributor

slava77 commented Jun 1, 2018

@cmsbuild please test

@pieterdavid
Copy link
Contributor Author

Thanks @perrotta , that message (and the corresponding summary of unpacker warnings at the end of the job) is indeed present in the three jobs with differences. I will investigate

@perrotta
Copy link
Contributor

+1

  • SiStrip unpacker adapted for non-standard ZS modes as described: user tests show that the unpacked data reproduce the input ones
  • No effect is visible in standard workflows, were those non-standard ZS modes are not at work
  • Warning messages have also been reorganized and improved: this generate an extra warning (once per job) in the 2011 pp workflows. It has to be investigated, but outside the scope of this PR

@civanch
Copy link
Contributor

civanch commented Jun 17, 2018

+1

@pieterdavid
Copy link
Contributor Author

Output of the packer-unpacker closure test (run with this script), as requested during Friday's RECO meeting:

---> Testing VR packing on 5 events
Packet code :  cms.string('VIRGIN_RAW')
      5 Found 14710 dets with identical raw digis and 0 with differences
Packet code :  cms.string('VIRGIN_RAW8_BOTBOT')
      5 Found 14710 dets with identical raw digis and 0 with differences
Packet code :  cms.string('VIRGIN_RAW8_TOPBOT')
      5 Found 14710 dets with identical raw digis and 0 with differences
Packet code :  cms.string('VIRGIN_RAW10')
      5 Found 14710 dets with identical raw digis and 0 with differences
---> Testing ZS(lite) packing on 5 events
Readout mode:  cms.string('Zero suppressed')
Packet code :  cms.string('ZERO_SUPPRESSED')
Found 14690 dets with identical digis and 0 with differences
Found 14392 dets with identical digis and 0 with differences
Found 14687 dets with identical digis and 0 with differences
Found 14603 dets with identical digis and 0 with differences
Found 14247 dets with identical digis and 0 with differences
Readout mode:  cms.string('Zero suppressed')
Packet code :  cms.string('ZERO_SUPPRESSED8_BOTBOT')
Found 14690 dets with identical digis and 0 with differences
Found 14392 dets with identical digis and 0 with differences
Found 14687 dets with identical digis and 0 with differences
Found 14603 dets with identical digis and 0 with differences
Found 14247 dets with identical digis and 0 with differences
Readout mode:  cms.string('Zero suppressed')
Packet code :  cms.string('ZERO_SUPPRESSED8_TOPBOT')
Found 14690 dets with identical digis and 0 with differences
Found 14392 dets with identical digis and 0 with differences
Found 14687 dets with identical digis and 0 with differences
Found 14603 dets with identical digis and 0 with differences
Found 14247 dets with identical digis and 0 with differences
Readout mode:  cms.string('Zero suppressed')
Packet code :  cms.string('ZERO_SUPPRESSED10')
Found 14690 dets with identical digis and 0 with differences
Found 14392 dets with identical digis and 0 with differences
Found 14687 dets with identical digis and 0 with differences
Found 14603 dets with identical digis and 0 with differences
Found 14247 dets with identical digis and 0 with differences
Readout mode:  cms.string('Zero suppressed lite8')
Packet code :  cms.string('')
Found 14690 dets with identical digis and 0 with differences
Found 14392 dets with identical digis and 0 with differences
Found 14687 dets with identical digis and 0 with differences
Found 14603 dets with identical digis and 0 with differences
Found 14247 dets with identical digis and 0 with differences
Readout mode:  cms.string('Zero suppressed lite8 BotBot')
Packet code :  cms.string('')
Found 14690 dets with identical digis and 0 with differences
Found 14392 dets with identical digis and 0 with differences
Found 14687 dets with identical digis and 0 with differences
Found 14603 dets with identical digis and 0 with differences
Found 14247 dets with identical digis and 0 with differences
Readout mode:  cms.string('Zero suppressed lite8 TopBot')
Packet code :  cms.string('')
Found 14690 dets with identical digis and 0 with differences
Found 14392 dets with identical digis and 0 with differences
Found 14687 dets with identical digis and 0 with differences
Found 14603 dets with identical digis and 0 with differences
Found 14247 dets with identical digis and 0 with differences
Readout mode:  cms.string('Zero suppressed lite10')
Packet code :  cms.string('')
Found 14690 dets with identical digis and 0 with differences
Found 14392 dets with identical digis and 0 with differences
Found 14687 dets with identical digis and 0 with differences
Found 14603 dets with identical digis and 0 with differences
Found 14247 dets with identical digis and 0 with differences

@pieterdavid
Copy link
Contributor Author

On the "FED data missing" warnings (likely due to disabled FEDs): for the 1000.0 and 1001.0 workflows (run 165121) they are all about FED 87. For the 4.22 workflow (2011 cosmics, run 160960) the warning is there for three FEDs, including 87 (I tried to rerun with debugging on to get the other two IDs, but the das query doesn't return any results, and if I take the file list from the comparisons the cern xrootd server says that the file doesn't exist - indeed I do not see it on eos).

@erikbutz
Copy link

The other two missing FED IDs should be 103 and 388 which are two FEDs that have been excluded for a long time already, FED 87 was a transitory problem that so happened to be present in the run that is used.
In post-LS1 runs I would expect to get messages for FEDs missing for FEDs 59, 103 and 388 unless runs used there again happen to have a transitory problem.

@mmusich
Copy link
Contributor

mmusich commented Jun 18, 2018

Hello, just to confirm that also on the offline side,
FED 87 (see it in the TrackerMap below: TIB L2, lower right corner)

tmap_fed87

is seen in the list of components masked in the SiStrip cabling + excluded FEDs (from RunInfo O2O ) for run 165121:

mergedbadcomponentstkmap_run_165121

@mmusich
Copy link
Contributor

mmusich commented Jun 18, 2018

@pieterdavid

For the 4.22 workflow (2011 cosmics, run 160960) the warning is there for three FEDs, including 87 (I tried to rerun with debugging on to get the other two IDs, but the das query doesn't return any results, and if I take the file list from the comparisons the cern xrootd server says that the file doesn't exist - indeed I do not see it on eos).

for testing those workflows you can use the --ibeos option, that makes use of the global xrootd redirector when input files are not found at T2_CH_CERN :

  • runTheMatrix.py -l 4.22 --ibeos

see discussion at #22278

@perrotta
Copy link
Contributor

Thank you all for checking
Maybe the masked components could be taken into account and not trigger the warning message, then...

@pieterdavid
Copy link
Contributor Author

Thanks @mmusich , with --ibeos it worked. The other FEDs (besides 87) for 4.22 (run 160960) are 135 and 137. Since these are different than those @erikbutz mentioned, maybe they also had transient problems? The unpacker gets the list of FEDs to unpack from the cabling map (SiStripFedCabling).

@mmusich
Copy link
Contributor

mmusich commented Jun 18, 2018

Hi @pieterdavid , thanks for the further checks.
Both FED 135 and 137 are located in TID- Disk2:

tmap_fed135_137

and in my recollection they are not permanently out of the cabling. Indeed looks like a transient problem (also in this case both the FEDs are seen in the offline masking for the SiStripQuality for run 160960):

mergedbadcomponentstkmap_run_160960

@fabiocos
Copy link
Contributor

@pieterdavid @perrotta from the thread above I understand that the new messages appearing for 2011 data are related to a particular data period and understood, and should not represent a general problem of the PR. In any case, I consider that the legacy release to study Run1 remains the 53X.
If you confirm my understanding, I am ready to integrate this PR.

@fabiocos
Copy link
Contributor

+operations

the updates to the StandardSequences appear coherent with the rest of the PR

@cmsbuild
Copy link
Contributor

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. @davidlange6, @slava77, @smuzaffar, @fabiocos (and backports should be raised in the release meeting by the corresponding L2)

@perrotta
Copy link
Contributor

Yes @fabiocos , I confirm that the messages are only exposed by this PR (as new warning messages are issued which did not exist before unless in some debug mode).
Therefore, whatever is the origin of those messages (and yes, according to the messages above that origin seems to be understood), it does not depend nor can be originated by this PR

@fabiocos
Copy link
Contributor

+1

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

9 participants