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
Fix SiPixelRawToClusterGPUKernel
for spurious ROCs [12.4.x]
#39713
Fix SiPixelRawToClusterGPUKernel
for spurious ROCs [12.4.x]
#39713
Conversation
type bugfix |
A new Pull Request was created by @fwyzard (Andrea Bocci) for CMSSW_12_4_X. It involves the following packages:
@cmsbuild, @mandrenguyen, @clacaputo can you please review it and eventually sign? Thanks. cms-bot commands are listed here
|
backport #39711 |
enable gpu |
please test |
// check for spurious channels | ||
if (roc > MAX_ROC or link > MAX_LINK) { | ||
if constexpr (debug) { | ||
printf("spurious roc %d found on link %d, detector %d (index %d)\n", roc, link, rawId, gIndex); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rawId is "of course" wrong at this point...
one could use
getRawId(cablingMap, fedId, link, 1).rawId;
under the assumption that all rocs in a link belong to the same det
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks.
Pull request #39713 was updated. @cmsbuild, @mandrenguyen, @clacaputo can you please check and sign again. |
02f4852
to
3b29a91
Compare
Pull request #39713 was updated. @cmsbuild, @mandrenguyen, @clacaputo can you please check and sign again. |
please test |
-1 Failed Tests: RelVals-GPU RelVals-GPU
Comparison SummarySummary:
|
means that (some of) the test machines need to reload the NVIDIA drivers. |
@fwyzard , we do not have any dedicated GPU resource so we use HTCondor GPU resources for PR/IB tests. I will ask CERN IT to reload the drivers for these resources. [a]
|
OK, thanks. |
-1 Failed Tests: RelVals-INPUT RelVals-INPUTThe relvals timed out after 4 hours. Comparison SummarySummary:
GPU Comparison SummarySummary:
|
urgent TSG asked for a patch release including this PR, for HLT online operations. @cms-sw/reconstruction-l2 , please prioritise the review of this PR (the |
please test It's clear that this PR passes the tests. We can anyway try to 'clean up the labels' overnight. I admit I don't know the cause behind these recurring "RelVals-INPUT" errors; I wonder whether it is not solvable, or if there is an issue about it. |
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-c7a516/28245/summary.html Comparison SummarySummary:
GPU Comparison SummarySummary:
|
Status described here: #39045 (comment) |
+1 |
This pull request is fully signed and it will be integrated in one of the next CMSSW_12_4_X IBs (tests are also fine) and once validation in the development release cycle CMSSW_12_6_X is complete. This pull request will now be reviewed by the release team before it's merged. @perrotta, @dpiparo, @rappoccio (and backports should be raised in the release meeting by the corresponding L2) |
+1 |
PR description:
Make the implementation of the pixel unpacker running on GPU skip spurious ROCs similar to the legacy version of the unpacker.
Fixes an HLT crash that has been happening more or less regularly for the past two months.
See #39045 for more details on the problem, investigation, and tests.
Also, changes the
debug
function parameter to a template parameter, to enable or disable the printouts at compile time.PR validation:
Running the HLT over the error stream files does not crash any more.
If this PR is a backport please specify the original PR and why you need to backport that PR. If this PR will be backported please specify to which release cycle the backport is meant for:
Backport of #39711 to 12.4.x for data taking.