Changed CSC binary examiner mask to enable ALCT CRC checks - CMSSW_10_2_X #24294
Conversation
|
A new Pull Request was created by @barvic (Victor Barashko) for CMSSW_10_2_X. It involves the following packages: EventFilter/CSCRawToDigi @perrotta, @cmsbuild, @slava77 can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
|
please test |
|
The tests are being triggered in jenkins. |
|
backport of #24295 |
|
Thank you @barvic for the explanations you provided in #24295 (comment) . About your last point [1], could you please elaborate here a bit more? If we merge this backport, we should understand whether any difference with the previous recorded data is expected, or if it is really only on the handling of corrupted data. Do you have any slide or presentation that explains what we should expect in detail? [1] |
|
Comparison job queued. |
|
Comparison is ready Comparison Summary:
|
|
@perrotta Don't really have any slides. This crash issue popped up just this Monday. |
|
@Martin-Grunewald, @ptcox (since @barvic is away in these days): do you have an idea how this mask works, and at which level? Or do you know who can we ask about it? For the possible backport I think one should better understand whether regular data taking can be affected at all, with differences in the expected event output, and the explanation in #24294 (comment) looks a bit too much generic (at least for me). I must admit that I tried to dig a bit in the code, but I was not able to find a clear answer to that question... Thank you. |
|
Hi Andrea,
Victor (possibly together with Valdas, when he worked on CSC) IS the author of the code. It is not like he is making some unwarranted assumption about how the unpacking code works, or is supposed to work. The CSC unpacker code in EventFilter/CSCRawToDigi has two main components - the Examiner (CSCDCCExaminer.cc), and the Unpacker. The Examiner checks that the CSC raw data formats are as expected, before unpacking proceeds.
What Victor explained to me in an earlier email is
"in general the Examiner is able to catch such corruption and reject events while it has ALCT CRC calculation and error check enabled via corresponding unpacker's error mask bit set in config. But I see that it is currently disabled, so such events would pass to the unpacking code with unpredictable results."
Victor discovered it had been deactivated since Run 1 and thinks its reactivation was just forgotten after some investigation of corrupt data in the past. Similar code is activated, and has always been activated, for other parts of the CSC data stream. The only effect should be to avoid attempting to unpack data which fail the check and hence do not have the expected format. I can see no downside to activating the check for ALCT data, just as they are for other parts of the CSC event.
On our request, Victor wrote up a CSC elgo entry with more details if you are keen to know more:
http://cmsonline.cern.ch/cms-elog/1058673
Here is where the bit that has just been set in the config mask is actually tested:
https://cmssdt.cern.ch/lxr/source/EventFilter/CSCRawToDigi/plugins/CSCDCCUnpacker.cc#0263
Regards, Tim
…On Aug 20, 2018, 15:43 +0200, perrotta ***@***.***>, wrote:
@Martin-Grunewald, @ptcox (since @barvic is away in these days): do you have an idea how this mask works, and at which level? Or do you know who can we ask about it?
For the possible backport I think one should better understand whether regular data taking can be affected at all, with differences in the expected event output, and the explanation in #24294 (comment) looks a bit too much generic (at least for me). I must admit that I tried to dig a bit in the code, but I was not able to find a clear answer to that question...
Thank you.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.
|
|
Thank you Tim, this is exactly the explanation I was looking for. It
was my fault not having been fast enough to ask Victor before he left
for vacations.
Now I understand that such a mask is for the unpacker error flag'
bits, and it must remain activated in order to prevent unpacking data
when those errors are present.
Given so, I see no issues or counterindications in the backport.
ptcox <notifications@github.com> ha scritto:
… Hi Andrea,
Victor (possibly together with Valdas, when he worked on CSC) IS the
author of the code. It is not like he is making some unwarranted
assumption about how the unpacking code works, or is supposed to
work. The CSC unpacker code in EventFilter/CSCRawToDigi has two main
components - the Examiner (CSCDCCExaminer.cc), and the Unpacker. The
Examiner checks that the CSC raw data formats are as expected,
before unpacking proceeds.
What Victor explained to me in an earlier email is
"in general the Examiner is able to catch such corruption and reject
events while it has ALCT CRC calculation and error check enabled via
corresponding unpacker's error mask bit set in config. But I see
that it is currently disabled, so such events would pass to the
unpacking code with unpredictable results."
Victor discovered it had been deactivated since Run 1 and thinks
its reactivation was just forgotten after some investigation of
corrupt data in the past. Similar code is activated, and has always
been activated, for other parts of the CSC data stream. The only
effect should be to avoid attempting to unpack data which fail the
check and hence do not have the expected format. I can see no
downside to activating the check for ALCT data, just as they are for
other parts of the CSC event.
On our request, Victor wrote up a CSC elgo entry with more details
if you are keen to know more:
http://cmsonline.cern.ch/cms-elog/1058673
Here is where the bit that has just been set in the config mask is
actually tested:
https://cmssdt.cern.ch/lxr/source/EventFilter/CSCRawToDigi/plugins/CSCDCCUnpacker.cc#0263
Regards, Tim
On Aug 20, 2018, 15:43 +0200, perrotta ***@***.***>, wrote:
> @Martin-Grunewald, @ptcox (since @barvic is away in these days): do
> you have an idea how this mask works, and at which level? Or do you
> know who can we ask about it?
> For the possible backport I think one should better understand
> whether regular data taking can be affected at all, with
> differences in the expected event output, and the explanation in
> #24294 (comment) looks a bit too much generic (at least for me). I
> must admit that I tried to dig a bit in the code, but I was not
> able to find a clear answer to that question...
> Thank you.
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub, or mute the thread.
--
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub:
#24294 (comment)
|
|
+1
|
cmsbuild
added
fully-signed
reconstruction-approved
and removed
reconstruction-pending
labels
|
This pull request is fully signed and it will be integrated in one of the next CMSSW_10_2_X IBs (tests are also fine) and once validation in the development release cycle CMSSW_10_3_X is complete. 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) |
|
+1 |
EventFilter/CSCRawToDigi - Changed CSC binary examiner mask to enable ALCT CRC checks.
It should prevent passing of corrupted (bad ALCT) CSC event data to main CSC data unpacking code.
Related to request from HLT group, which was investigating random HLT crashes in run 320917