GPUvsCPU: updates to modules and workflows for pixel reconstruction #37617

AdrianoDee · 2022-04-19T10:17:06Z

PR description:

Updates for pixel only workflows:

rearranging .502 and .503 to avoid running the CPU chain when a GPU is available (due to the fact that the DQM consumes hit SoA and that is only available on CPU);
introducing a module to copy GPU SoA to CPU;
~~running both CPU and GPU chains in .503;~~
~~introducing gpuPixelValidation modifier;~~
~~dropping .501 that only generates confusion.~~

PR validation:

Running .502 and .503.

AdrianoDee · 2022-04-19T10:20:34Z

enable gpu

cmsbuild · 2022-04-19T10:23:20Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-37617/29386

This PR adds an extra 28KB to repository

cmsbuild · 2022-04-19T10:23:42Z

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

It involves the following packages:

CUDADataFormats/TrackingRecHit (heterogeneous, reconstruction)
DQM/SiPixelPhase1Heterogeneous (dqm)
RecoLocalTracker/SiPixelRecHits (reconstruction)
RecoPixelVertexing/Configuration (reconstruction)
RecoPixelVertexing/PixelTrackFitting (reconstruction)

@emanueleusai, @makortel, @fwyzard, @ahmad3213, @cmsbuild, @jfernan2, @clacaputo, @slava77, @jpata, @pmandrik, @micsucmed, @rvenditti can you please review it and eventually sign? Thanks.
@ferencek, @hdelanno, @felicepantaleo, @GiacomoSguazzoni, @JanFSchulte, @rovere, @VinInn, @gpetruc, @OzAmram, @fioriNTU, @jandrea, @mtosi, @idebruyn, @mmusich, @dkotlins, @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

fwyzard · 2022-04-19T10:38:53Z

CUDADataFormats/TrackingRecHit/interface/TrackingRecHit2DHeterogeneous.h

+  view->m_hitsModuleStart = m_hitsModuleStart;
+
+  //store transfer
+  if constexpr (std::is_same<Traits, cms::cudacompat::GPUTraits>::value) {


just a suggestion:

Suggested change

if constexpr (std::is_same<Traits, cms::cudacompat::GPUTraits>::value) {

if constexpr (std::is_same_v<Traits, cms::cudacompat::GPUTraits>) {

fwyzard · 2022-04-19T10:44:36Z

The c++ part looks good to me :-)
Now to understand the python part ...

fwyzard · 2022-04-19T10:44:48Z

enable gpu

fwyzard · 2022-04-19T10:44:52Z

please test

DQM/SiPixelPhase1Heterogeneous/python/SiPixelPhase1HeterogenousDQM_FirstStep_cff.py

AdrianoDee · 2022-04-19T12:19:38Z

@mmusich you are right, I hadn't pushed the modifications to Configuration package. Now it's there. For the possible conflicts we can choose how to proceed and which goes in first. No preference from my side.

@fwyzard may be helpful: find here the dependency graphs for .502 and .503 when a GPU is available (or not). The idea is that:

we have both siPixelRecHitsPreSplittingCUDA and siPixelRecHitsPreSplittingCPU in the task having only one of the two running (being consumed by the downstream reconstruction);
the siPixelRecHitsSoA is now a SwitchProducer(s) that either convert the CUDA to SoA if running on GPU (used only if requested by DQM) either is an alias for SiPixelRecHitsSoAFromLegacy

For the suggestion to move std::is_same with std::is_same_v I've supposed the suggestion was to move it everywhere.

Anyway, I'll squash all the commits at the end of the review.

cmsbuild · 2022-04-19T12:27:43Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-37617/29389

This PR adds an extra 44KB to repository
There are other open Pull requests which might conflict with changes you have proposed:
- File Configuration/ProcessModifiers/python/gpuValidationPixel_cff.py modified in PR(s): Pixel Heterogeneous DQM: add modules for Tracks and Vertices @cpu / @gpu comparisons #37619
- File Configuration/ProcessModifiers/python/gpuValidation_cff.py modified in PR(s): Pixel Heterogeneous DQM: add modules for Tracks and Vertices @cpu / @gpu comparisons #37619
- File Configuration/PyReleaseValidation/python/upgradeWorkflowComponents.py modified in PR(s): DeepSC algorithm for ECAL SuperClusters #37115, Dynamic Reduction Network for photon energy regression in miniAOD using the SonicTriton service #37134, Add HLT75e33 step to Phase-2 workflows #37324, Add run3 tracking low pu era #33532, Add wfs with HLT as separate step #37603
- File DQM/SiPixelPhase1Heterogeneous/python/SiPixelPhase1HeterogenousDQM_FirstStep_cff.py modified in PR(s): Pixel Heterogeneous DQM: add modules for Tracks and Vertices @cpu / @gpu comparisons #37619

jpata · 2022-05-03T11:25:30Z

+reconstruction

no reco changes (DQM-related reorganization)

jpata · 2022-05-03T11:25:30Z

+reconstruction

no reco changes (DQM-related reorganization)

fwyzard · 2022-05-03T13:38:48Z

Thanks @AdrianoDee for the dependency graphs.

Here is a comparison of the .502 workflow running only on CPU before this PR

and after:

The difference is that the siPixelRecHitsPreSplittingSoA module has been replaced by a SwitchProducer, and the module doing the reconstruction on CPU now is siPixelRecHitsPreSplittingCPU.

Here is the same comparison for the .502 workflow running on GPU, before

and after

The siPixelRecHitsPreSplittingSoA module hre is replaced by the @cuda branch of the SwitchProducer, which runs the SiPixelRecHitSoAFromCUDA module instead of the SiPixelRecHitSoAFromLegacy one.

fwyzard · 2022-05-03T13:38:57Z

+heterogeneous

cmsbuild · 2022-05-03T13:39:07Z

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)

fwyzard · 2022-05-03T13:39:40Z

@AdrianoDee with these changes, how does the .502 workflow running on CPU compare with the .501 workflow ?

perrotta · 2022-05-03T17:49:39Z

+1

AdrianoDee · 2022-05-03T18:19:52Z

@fwyzard, I checked the dependency graphs (in pdf here) and the tasks and there is a misalignment in the names in the local reco:

in .501 the siPixelRecHitsPresplitting holds the legacy and the SoA and it is a SwitchProducedCUDA with the @cpu branch only;
in .502 on GPU the siPixelRecHitsPresplitting holds only the legacy hits (either through and EDAlias for @cpu or with the SiPixelRecHitFromCUDA new producer) and the SoA is in siPixelRecHitsPreSplittingSoA (again either through and EDAlias for @cpu or with the SiPixelRecHitsSoAFromCUDA new producer), being both a SwitchProducedCUDA with both branches;

For the rest (and the final result) they run the same. Is this naming mismatch a possible issue? In case I'm trying to figure out a possible workaround given that, as far as I have understood, SwitchProducedCUDA can't hold a single branch with an EDAlias and a producer may not be replaced with with an EDAlias.

AdrianoDee · 2022-05-03T18:33:43Z

Also, do we need a backport of this? I missed the ORP today, sorry.

fwyzard · 2022-05-03T19:12:51Z

Also, do we need a backport of this? I missed the ORP today, sorry.

No, I don't think so.

fwyzard · 2022-05-03T19:14:38Z

SwitchProducedCUDA can't hold a single branch with an EDAlias

Actually, I don't know if that would be a problem -- is it something you tried and didn't work ?

qliphy · 2022-05-04T08:15:01Z

@AdrianoDee There is a IB issue after this PR gets merged, would you please have a check?
https://cmssdt.cern.ch/SDT/cgi-bin/logreader/slc7_amd64_gcc10/CMSSW_12_4_X_2022-05-03-2300/pyRelValMatrixLogs/run/39434.501_TTbar_14TeV+2026D88_Patatrack_PixelOnlyCPU+TTbar_14TeV_TuneCP5_GenSimHLBeamSpot14INPUT+DigiTrigger+RecoGlobal+HARVESTGlobal/step3_TTbar_14TeV+2026D88_Patatrack_PixelOnlyCPU+TTbar_14TeV_TuneCP5_GenSimHLBeamSpot14INPUT+DigiTrigger+RecoGlobal+HARVESTGlobal.log#/

cmsRun: /data/cmsbld/jenkins/workspace/build-any-ib/w/tmp/BUILDROOT/5525f5050df49f51ff7c29cd32fd5dea/opt/cmssw/slc7_amd64_gcc10/cms/cmssw/CMSSW_12_4_X_2022-05-03-2300/src/RecoLocalTracker/SiPixelRecHits/plugins/SiPixelRecHitSoAFromLegacy.cc:144: virtual void SiPixelRecHitSoAFromLegacy::produce(edm::StreamID, edm::Event&, const edm::EventSetup&) const: Assertion gind < nMaxModules' failed. cmsRun: /data/cmsbld/jenkins/workspace/build-any-ib/w/tmp/BUILDROOT/5525f5050df49f51ff7c29cd32fd5dea/opt/cmssw/slc7_amd64_gcc10/cms/cmssw/CMSSW_12_4_X_2022-05-03-2300/src/RecoLocalTracker/SiPixelRecHits/plugins/SiPixelRecHitSoAFromLegacy.cc:144: virtual void SiPixelRecHitSoAFromLegacy::produce(edm::StreamID, edm::Event&, const edm::EventSetup&) const: Assertion gind < nMaxModules' failed.

AdrianoDee · 2022-05-04T08:18:12Z

@AdrianoDee There is a IB issue after this PR gets merged, would you please have a check?

https://cmssdt.cern.ch/SDT/cgi-bin/logreader/slc7_amd64_gcc10/CMSSW_12_4_X_2022-05-03-2300/pyRelValMatrixLogs/run/39434.501_TTbar_14TeV+2026D88_Patatrack_PixelOnlyCPU+TTbar_14TeV_TuneCP5_GenSimHLBeamSpot14INPUT+DigiTrigger+RecoGlobal+HARVESTGlobal/step3_TTbar_14TeV+2026D88_Patatrack_PixelOnlyCPU+TTbar_14TeV_TuneCP5_GenSimHLBeamSpot14INPUT+DigiTrigger+RecoGlobal+HARVESTGlobal.log#/

cmsRun: /data/cmsbld/jenkins/workspace/build-any-ib/w/tmp/BUILDROOT/5525f5050df49f51ff7c29cd32fd5dea/opt/cmssw/slc7_amd64_gcc10/cms/cmssw/CMSSW_12_4_X_2022-05-03-2300/src/RecoLocalTracker/SiPixelRecHits/plugins/SiPixelRecHitSoAFromLegacy.cc:144: virtual void SiPixelRecHitSoAFromLegacy::produce(edm::StreamID, edm::Event&, const edm::EventSetup&) const: Assertion `gind < nMaxModules' failed.

cmsRun: /data/cmsbld/jenkins/workspace/build-any-ib/w/tmp/BUILDROOT/5525f5050df49f51ff7c29cd32fd5dea/opt/cmssw/slc7_amd64_gcc10/cms/cmssw/CMSSW_12_4_X_2022-05-03-2300/src/RecoLocalTracker/SiPixelRecHits/plugins/SiPixelRecHitSoAFromLegacy.cc:144: virtual void SiPixelRecHitSoAFromLegacy::produce(edm::StreamID, edm::Event&, const edm::EventSetup&) const: Assertion `gind < nMaxModules' failed.

@qliphy on it

AdrianoDee · 2022-05-04T10:58:30Z

@qliphy fixed in #37798

Actually, I don't know if that would be a problem -- is it something you tried and didn't work ?

@fwyzard I was doing some other stupid mistake. It actually works and I implemented in #37798

cmsbuild added this to the CMSSW_12_4_X milestone Apr 19, 2022

cmsbuild added code-checks-pending dqm-pending heterogeneous-pending orp-pending pending-signatures reconstruction-pending tests-pending labels Apr 19, 2022

cmsbuild added code-checks-approved and removed code-checks-pending labels Apr 19, 2022

fwyzard reviewed Apr 19, 2022

View reviewed changes

cmsbuild added tests-started and removed tests-pending labels Apr 19, 2022

cmsbuild mentioned this pull request Apr 19, 2022

Pixel Heterogeneous DQM: add modules for Tracks and Vertices @cpu / @gpu comparisons #37619

Merged

mmusich reviewed Apr 19, 2022

View reviewed changes

DQM/SiPixelPhase1Heterogeneous/python/SiPixelPhase1HeterogenousDQM_FirstStep_cff.py Outdated Show resolved Hide resolved

cmsbuild added code-checks-pending operations-pending pdmv-pending tests-pending upgrade-pending and removed tests-started code-checks-approved labels Apr 19, 2022

cmsbuild added reconstruction-approved and removed reconstruction-pending labels May 3, 2022

cmsbuild added fully-signed heterogeneous-approved and removed pending-signatures heterogeneous-pending labels May 3, 2022

cmsbuild added orp-approved and removed orp-pending labels May 3, 2022

cmsbuild merged commit 300b12d into cms-sw:master May 3, 2022

cmsbuild mentioned this pull request May 4, 2022

Update vecgeom to 1.1.20 cms-sw/cmsdist#7735

Merged

AdrianoDee mentioned this pull request May 4, 2022

Follow up to 37617 #37798

Merged

AdrianoDee deleted the gpu_dqm_pixels_124 branch May 4, 2022 10:56

cmsbuild mentioned this pull request May 4, 2022

fix use after scope in DMRChecker::bookResidualsHistogram #37796

Merged

qliphy mentioned this pull request May 5, 2022

WF 39434.501 step 3: Assertion failure in SiPixelRecHitSoAFromLegacy::produce #37810

Closed

This was referenced May 12, 2022

[12_3_X] Backport of 37617 + 37798 #37933

Merged

Pixel Heterogeneous DQM: add module rechit @cpu / @gpu comparisons #37860

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

GPUvsCPU: updates to modules and workflows for pixel reconstruction #37617

GPUvsCPU: updates to modules and workflows for pixel reconstruction #37617

AdrianoDee commented Apr 19, 2022 •

edited

Loading

AdrianoDee commented Apr 19, 2022

cmsbuild commented Apr 19, 2022

cmsbuild commented Apr 19, 2022

fwyzard Apr 19, 2022

fwyzard commented Apr 19, 2022

fwyzard commented Apr 19, 2022

fwyzard commented Apr 19, 2022

AdrianoDee commented Apr 19, 2022

cmsbuild commented Apr 19, 2022

jpata commented May 3, 2022

jpata commented May 3, 2022

fwyzard commented May 3, 2022

fwyzard commented May 3, 2022

cmsbuild commented May 3, 2022

fwyzard commented May 3, 2022

perrotta commented May 3, 2022

AdrianoDee commented May 3, 2022 •

edited

Loading

AdrianoDee commented May 3, 2022

fwyzard commented May 3, 2022

fwyzard commented May 3, 2022 •

edited

Loading

qliphy commented May 4, 2022

AdrianoDee commented May 4, 2022

AdrianoDee commented May 4, 2022

	if constexpr (std::is_same<Traits, cms::cudacompat::GPUTraits>::value) {
	if constexpr (std::is_same_v<Traits, cms::cudacompat::GPUTraits>) {

GPUvsCPU: updates to modules and workflows for pixel reconstruction #37617

GPUvsCPU: updates to modules and workflows for pixel reconstruction #37617

Conversation

AdrianoDee commented Apr 19, 2022 • edited Loading

PR description:

PR validation:

AdrianoDee commented Apr 19, 2022

cmsbuild commented Apr 19, 2022

cmsbuild commented Apr 19, 2022

fwyzard Apr 19, 2022

Choose a reason for hiding this comment

fwyzard commented Apr 19, 2022

fwyzard commented Apr 19, 2022

fwyzard commented Apr 19, 2022

AdrianoDee commented Apr 19, 2022

cmsbuild commented Apr 19, 2022

jpata commented May 3, 2022

jpata commented May 3, 2022

fwyzard commented May 3, 2022

fwyzard commented May 3, 2022

cmsbuild commented May 3, 2022

fwyzard commented May 3, 2022

perrotta commented May 3, 2022

AdrianoDee commented May 3, 2022 • edited Loading

AdrianoDee commented May 3, 2022

fwyzard commented May 3, 2022

fwyzard commented May 3, 2022 • edited Loading

qliphy commented May 4, 2022

AdrianoDee commented May 4, 2022

AdrianoDee commented May 4, 2022

AdrianoDee commented Apr 19, 2022 •

edited

Loading

AdrianoDee commented May 3, 2022 •

edited

Loading

fwyzard commented May 3, 2022 •

edited

Loading