TrackingRecHitSoA Classes Clean Up #40575

AdrianoDee · 2023-01-20T09:54:44Z

PR description:

Prompted by #40465 (comment) fixes the issue. Took the chance to do a minor cleanup of the TrackingRecHitSoA* classes.

Technical PR. No change expected.

cmsbuild · 2023-01-20T10:01:30Z

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-40575/33820

This PR adds an extra 28KB to repository
There are other open Pull requests which might conflict with changes you have proposed:
- File RecoLocalTracker/SiPixelRecHits/plugins/PixelRecHitGPUKernel.cu modified in PR(s): New Heterogeneous Memory Pool #37952, Use cooperative groups to populate Associations (Histograms) in Pixel Patatrack #35713
- File RecoPixelVertexing/PixelTriplets/plugins/CAHitNtupletGeneratorKernels.cc modified in PR(s): New Heterogeneous Memory Pool #37952, Use cooperative groups to populate Associations (Histograms) in Pixel Patatrack #35713
- File RecoPixelVertexing/PixelTriplets/plugins/CAHitNtupletGeneratorKernels.cu modified in PR(s): New Heterogeneous Memory Pool #37952, Use cooperative groups to populate Associations (Histograms) in Pixel Patatrack #35713
- File RecoPixelVertexing/PixelTriplets/plugins/CAHitNtupletGeneratorKernels.h modified in PR(s): New Heterogeneous Memory Pool #37952, Use cooperative groups to populate Associations (Histograms) in Pixel Patatrack #35713
- File RecoPixelVertexing/PixelTriplets/plugins/CAHitNtupletGeneratorOnGPU.cc modified in PR(s): New Heterogeneous Memory Pool #37952, Simplify pixel typedefs #40565
- File RecoPixelVertexing/PixelTriplets/plugins/CAHitNtupletGeneratorOnGPU.h modified in PR(s): Simplify pixel typedefs #40565

Code check has found code style and quality issues which could be resolved by applying following patch(s)

code-format:
https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-40575/33820/code-format.patch
e.g. curl -k https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-40575/33820/code-format.patch | patch -p1
You can also run scram build code-format to apply code format directly

cmsbuild · 2023-01-20T10:15:46Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-40575/33821

This PR adds an extra 28KB to repository
There are other open Pull requests which might conflict with changes you have proposed:
- File RecoLocalTracker/SiPixelRecHits/plugins/PixelRecHitGPUKernel.cu modified in PR(s): New Heterogeneous Memory Pool #37952, Use cooperative groups to populate Associations (Histograms) in Pixel Patatrack #35713
- File RecoPixelVertexing/PixelTriplets/plugins/CAHitNtupletGeneratorKernels.cc modified in PR(s): New Heterogeneous Memory Pool #37952, Use cooperative groups to populate Associations (Histograms) in Pixel Patatrack #35713
- File RecoPixelVertexing/PixelTriplets/plugins/CAHitNtupletGeneratorKernels.cu modified in PR(s): New Heterogeneous Memory Pool #37952, Use cooperative groups to populate Associations (Histograms) in Pixel Patatrack #35713
- File RecoPixelVertexing/PixelTriplets/plugins/CAHitNtupletGeneratorKernels.h modified in PR(s): New Heterogeneous Memory Pool #37952, Use cooperative groups to populate Associations (Histograms) in Pixel Patatrack #35713
- File RecoPixelVertexing/PixelTriplets/plugins/CAHitNtupletGeneratorOnGPU.cc modified in PR(s): New Heterogeneous Memory Pool #37952, Simplify pixel typedefs #40565
- File RecoPixelVertexing/PixelTriplets/plugins/CAHitNtupletGeneratorOnGPU.h modified in PR(s): Simplify pixel typedefs #40565

cmsbuild · 2023-01-20T10:16:09Z

A new Pull Request was created by @AdrianoDee (Adriano Di Florio) for master.

It involves the following packages:

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

@cmsbuild, @makortel, @mandrenguyen, @clacaputo, @fwyzard can you please review it and eventually sign? Thanks.
@mtosi, @VourMa, @felicepantaleo, @GiacomoSguazzoni, @JanFSchulte, @rovere, @VinInn, @mroguljic, @missirol, @dkotlins, @ferencek, @gpetruc, @mmusich, @threus, @dgulhan, @tvami this is something you requested to watch as well.
@perrotta, @dpiparo, @rappoccio you are the release manager for this.

cms-bot commands are listed here

AdrianoDee · 2023-01-20T10:19:27Z

test parameters:

enable_tests = gpu
workflows_gpu = 21034.507, 20834.507
relvals_opt= -w upgrade
relvals_opt_gpu = -w upgrade

AdrianoDee · 2023-01-20T10:25:05Z

please test

fwyzard · 2023-01-20T10:44:18Z

@AdrianoDee I have mixed feelings about these changes:

the removal of nHits_ replaced by view().metadata().size() makes sense
the removal of phiBinnerStorage_ and cpeParams_ that were unused makes sense
I would suggest to keep offsetBPIX2_ also in the Host class, because I think we should have it there for the Alpaka migration

AdrianoDee · 2023-01-20T11:37:27Z

I would suggest to keep offsetBPIX2_ also in the Host class, because I think we should have it there for the Alpaka migration

I see your point here, let me restore it.

AdrianoDee · 2023-01-20T13:04:45Z

please abort

smuzaffar · 2023-01-23T14:06:17Z

It seems it's not working as expected or I'm misspelling something.

ah as we have multiple archs for LTO builds that is why bot was confused which one to use. I will update the configuration to use amd64 arch as default . For now I have triggered the tests by explicitly telling bot to use el8_amd64_gcc11

AdrianoDee · 2023-01-23T14:07:32Z

It seems it's not working as expected or I'm misspelling something.

ah as we have multiple archs for LTO builds that is why bot was confused which one to use. I will update the configuration to use amd64 arch as default . For now I have triggered the tests by explicitly telling bot to use el8_amd64_gcc11

I see, thanks @smuzaffar.

cmsbuild · 2023-01-23T23:23:04Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-79f142/30124/summary.html
COMMIT: 2b5ba63
CMSSW: CMSSW_13_0_LTO_X_2023-01-23-1100/el8_amd64_gcc11
Additional Tests: GPU
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/40575/30124/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: 65047 differences found in the comparisons
DQMHistoTests: Total files compared: 49
DQMHistoTests: Total histograms compared: 3555495
DQMHistoTests: Total failures: 150336
DQMHistoTests: Total nulls: 248
DQMHistoTests: Total successes: 3404889
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.4149999999999999 KiB( 48 files compared)
DQMHistoSizes: changed ( 10224.0 ): 0.715 KiB SiStrip/MechanicalView
DQMHistoSizes: changed ( 11834.0 ): -1.439 KiB SiStrip/MechanicalView
DQMHistoSizes: changed ( 250202.181 ): 0.885 KiB SiStrip/MechanicalView
DQMHistoSizes: changed ( 25202.0 ): 0.254 KiB SiStrip/MechanicalView
Checked 211 log files, 162 edm output root files, 49 DQM output files
TriggerResults: found differences in 16 / 47 workflows

clacaputo · 2023-02-01T14:54:40Z

Reco comparison results: 65047 differences found in the comparisons, but just because the baseline used for the comparison hasn't been built with LTO. @smuzaffar , is there a way to make the bot compare the results with an LTO IB?

smuzaffar · 2023-02-01T15:09:51Z

Reco comparison results: 65047 differences found in the comparisons, but just because the baseline used for the comparison hasn't been built with LTO. @smuzaffar , is there a way to make the bot compare the results with an LTO IB?

there is nothing which works out of the box but I can force re-build the comparison job to use LTO baseline. I will start it now

smuzaffar · 2023-02-01T15:15:40Z

please test for CMSSW_13_0_LTO_X/el8_amd64_gcc11

lets rerun with latest LTO IB

smuzaffar · 2023-02-01T16:29:33Z

please test for CMSSW_13_0_X/el8_amd64_gcc11

lets test it for default/production IB too

cmsbuild · 2023-02-01T17:57:18Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-79f142/30333/summary.html
COMMIT: 2b5ba63
CMSSW: CMSSW_13_0_LTO_X_2023-01-31-2300/el8_amd64_gcc11
Additional Tests: GPU
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/40575/30333/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: 6 differences found in the comparisons
DQMHistoTests: Total files compared: 49
DQMHistoTests: Total histograms compared: 3555495
DQMHistoTests: Total failures: 37
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3555436
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 48 files compared)
Checked 211 log files, 162 edm output root files, 49 DQM output files
TriggerResults: no differences found

cmsbuild · 2023-02-01T19:10:12Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-79f142/30338/summary.html
COMMIT: 2b5ba63
CMSSW: CMSSW_13_0_X_2023-01-31-2300/el8_amd64_gcc11
Additional Tests: GPU
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/40575/30338/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

You potentially added 1 lines to the logs
Reco comparison results: 5 differences found in the comparisons
DQMHistoTests: Total files compared: 49
DQMHistoTests: Total histograms compared: 3555495
DQMHistoTests: Total failures: 6
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3555467
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 48 files compared)
Checked 211 log files, 162 edm output root files, 49 DQM output files
TriggerResults: no differences found

GPU Comparison Summary

Summary:

You potentially removed 135 lines from the logs
Reco comparison results: 0 differences found in the comparisons
DQMHistoTests: Total files compared: 4
DQMHistoTests: Total histograms compared: 19862
DQMHistoTests: Total failures: 78
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 19784
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: found differences in 2 / 3 workflows

clacaputo · 2023-02-03T08:37:44Z

+reconstruction

makortel · 2023-02-03T17:43:34Z

+heterogeneous

cmsbuild · 2023-02-03T17:44:03Z

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

perrotta · 2023-02-04T06:40:00Z

+1

cmsbuild added this to the CMSSW_13_0_X milestone Jan 20, 2023

cmsbuild added code-checks-pending heterogeneous-pending orp-pending pending-signatures reconstruction-pending tests-pending tracking trk labels Jan 20, 2023

AdrianoDee mentioned this pull request Jan 20, 2023

Portable Data Formats for Pixel Track Reconstruction #40465

Merged

cmsbuild added code-checks-rejected code-checks-pending and removed code-checks-pending code-checks-rejected labels Jan 20, 2023

cmsbuild added code-checks-approved and removed code-checks-pending labels Jan 20, 2023

cmsbuild added tests-started and removed tests-pending labels Jan 20, 2023

cmsbuild added tests-pending and removed tests-started labels Jan 20, 2023

AdrianoDee force-pushed the portable_hits_uninit_fix branch from f490f8f to df3c5f8 Compare January 20, 2023 13:16

cmsbuild removed the code-checks-approved label Jan 20, 2023

cmsbuild added tests-started and removed tests-pending labels Jan 23, 2023

cmsbuild added tests-approved and removed tests-started labels Jan 23, 2023

cmsbuild added tests-started and removed tests-approved labels Feb 1, 2023

cmsbuild added tests-approved and removed tests-started labels Feb 1, 2023

cmsbuild added reconstruction-approved and removed reconstruction-pending labels Feb 3, 2023

cmsbuild added fully-signed heterogeneous-approved and removed pending-signatures heterogeneous-pending labels Feb 3, 2023

cmsbuild added orp-approved and removed orp-pending labels Feb 4, 2023

cmsbuild merged commit 3e10dc1 into cms-sw:master Feb 4, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

TrackingRecHitSoA Classes Clean Up #40575

TrackingRecHitSoA Classes Clean Up #40575

AdrianoDee commented Jan 20, 2023

cmsbuild commented Jan 20, 2023

cmsbuild commented Jan 20, 2023

cmsbuild commented Jan 20, 2023

AdrianoDee commented Jan 20, 2023

AdrianoDee commented Jan 20, 2023

fwyzard commented Jan 20, 2023

AdrianoDee commented Jan 20, 2023

AdrianoDee commented Jan 20, 2023

smuzaffar commented Jan 23, 2023

AdrianoDee commented Jan 23, 2023

cmsbuild commented Jan 23, 2023

clacaputo commented Feb 1, 2023

smuzaffar commented Feb 1, 2023

smuzaffar commented Feb 1, 2023

smuzaffar commented Feb 1, 2023 •

edited

cmsbuild commented Feb 1, 2023

cmsbuild commented Feb 1, 2023

clacaputo commented Feb 3, 2023

makortel commented Feb 3, 2023

cmsbuild commented Feb 3, 2023

perrotta commented Feb 4, 2023

TrackingRecHitSoA Classes Clean Up #40575

TrackingRecHitSoA Classes Clean Up #40575

Conversation

AdrianoDee commented Jan 20, 2023

PR description:

cmsbuild commented Jan 20, 2023

cmsbuild commented Jan 20, 2023

cmsbuild commented Jan 20, 2023

AdrianoDee commented Jan 20, 2023

AdrianoDee commented Jan 20, 2023

fwyzard commented Jan 20, 2023

AdrianoDee commented Jan 20, 2023

AdrianoDee commented Jan 20, 2023

smuzaffar commented Jan 23, 2023

AdrianoDee commented Jan 23, 2023

cmsbuild commented Jan 23, 2023

Comparison Summary

clacaputo commented Feb 1, 2023

smuzaffar commented Feb 1, 2023

smuzaffar commented Feb 1, 2023

smuzaffar commented Feb 1, 2023 • edited

cmsbuild commented Feb 1, 2023

Comparison Summary

cmsbuild commented Feb 1, 2023

Comparison Summary

GPU Comparison Summary

clacaputo commented Feb 3, 2023

makortel commented Feb 3, 2023

cmsbuild commented Feb 3, 2023

perrotta commented Feb 4, 2023

smuzaffar commented Feb 1, 2023 •

edited