[14_1_X] Add runtime allocation of PF rechit fraction SoA #46136

jsamudio · 2024-09-26T14:43:34Z

PR description:

This PR is made to address crashes of the type in #45477 and #44634. It provides runtime allocation of the PF RecHit Fraction SoA to minimize memory usage and avoid crashes when the allocation needed was not a reasonable multiple of the number of PF RecHits. This change is made in a way which does not touch the configuration, and therefore is transparent to the HLT menu.

PR validation:

PR physics performance was validated using the matrix workflow 12434.423 and checking output Legacy CPU vs GPU comparisons. The event throughput was tested on a HLT test machine with the following results:

Running 3 times over 6300 events with 8 jobs, each with 32 threads, 32 streams and 1 GPUs

Reference: 565.8 ±   0.3 ev/s

This PR: 564.7 ±   1.3 ev/s

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:

Verbatim backport of #46135 for 14_1_X
Backport of #46135 for 14_1_X, but differs by one commit which makes the pfRecHitFractionAllocation parameter optional so that this update will not conflict with the current HLT menu. Also does not include the change to the menu customization.

@fwyzard @waredjeb

cmsbuild · 2024-09-26T14:43:59Z

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

It involves the following packages:

RecoParticleFlow/PFClusterProducer (reconstruction)

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

cms-bot commands are listed here

Backported from Add runtime allocation of PF rechit fraction SoA #46135

cmsbuild · 2024-09-26T14:43:59Z

cms-bot internal usage

mmusich · 2024-09-26T14:45:22Z

backport of #46135

jfernan2 · 2024-09-26T18:06:42Z

please test

cmsbuild · 2024-09-26T21:47:57Z

+1

Size: This PR adds an extra 28KB to repository
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-29c216/41772/summary.html
COMMIT: 155d540
CMSSW: CMSSW_14_1_X_2024-09-26-1100/el8_amd64_gcc12
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/46136/41772/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

You potentially removed 2 lines from the logs
Reco comparison results: 242 differences found in the comparisons
DQMHistoTests: Total files compared: 44
DQMHistoTests: Total histograms compared: 3331166
DQMHistoTests: Total failures: 5399
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3325747
DQMHistoTests: Total skipped: 20
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 43 files compared)
Checked 191 log files, 161 edm output root files, 44 DQM output files
TriggerResults: no differences found

jfernan2 · 2024-09-30T08:05:43Z

+1

cmsbuild · 2024-09-30T08:06:03Z

This pull request is fully signed and it will be integrated in one of the next CMSSW_14_1_X IBs (tests are also fine) and once validation in the development release cycle CMSSW_14_2_X is complete. This pull request will now be reviewed by the release team before it's merged. @mandrenguyen, @sextonkennedy, @antoniovilela, @rappoccio (and backports should be raised in the release meeting by the corresponding L2)

mandrenguyen · 2024-09-30T13:57:57Z

hold
Just to remind that master version is also on hold

cmsbuild · 2024-09-30T13:58:20Z

Pull request has been put on hold by @mandrenguyen
They need to issue an unhold command to remove the hold state or L1 can unhold it for all

mandrenguyen · 2024-10-03T05:06:03Z

unhold

mmusich · 2024-10-04T16:43:38Z

Not sure it's the best solution, but I don't see a problem with closing this PR and opening a new one where you cherry-pick only the relevant commits.

I'd rather rebase against CMSSW_14_1_X to avoid loosing the history. If you cherry pick the commits you can force push on this same branch (no need to close this PR)

Fixed to work with latest updates Fix for case with 0 nRH Fix bad allocation crash Remove extra lines Simplify memcpy to single int Code checks and formatting Implement review suggestions

Adjust formatting Rename data member and remove unnecessary include

cmsbuild · 2024-10-04T16:47:12Z

Pull request #46136 was updated. @Martin-Grunewald, @cmsbuild, @fwyzard, @jfernan2, @makortel, @mandrenguyen, @mmusich can you please check and sign again.

mmusich · 2024-10-04T17:02:19Z

enable gpu

mmusich · 2024-10-04T17:02:28Z

@cmsbuild, please test

cmsbuild · 2024-10-04T20:26:24Z

+1

Size: This PR adds an extra 20KB to repository
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-29c216/41985/summary.html
COMMIT: 8ccf6b5
CMSSW: CMSSW_14_1_X_2024-10-04-1100/el8_amd64_gcc12
Additional Tests: GPU
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/46136/41985/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

No significant changes to the logs found
Reco comparison results: 24 differences found in the comparisons
DQMHistoTests: Total files compared: 44
DQMHistoTests: Total histograms compared: 3331166
DQMHistoTests: Total failures: 1429
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3329717
DQMHistoTests: Total skipped: 20
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 43 files compared)
Checked 191 log files, 161 edm output root files, 44 DQM output files
TriggerResults: no differences found

GPU Comparison Summary

Summary:

No significant changes to the logs found
Reco comparison results: 0 differences found in the comparisons
DQMHistoTests: Total files compared: 7
DQMHistoTests: Total histograms compared: 53015
DQMHistoTests: Total failures: 25
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 52990
DQMHistoTests: Total skipped: 0
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 6 files compared)
Checked 24 log files, 30 edm output root files, 7 DQM output files
TriggerResults: no differences found

mmusich · 2024-10-04T20:42:28Z

+hlt

for jsamudio@8ccf6b5
verbatim backport of Add runtime allocation of PF rechit fraction SoA #46135 to the 2024 PRef and HIon run data-taking release;

fwyzard · 2024-10-04T21:12:06Z

+1

mandrenguyen · 2024-10-05T05:33:05Z

+1
Assuming reco re-sign

cmsbuild · 2024-10-05T05:33:28Z

This pull request is fully signed and it will be integrated in one of the next CMSSW_14_1_X IBs (tests are also fine) and once validation in the development release cycle CMSSW_14_2_X is complete. This pull request will be automatically merged.

cmsbuild added this to the CMSSW_14_1_X milestone Sep 26, 2024

cmsbuild added reconstruction-pending pending-signatures tests-pending orp-pending labels Sep 26, 2024

cmsbuild added the backport label Sep 26, 2024

cmsbuild added tests-started and removed tests-pending labels Sep 26, 2024

cmsbuild added tests-approved and removed tests-started labels Sep 26, 2024

cmsbuild added reconstruction-approved fully-signed and removed reconstruction-pending pending-signatures labels Sep 30, 2024

jsamudio mentioned this pull request Sep 30, 2024

Add runtime allocation of PF rechit fraction SoA #46135

Merged

cmsbuild added pending-signatures hold and removed fully-signed labels Sep 30, 2024

cmsbuild removed pending-signatures hold labels Oct 3, 2024

jsamudio added 3 commits October 4, 2024 11:46

Add dynamic allocation of the PFRecHitFraction SoA

76edb06

Fixed to work with latest updates Fix for case with 0 nRH Fix bad allocation crash Remove extra lines Simplify memcpy to single int Code checks and formatting Implement review suggestions

Update HLT customization for removed PFClusterSoAProducer parameter

6c03ad0

Rename variables and adjust allocation syntax

8ccf6b5

Adjust formatting Rename data member and remove unnecessary include

jsamudio force-pushed the dev_pfrhfAlloc_14_1_X branch from 36f2bb0 to 8ccf6b5 Compare October 4, 2024 16:46

cmsbuild added tests-started and removed tests-pending labels Oct 4, 2024

cmsbuild added tests-approved and removed tests-started labels Oct 4, 2024

cmsbuild added hlt-approved and removed hlt-pending labels Oct 4, 2024

cmsbuild added heterogeneous-approved and removed heterogeneous-pending labels Oct 4, 2024

cmsbuild added reconstruction-approved fully-signed orp-approved and removed reconstruction-pending pending-signatures orp-pending labels Oct 5, 2024

cmsbuild merged commit 4bc3e62 into cms-sw:CMSSW_14_1_X Oct 5, 2024
12 checks passed

This was referenced Oct 5, 2024

HLT Crashes in HCAL PFClustering in Cosmics Run 383219 #45477

Closed

HLT Farm crashes in run 378940 #44634

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[14_1_X] Add runtime allocation of PF rechit fraction SoA #46136

[14_1_X] Add runtime allocation of PF rechit fraction SoA #46136

jsamudio commented Sep 26, 2024 •

edited

Loading

cmsbuild commented Sep 26, 2024 •

edited

Loading

cmsbuild commented Sep 26, 2024 •

edited

Loading

mmusich commented Sep 26, 2024

jfernan2 commented Sep 26, 2024

cmsbuild commented Sep 26, 2024

jfernan2 commented Sep 30, 2024

cmsbuild commented Sep 30, 2024

mandrenguyen commented Sep 30, 2024

cmsbuild commented Sep 30, 2024

mandrenguyen commented Oct 3, 2024

mmusich commented Oct 4, 2024

cmsbuild commented Oct 4, 2024

mmusich commented Oct 4, 2024

mmusich commented Oct 4, 2024

cmsbuild commented Oct 4, 2024

mmusich commented Oct 4, 2024

fwyzard commented Oct 4, 2024

mandrenguyen commented Oct 5, 2024

cmsbuild commented Oct 5, 2024

[14_1_X] Add runtime allocation of PF rechit fraction SoA #46136

[14_1_X] Add runtime allocation of PF rechit fraction SoA #46136

Conversation

jsamudio commented Sep 26, 2024 • edited Loading

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 Sep 26, 2024 • edited Loading

cmsbuild commented Sep 26, 2024 • edited Loading

mmusich commented Sep 26, 2024

jfernan2 commented Sep 26, 2024

cmsbuild commented Sep 26, 2024

Comparison Summary

jfernan2 commented Sep 30, 2024

cmsbuild commented Sep 30, 2024

mandrenguyen commented Sep 30, 2024

cmsbuild commented Sep 30, 2024

mandrenguyen commented Oct 3, 2024

mmusich commented Oct 4, 2024

cmsbuild commented Oct 4, 2024

mmusich commented Oct 4, 2024

mmusich commented Oct 4, 2024

cmsbuild commented Oct 4, 2024

Comparison Summary

GPU Comparison Summary

mmusich commented Oct 4, 2024

fwyzard commented Oct 4, 2024

mandrenguyen commented Oct 5, 2024

cmsbuild commented Oct 5, 2024

jsamudio commented Sep 26, 2024 •

edited

Loading

cmsbuild commented Sep 26, 2024 •

edited

Loading

cmsbuild commented Sep 26, 2024 •

edited

Loading