reduce size of isOuterHitOfCell in patatrack #35285

VinInn · 2021-09-15T12:54:58Z

hits in BPIX1 can not be OuterHitOfCell. BPIX1 holds 1/ 4 of the hits.
This PR removes BPIX1 hits from isOuterHitOfCell.
Properly resize the array and the kernel launch boundaries.

Took the opportunity to reduce the number of device2host memcpy as well.

small throughput improvement observed.

technical: no regression expected. no regression observed.

VinInn · 2021-09-15T12:55:47Z

@cmsbuild , please test

VinInn · 2021-09-15T12:55:57Z

enable gpu

cmsbuild · 2021-09-15T13:02:12Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-35285/25287

This PR adds an extra 56KB to repository
There are other open Pull requests which might conflict with changes you have proposed:
- File RecoLocalTracker/SiPixelClusterizer/plugins/SiPixelRawToClusterGPUKernel.h modified in PR(s): Common digi packing for CPU and GPU implementations #34884

cmsbuild · 2021-09-15T13:02:40Z

A new Pull Request was created by @VinInn (Vincenzo Innocente) for master.

It involves the following packages:

CUDADataFormats/SiPixelCluster (heterogeneous, reconstruction)
CUDADataFormats/TrackingRecHit (heterogeneous, reconstruction)
RecoLocalTracker/SiPixelClusterizer (reconstruction)
RecoLocalTracker/SiPixelRecHits (reconstruction)
RecoPixelVertexing/PixelTriplets (reconstruction)

@makortel, @jpata, @fwyzard, @slava77 can you please review it and eventually sign? Thanks.
@mtosi, @felicepantaleo, @GiacomoSguazzoni, @JanFSchulte, @rovere, @VinInn, @OzAmram, @ferencek, @dkotlins, @gpetruc, @mmusich, @threus, @dgulhan, @tvami this is something you requested to watch as well.
@perrotta, @dpiparo, @qliphy you are the release manager for this.

cms-bot commands are listed here

RecoPixelVertexing/PixelTriplets/plugins/CAHitNtupletGeneratorKernels.cc

RecoPixelVertexing/PixelTriplets/plugins/CAConstants.h

cmsbuild · 2021-09-15T16:24:18Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-822c8d/18626/summary.html
COMMIT: bd9645a
CMSSW: CMSSW_12_1_X_2021-09-14-1100/slc7_amd64_gcc900
Additional Tests: GPU
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/35285/18626/install.sh to create a dev area with all the needed externals and cmssw changes.

GPU Comparison Summary

Summary:

No significant changes to the logs found
Reco comparison results: 0 differences found in the comparisons
DQMHistoTests: Total files compared: 4
DQMHistoTests: Total histograms compared: 19735
DQMHistoTests: Total failures: 6
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 19729
DQMHistoTests: Total skipped: 0
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 3 files compared)
Checked 12 log files, 9 edm output root files, 4 DQM output files
TriggerResults: no differences found

Comparison Summary

Summary:

No significant changes to the logs found
Reco comparison results: 2 differences found in the comparisons
DQMHistoTests: Total files compared: 39
DQMHistoTests: Total histograms compared: 3000833
DQMHistoTests: Total failures: 6
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3000805
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 38 files compared)
Checked 165 log files, 37 edm output root files, 39 DQM output files
TriggerResults: no differences found

VinInn · 2021-09-15T16:36:04Z

will wait for further comments before making a new commit to fix what found so far

VinInn · 2021-09-22T06:58:52Z

@cmsbuild , please test

cmsbuild · 2021-09-22T07:05:37Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-35285/25443

This PR adds an extra 64KB to repository

cmsbuild · 2021-09-22T07:05:57Z

Pull request #35285 was updated. @makortel, @jpata, @fwyzard, @slava77 can you please check and sign again.

cmsbuild · 2021-09-22T10:40:11Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-822c8d/18816/summary.html
COMMIT: 592ffaa
CMSSW: CMSSW_12_1_X_2021-09-21-2300/slc7_amd64_gcc900
Additional Tests: GPU
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/35285/18816/install.sh to create a dev area with all the needed externals and cmssw changes.

GPU Comparison Summary

Summary:

No significant changes to the logs found
Reco comparison results: 0 differences found in the comparisons
DQMHistoTests: Total files compared: 4
DQMHistoTests: Total histograms compared: 19735
DQMHistoTests: Total failures: 21
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 19714
DQMHistoTests: Total skipped: 0
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 3 files compared)
Checked 12 log files, 9 edm output root files, 4 DQM output files
TriggerResults: no differences found

Comparison Summary

Summary:

No significant changes to the logs found
Reco comparison results: 0 differences found in the comparisons
DQMHistoTests: Total files compared: 40
DQMHistoTests: Total histograms compared: 3211080
DQMHistoTests: Total failures: 0
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3211058
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 39 files compared)
Checked 169 log files, 37 edm output root files, 40 DQM output files
TriggerResults: no differences found

slava77 · 2021-09-22T14:18:56Z

+reconstruction

for #35285 592ffaa

code changes are technical, in line with the PR description and the follow up review
jenkins tests pass and comparisons with the baseline show no relevant differences
- there are small numerical differences in 10824.502 in Tracking/PixelTrack/pixel*, which I think are not related to this PR (there was no change in the previous set of tests and the last change was trivial). @VinInn I usually see changes only in the OfflinePixelPV/ folder, while the track parameters themselves usually do not change. Perhaps this is something to check.

slava77 · 2021-09-27T13:58:53Z

@cms-sw/heterogeneous-l2
please check this PR.

makortel · 2021-09-30T18:40:06Z

+heterogeneous

cmsbuild · 2021-09-30T18:43:42Z

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

perrotta · 2021-10-01T11:15:21Z

+1

fwyzard · 2021-10-05T12:48:02Z

For the record, here is a comparison of the throughput with respect to CMSSW_12_1_0_pre3, running over (uncompressed) TTbar events with pileup:

2 CPUs:
  0: Intel(R) Xeon(R) Gold 6130 CPU @ 2.10GHz (16 cores, 32 threads)
  1: Intel(R) Xeon(R) Gold 6130 CPU @ 2.10GHz (16 cores, 32 threads)

1 visible NVIDIA GPUs:
  0: Tesla T4

Running 4 times over 1000 events with 4 jobs, each with 16 threads, 16 streams and 1 GPUs

measurement	CMSSW_12_1_0_pre3	with #35285
I/O throughput	~ 2 kev/s	~ 2 kev/s
quadruplets	1071 ± 3 ev/s	1084 ± 3 ev/s
triplets	560 ± 2 ev/s	564 ± 1 ev/s

VinInn added 5 commits September 14, 2021 15:21

remove bpix1 from isOuterHitOfCell, crashes

32089f0

works

42b49d4

code format

25bcb7d

adjust lauch limits

1f87cef

reduce number of memcpy

bd9645a

cmsbuild added this to the CMSSW_12_1_X milestone Sep 15, 2021

cmsbuild added code-checks-pending heterogeneous-pending orp-pending pending-signatures reconstruction-pending tests-pending labels Sep 15, 2021

cmsbuild added tests-started and removed tests-pending labels Sep 15, 2021

cmsbuild added code-checks-approved and removed code-checks-pending labels Sep 15, 2021

fwyzard reviewed Sep 15, 2021

View reviewed changes

RecoPixelVertexing/PixelTriplets/plugins/CAHitNtupletGeneratorKernels.cc Outdated Show resolved Hide resolved

VinInn commented Sep 15, 2021

View reviewed changes

RecoPixelVertexing/PixelTriplets/plugins/CAConstants.h Outdated Show resolved Hide resolved

VinInn commented Sep 15, 2021

View reviewed changes

RecoPixelVertexing/PixelTriplets/plugins/CAConstants.h Outdated Show resolved Hide resolved

cmsbuild added tests-approved and removed tests-started labels Sep 15, 2021

remove assert and printout

7dc7977

cmsbuild removed tests-approved code-checks-approved labels Sep 17, 2021

cmsbuild added code-checks-pending tests-pending labels Sep 22, 2021

cmsbuild added tests-started and removed tests-pending labels Sep 22, 2021

cmsbuild added code-checks-approved and removed code-checks-pending labels Sep 22, 2021

cmsbuild added tests-approved and removed tests-started labels Sep 22, 2021

VinInn mentioned this pull request Sep 22, 2021

define (and use) single variable constexpr for "magic" numbers used in device code #35370

Closed

cmsbuild added reconstruction-approved and removed reconstruction-pending labels Sep 22, 2021

cmsbuild mentioned this pull request Sep 29, 2021

remove runtime recursion from find_ntuplets in Patatrack #35473

Merged

cmsbuild added fully-signed heterogeneous-approved and removed pending-signatures heterogeneous-pending labels Sep 30, 2021

cmsbuild added orp-approved and removed orp-pending labels Oct 1, 2021

cmsbuild merged commit 7e79ae6 into cms-sw:master Oct 1, 2021

fwyzard mentioned this pull request Oct 13, 2021

Do not call the fishbone for events with pixel hits only in BPIX1 #35638

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

reduce size of isOuterHitOfCell in patatrack #35285

reduce size of isOuterHitOfCell in patatrack #35285

VinInn commented Sep 15, 2021 •

edited

VinInn commented Sep 15, 2021

VinInn commented Sep 15, 2021

cmsbuild commented Sep 15, 2021

cmsbuild commented Sep 15, 2021

cmsbuild commented Sep 15, 2021

VinInn commented Sep 15, 2021

VinInn commented Sep 22, 2021

cmsbuild commented Sep 22, 2021

cmsbuild commented Sep 22, 2021

cmsbuild commented Sep 22, 2021

slava77 commented Sep 22, 2021

slava77 commented Sep 27, 2021

makortel commented Sep 30, 2021

cmsbuild commented Sep 30, 2021

perrotta commented Oct 1, 2021

fwyzard commented Oct 5, 2021 •

edited

reduce size of isOuterHitOfCell in patatrack #35285

reduce size of isOuterHitOfCell in patatrack #35285

Conversation

VinInn commented Sep 15, 2021 • edited

VinInn commented Sep 15, 2021

VinInn commented Sep 15, 2021

cmsbuild commented Sep 15, 2021

cmsbuild commented Sep 15, 2021

cmsbuild commented Sep 15, 2021

GPU Comparison Summary

Comparison Summary

VinInn commented Sep 15, 2021

VinInn commented Sep 22, 2021

cmsbuild commented Sep 22, 2021

cmsbuild commented Sep 22, 2021

cmsbuild commented Sep 22, 2021

GPU Comparison Summary

Comparison Summary

slava77 commented Sep 22, 2021

slava77 commented Sep 27, 2021

makortel commented Sep 30, 2021

cmsbuild commented Sep 30, 2021

perrotta commented Oct 1, 2021

fwyzard commented Oct 5, 2021 • edited

VinInn commented Sep 15, 2021 •

edited

fwyzard commented Oct 5, 2021 •

edited