Fix logic in PFClusterSoAProducer kernels #44273

jsamudio · 2024-02-29T18:30:03Z

PR description:

This PR fixes an issue with the clustering logic in PFClusterSoAProducer. The issue was highlighted by 2024 thresholds giving rise to topological clusters exceeding 100 seeds, which we had never naturally encountered in previous validations of the code in order to catch this flaw. The fall-back kernel was not being launched as intended in the previous logic, and is now changed to behave as intended.

PR validation:

For CPU-serial and GPU alpaka backends:
Validated QCDPU sample on 2022, 2023, and 2024 globalTag thresholds, and validated change in Pixel+ECAL+PF Alpaka HLT test workflow.

Cluster-level validation plots can be found here: https://hep.baylor.edu/jsamudio/fixLogic/

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:

Not a backport, but probably needs to be included in 14_0_0 or later 14_0_X for HLT integration. Although I am not sure how to proceed since the 14_0_0 release is closed.

@hatakeyamak @waredjeb @fwyzard @swagata87 @stahlleiton

cmsbuild · 2024-02-29T18:30:33Z

cms-bot internal usage

fwyzard · 2024-02-29T18:32:15Z

RecoParticleFlow/PFClusterProducer/plugins/alpaka/PFClusterSoAProducerKernel.dev.cc

@@ -1429,7 +1429,8 @@ namespace ALPAKA_ACCELERATOR_NAMESPACE {
        int nRHTopo = pfClusteringVars[topoId].topoRHCount();
        int nSeeds = pfClusteringVars[topoId].topoSeedCount();

-        if (nRHTopo > 0 && nSeeds > 400 && nRHTopo - nSeeds > 1500) {
+        // nSeeds value must match multiSeedIterative in FastCluster
+        if (nRHTopo > 0 && nSeeds > 400) {


Shouldn't this be

Suggested change

if (nRHTopo > 0 && nSeeds > 400) {

if (nRHTopo > 0 && nSeeds > 400 || nRHTopo - nSeeds > 1500) {

?

cmsbuild · 2024-02-29T18:38:16Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-44273/39290

This PR adds an extra 24KB to repository

cmsbuild · 2024-02-29T18:38:37Z

A new Pull Request was created by @jsamudio for master.

It involves the following packages:

RecoParticleFlow/PFClusterProducer (reconstruction)

@cmsbuild, @jfernan2, @mandrenguyen can you please review it and eventually sign? Thanks.
@seemasharmafnal, @lgray, @sameasy, @mmarionncern, @felicepantaleo, @hatakeyamak, @rovere, @missirol this is something you requested to watch as well.
@antoniovilela, @rappoccio, @sextonkennedy you are the release manager for this.

cms-bot commands are listed here

Add non-seed rechit condition in exotic clustering

cmsbuild · 2024-02-29T19:41:20Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-44273/39293

This PR adds an extra 20KB to repository

cmsbuild · 2024-02-29T19:41:46Z

Pull request #44273 was updated. @mandrenguyen, @jfernan2, @cmsbuild can you please check and sign again.

jsamudio · 2024-02-29T20:30:58Z

test parameters:

enable = gpu
workflows = 12434.422,12434.423,12434.424,12434.492
workflows_gpu = 12434.422,12434.423,12434.424,12434.492
workflow_opts = -w upgrade
workflow_opts_gpu = -w upgrade

hatakeyamak · 2024-02-29T21:27:51Z

Maybe somebody can trigger jenkins?

mandrenguyen · 2024-02-29T21:36:53Z

please test

fwyzard · 2024-02-29T22:24:51Z

type bugfix

fwyzard · 2024-02-29T22:25:17Z

urgent

the backport is needed for data taking

fwyzard · 2024-02-29T22:26:59Z

allow @jsamudio test rights

fwyzard · 2024-02-29T22:27:15Z

(in case it needs to be updated and re-tested)

missirol · 2024-02-29T23:26:03Z

test parameters:

enable = gpu

workflows = 12434.422,12434.423,12434.424,12434.492

workflows_gpu = 12434.422,12434.423,12434.424,12434.492

workflow_opts = -w upgrade

workflow_opts_gpu = -w upgrade

I don't think the bot picked up #44273 (comment) (maybe because not done by ORP/L2s and only before @jsamudio was given rights). The bot should give a thumbs-up when it receives it.

jsamudio · 2024-02-29T23:38:47Z

@cmsbuild, please abort

jsamudio · 2024-02-29T23:39:33Z

@cmsbuild, test parameters:

enable = gpu
workflows = 12434.422,12434.423,12434.424,12434.492
workflows_gpu = 12434.422,12434.423,12434.424,12434.492
workflow_opts = -w upgrade
workflow_opts_gpu = -w upgrade

jsamudio · 2024-02-29T23:40:36Z

@cmsbuild, please test

swagata87 · 2024-03-01T00:50:51Z

type pf

cmsbuild · 2024-03-01T02:41:41Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-befbd6/37840/summary.html
COMMIT: 8d6e1dd
CMSSW: CMSSW_14_1_X_2024-02-29-1100/el8_amd64_gcc12
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/44273/37840/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

You potentially added 3 lines to the logs
Reco comparison results: 53 differences found in the comparisons
DQMHistoTests: Total files compared: 49
DQMHistoTests: Total histograms compared: 3338784
DQMHistoTests: Total failures: 6
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3338756
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 48 files compared)
Checked 205 log files, 166 edm output root files, 49 DQM output files
TriggerResults: no differences found

fwyzard · 2024-03-01T07:09:45Z

assign heterogeneous

fwyzard · 2024-03-01T07:09:56Z

+heterogeneous

cmsbuild · 2024-03-01T07:10:06Z

New categories assigned: heterogeneous

@fwyzard,@makortel you have been requested to review this Pull request/Issue and eventually sign? Thanks

mandrenguyen · 2024-03-01T08:19:28Z

+1

cmsbuild · 2024-03-01T08:19:51Z

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, @antoniovilela, @rappoccio (and backports should be raised in the release meeting by the corresponding L2)

antoniovilela · 2024-03-01T10:58:11Z

+1

cmsbuild added this to the CMSSW_14_1_X milestone Feb 29, 2024

cmsbuild added reconstruction-pending pending-signatures tests-pending orp-pending code-checks-pending labels Feb 29, 2024

fwyzard reviewed Feb 29, 2024

View reviewed changes

cmsbuild added code-checks-approved and removed code-checks-pending labels Feb 29, 2024

Fix logic in FastCluster kernels

8d6e1dd

Add non-seed rechit condition in exotic clustering

jsamudio force-pushed the fix_AlpakaPFLogic branch from 4fae676 to 8d6e1dd Compare February 29, 2024 19:35

cmsbuild added code-checks-pending and removed code-checks-approved labels Feb 29, 2024

cmsbuild added code-checks-approved and removed code-checks-pending labels Feb 29, 2024

jsamudio mentioned this pull request Feb 29, 2024

[14_0_X] Fix logic in PFClusterSoAProducer kernels #44278

Merged

cmsbuild added tests-started and removed tests-pending labels Feb 29, 2024

cmsbuild added the bug-fix label Feb 29, 2024

cmsbuild added the urgent label Feb 29, 2024

cmsbuild added the allow-jsamudio label Feb 29, 2024

cmsbuild added tests-pending and removed tests-started labels Feb 29, 2024

cmsbuild added tests-started and removed tests-pending labels Feb 29, 2024

cmsbuild added the pf label Mar 1, 2024

cmsbuild added tests-approved and removed tests-started labels Mar 1, 2024

cmsbuild added the heterogeneous-approved label Mar 1, 2024

cmsbuild added reconstruction-approved fully-signed and removed reconstruction-pending pending-signatures labels Mar 1, 2024

cmsbuild added orp-approved and removed orp-pending labels Mar 1, 2024

cmsbuild merged commit 0d05219 into cms-sw:master Mar 1, 2024
12 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix logic in PFClusterSoAProducer kernels #44273

Fix logic in PFClusterSoAProducer kernels #44273

jsamudio commented Feb 29, 2024

cmsbuild commented Feb 29, 2024 •

edited

Loading

fwyzard Feb 29, 2024

jsamudio Feb 29, 2024

cmsbuild commented Feb 29, 2024

cmsbuild commented Feb 29, 2024

cmsbuild commented Feb 29, 2024

cmsbuild commented Feb 29, 2024

jsamudio commented Feb 29, 2024 •

edited

Loading

hatakeyamak commented Feb 29, 2024

mandrenguyen commented Feb 29, 2024

fwyzard commented Feb 29, 2024

fwyzard commented Feb 29, 2024

fwyzard commented Feb 29, 2024

fwyzard commented Feb 29, 2024

missirol commented Feb 29, 2024

jsamudio commented Feb 29, 2024

jsamudio commented Feb 29, 2024 •

edited

Loading

jsamudio commented Feb 29, 2024

swagata87 commented Mar 1, 2024

cmsbuild commented Mar 1, 2024

fwyzard commented Mar 1, 2024

fwyzard commented Mar 1, 2024

cmsbuild commented Mar 1, 2024

mandrenguyen commented Mar 1, 2024

cmsbuild commented Mar 1, 2024

antoniovilela commented Mar 1, 2024

	if (nRHTopo > 0 && nSeeds > 400) {
	if (nRHTopo > 0 && nSeeds > 400 \|\| nRHTopo - nSeeds > 1500) {

Fix logic in PFClusterSoAProducer kernels #44273

Fix logic in PFClusterSoAProducer kernels #44273

Conversation

jsamudio commented Feb 29, 2024

PR description:

PR validation:

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:

cmsbuild commented Feb 29, 2024 • edited Loading

fwyzard Feb 29, 2024

Choose a reason for hiding this comment

jsamudio Feb 29, 2024

Choose a reason for hiding this comment

cmsbuild commented Feb 29, 2024

cmsbuild commented Feb 29, 2024

cmsbuild commented Feb 29, 2024

cmsbuild commented Feb 29, 2024

jsamudio commented Feb 29, 2024 • edited Loading

hatakeyamak commented Feb 29, 2024

mandrenguyen commented Feb 29, 2024

fwyzard commented Feb 29, 2024

fwyzard commented Feb 29, 2024

fwyzard commented Feb 29, 2024

fwyzard commented Feb 29, 2024

missirol commented Feb 29, 2024

jsamudio commented Feb 29, 2024

jsamudio commented Feb 29, 2024 • edited Loading

jsamudio commented Feb 29, 2024

swagata87 commented Mar 1, 2024

cmsbuild commented Mar 1, 2024

Comparison Summary

fwyzard commented Mar 1, 2024

fwyzard commented Mar 1, 2024

cmsbuild commented Mar 1, 2024

mandrenguyen commented Mar 1, 2024

cmsbuild commented Mar 1, 2024

antoniovilela commented Mar 1, 2024

cmsbuild commented Feb 29, 2024 •

edited

Loading

jsamudio commented Feb 29, 2024 •

edited

Loading

jsamudio commented Feb 29, 2024 •

edited

Loading