-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
[DBG_X] RelVal 140.58 Step 2: Corrupted data in SiStripNoises::getNoise #42162
Comments
assign reconstruction |
New categories assigned: reconstruction @mandrenguyen,@clacaputo you have been requested to review this Pull request/Issue and eventually sign? Thanks |
A new Issue was created by @iarspider . @Dr15Jones, @perrotta, @dpiparo, @rappoccio, @makortel, @smuzaffar can you please review it and eventually sign/assign? Thanks. cms-bot commands are listed here |
Both this and the other companion issue #42131 are caused because there is a condition (
when it looks for the noise of strip n. 768 for Detid 369120277. |
Hello! There are three new workflows failing with the same issue for CMSSW_13_2 2023-07-13-2300 (latest DBG IB). I link the logs here: 159.03, 160.02 and 160.03. |
There is a workaround to this (to just not use that strip) but, still, the question stands as to why the payload stored in the tag SiStripNoise_v2_prompt for run 326417 (which is the run used for wf 140.58) is missing this particular strip as @mmusich mentioned. |
assign alca FYI @cms-sw/trk-dpg-l2 |
New categories assigned: alca @perrotta,@consuegs,@francescobrivio,@saumyaphor4252,@tvami you have been requested to review this Pull request/Issue and eventually sign? Thanks |
I dont know how one can figure out why that strips is missing since this is a story from 2018 (Nov 9, 2018)... By putting "326417" into my gmail, I find that the payload is coming from an O2O from some HI running. The msg from Raffaele says "after the first HI fills of yesterday, we saw, as expected from the online calibrations loaded one week ago, a reduction in the S/N particularly visible in TID and TECs. With this o2o, a new set of pedestal and noise values measured yesterday are deployed offline. We can profit of the next cosmics/collisions to quickly validate them. In case they will create troubles, a roll back is possible in 10 minutes. As far as i can tell, no large differences observed in pedestals, reduction of noise in TEC/TID and inner TIB layers of about 5%." which is followed by an unanswered question from Andrea Venturi saying " I am wondering if there is a way to produce S/N distributions selecting only (on-track) clusters in APVs that were zero-suppressed in the standard way. Just to be sure that the hybrid ZS and the clusterizer do not introduce a reduction of S and, therefore, of S/N." --> so was that strip ZS differently than the rest and somehow didnt enter the payload? That's all I can offer. Actually here is the tracker map of the noise in the relevant payload: https://cern.ch/u3ouy And I also pingged the strips convenors |
I checked a recent noise payload, and it seems that this strip never exists. For this detId (and similar modules), the strip range is from 0 to 767. |
One would presume that protecting this call: cmssw/RecoLocalTracker/SiStripClusterizer/plugins/SiStripClusters2ApproxClusters.cc Line 134 in c2e865a
with the output of this filter: cmssw/RecoLocalTracker/SiStripClusterizer/plugins/SiStripClusters2ApproxClusters.cc Line 129 in c2e865a
would prevent reading off domain, but apparently this condition:
is not restrictive enough. |
Given that the access in
is with firstStrip + 1 == 768 , and firstStrip == 767 seems to be a valid value for a strip (i.e. assuming on the caller sidecmssw/RecoLocalTracker/SiStripClusterizer/plugins/SiStripClusters2ApproxClusters.cc Line 134 in c2e865a
observing the last strip of a module is ok), could it be the PeakFinderTest does not properly handle the edge case?
Actually, why does the |
I think that's possiblity, but I am wondering why we observe this now (a very similar code was already used in cmssw/RecoTracker/PixelLowPtUtilities/src/StripSubClusterShapeTrajectoryFilter.cc Line 84 in c2e865a
that's also the question I asked myself, but could not find an answer. I suspect all of this code is from Run1. |
Do I understand correctly that this is present only in DBG_X ? (not in other builds) |
A similar issue happens in ASAN_X: #42131 |
The exception occurs only in DBG_X builds because the cmssw/CondFormats/SiStripObjects/interface/SiStripNoises.h Lines 73 to 78 in c2e865a
To me it looks like the issue is there in all builds, but doesn't show any (technical) sign exception in DBG_X and ASAN_X. |
type trk |
answering to myself, this doesn't happen because of this check: cmssw/RecoTracker/PixelLowPtUtilities/src/StripSubClusterShapeTrajectoryFilter.cc Lines 265 to 267 in c2e865a
I propose the same kind of protection at #42486 |
The following exceptions are reported:
There are also warnings about NULL pointers to FEDRawData, not sure if they are related or harmless:
The text was updated successfully, but these errors were encountered: