Fix crash in PF Clustering due to empty HCAL RecHits collection #44693
Conversation
|
cms-bot internal usage |
|
-code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-44693/39900
Code check has found code style and quality issues which could be resolved by applying following patch(s)
|
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-44693/39901
|
|
@cmsbuild, please test |
cmsbuild
added
tests-started
code-checks-approved
and removed
tests-pending
code-checks-pending
labels
|
A new Pull Request was created by @jsamudio for master. It involves the following packages:
@mandrenguyen, @jfernan2 can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
|
type pf |
| for (const auto& token : recHitsToken_) | ||
| kernel.processRecHits(event.queue(), event.get(token.first), setup.getData(token.second), topology, pfRecHits); | ||
| kernel.associateTopologyInfo(event.queue(), topology, pfRecHits); | ||
| if (pfRecHits->metadata().size() != 0) { |
There was a problem hiding this comment.
| if (pfRecHits->metadata().size() != 0) { | |
| if (num_recHits != 0) { |
?
| reco::PFRecHitCollection out; | ||
| out.reserve(alpakaPfRecHits.size()); | ||
| int reserveSize = 0; | ||
| if (alpakaPfRecHits.metadata().size() != 0) | ||
| reserveSize = alpakaPfRecHits.size(); | ||
| out.reserve(reserveSize); | ||
|
|
||
| for (size_t i = 0; i < alpakaPfRecHits.size(); i++) { | ||
| reco::PFRecHit& pfrh = | ||
| out.emplace_back(caloGeo_.at(alpakaPfRecHits[i].layer())->getGeometry(alpakaPfRecHits[i].detId()), | ||
| alpakaPfRecHits[i].detId(), | ||
| alpakaPfRecHits[i].layer(), | ||
| alpakaPfRecHits[i].energy()); | ||
| pfrh.setTime(alpakaPfRecHits[i].time()); | ||
| pfrh.setDepth(alpakaPfRecHits[i].depth()); | ||
| if (alpakaPfRecHits.metadata().size() != 0) { |
There was a problem hiding this comment.
this could be more simply
reco::PFRecHitCollection out;
if (alpakaPfRecHits.metadata().size() != 0) {
out.reserve(alpakaPfRecHits.size());
for (size_t i = 0; i < alpakaPfRecHits.size(); i++) {
...
|
assign heterogeneous |
|
+heterogeneous |
|
just to say that the changes look good to me |
|
please test |
|
to see if the DAS errors are only transient |
|
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-1e1055/38756/summary.html Comparison SummarySummary:
GPU Comparison SummarySummary:
|
|
+reconstruction |
cmsbuild
added
reconstruction-approved
fully-signed
and removed
reconstruction-pending
pending-signatures
labels
|
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. @sextonkennedy, @rappoccio, @antoniovilela (and backports should be raised in the release meeting by the corresponding L2) |
|
+1 |
PR description:
This fix addresses #44668 where Alpaka-based PF Clustering was crashing at HLT when HCAL is off. The reason was due to the input HCAL RecHit number being used to specify the work division for kernels, and it was not protected against a case of requesting 0 blocks when there are no RecHits.
Included now are checks on the number of rechits and the SoA size to only launch the Alpaka kernels if they are non-zero.
PR validation:
In
14_1_0_pre2, tested on matrix workflows12434.422, 12434.424. The missing HCAL validation for HLT is done in the backport.