Portable Data Formats for Pixel Track Reconstruction #40465
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
nothingface0
changed the title
|
@cms-sw/reconstruction-l2 this is something that we would like to have in pre3, in order to do a full GPU-vs-GPU and GPU-vs-CPU validation (PPD is aware3) I do acknowledge that it's a large PR arriving at the last minute... and it likely still needs some clean up. |
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-40465/33636
|
|
A new Pull Request was created by @nothingface0 (Dimitris Papagiannis) for master. It involves the following packages:
@Martin-Grunewald, @civanch, @Dr15Jones, @clacaputo, @bsunanda, @makortel, @micsucmed, @emanueleusai, @ianna, @ahmad3213, @cmsbuild, @missirol, @mdhildreth, @syuvivida, @fwyzard, @pmandrik, @mandrenguyen, @rvenditti can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
|
Thanks for the heads up @fwyzard |
mmusich
reviewed
Jan 10, 2023
DQM/SiPixelPhase1Heterogeneous/plugins/SiPixelPhase1CompareRecHitsSoA.cc
Outdated
Show resolved
Hide resolved
No reason. Apparently we missed the commit with the cleanup. Recovering it! |
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-40465/33639
|
|
Pull request #40465 was updated. @Martin-Grunewald, @civanch, @Dr15Jones, @clacaputo, @bsunanda, @makortel, @micsucmed, @emanueleusai, @ianna, @ahmad3213, @cmsbuild, @missirol, @mdhildreth, @syuvivida, @fwyzard, @pmandrik, @mandrenguyen, @rvenditti can you please check and sign again. |
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-40465/33640
|
|
+1 |
cmsbuild
added
fully-signed
geometry-approved
and removed
geometry-pending
pending-signatures
labels
|
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) |
|
+1
|
|
This PR caused many |
|
The issue seems to be that these constructors // This SoA Host is used basically only for DQM
// so we just need a slim constructor
explicit TrackingRecHitSoAHost(uint32_t nHits)
: cms::cuda::PortableHostCollection<TrackingRecHitLayout<TrackerTraits>>(nHits) {}
explicit TrackingRecHitSoAHost(uint32_t nHits, cudaStream_t stream)
: cms::cuda::PortableHostCollection<TrackingRecHitLayout<TrackerTraits>>(nHits, stream) {}do not set the data members: uint32_t nHits_;
ParamsOnGPU const* cpeParams_;
uint32_t offsetBPIX2_;
PhiBinnerStorageType* phiBinnerStorage_;and the compiler has no way of knowing that those values will not be used later on. So, they should either be removed (if they are really never used) or be properly initialised. |
|
Thank you @makortel and @fwyzard |
|
@perrotta taking care of it as soon as possible (anyway today at latest). |
|
Opened #40575 should address this. I'm saying should because I don't know if there's something to do to turn on the printouts for those warnings. Is it there? |
mmusich
reviewed
May 18, 2023
| hCurvdiffMatched_->Fill((helper::charge(tsoaCPU.view(), it) / tsoaCPU.view()[it].pt()) - | ||
| (helper::charge(tsoaGPU.view(), closestTkidx) / tsoaGPU.view()[closestTkidx].pt())); | ||
| hetadiffMatched_->Fill(etaCPU - tsoaGPU.view()[closestTkidx].eta()); | ||
| hphidiffMatched_->Fill(reco::deltaPhi(etaCPU, helper::phi(tsoaGPU.view(), closestTkidx))); |
There was a problem hiding this comment.
I guess you meant:
hphidiffMatched_->Fill(reco::deltaPhi(phiCPU, helper::phi(tsoaGPU.view(), closestTkidx)));
mmusich
reviewed
May 18, 2023
| hpt_->Fill(tsoaCPU.view()[it].pt(), tsoaGPU.view()[closestTkidx].pt()); | ||
| hptLogLog_->Fill(tsoaCPU.view()[it].pt(), tsoaGPU.view()[closestTkidx].pt()); | ||
| heta_->Fill(etaCPU, tsoaGPU.view()[closestTkidx].eta()); | ||
| hphi_->Fill(etaCPU, helper::phi(tsoaGPU.view(), closestTkidx)); |
There was a problem hiding this comment.
I guess you meant:
hphi_->Fill(phiCPU, helper::phi(tsoaGPU.view(), closestTkidx));
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Co-authored-by: Adriano DiFlorio [email protected]
Co-authored-by: Breno Orzari [email protected]
Co-authored-by: Dimitris Papagiannis [email protected]
PR description:
A joint work from @borzari, @nothingface0 and @AdrianoDee.
This PR proposes moving the data structures used for the pixel local and track reconstruction to the
Portable{Host,Device}Collectionlogic. This includes moving all the methods used in the chain to make use. The modifications are all technical and no change in performance observed nor expected. This is the first of two steps towards the pixel track reconstruction CUDA to Alpaka port.Physics Performance
Local SiPixel Reco
No difference shown. Posting
FDisk -2hit charge as an example.Further plots here.
Pixel Track Reco
No difference shown. Posting pixel track efficiencies and vertices RecoVsSim as examples. The four perfectly overlapping trends compare the GPU vs CPU and PR (
PixelPortableDataFormats) vsCMSSW_13_0_X_2023-01-09-2300.Further MTV plots here.
Computational Performance
/EphemeralHLTPhysics1/Run2018D-v1/RAW run=323775 lumi=53Evaluated on
fu-c2a02-37-02T4@P5 via patatrack-scripts.Physics Validation Plots
Run3
RelVal
ttbarsamples withPUand125X_mcRun3_2022_realistic_v3GT:triplets
quadruplets
Phase2
RelVal
ttbarsamples withPUand125X_mcRun4_realistic_v5GT:triplets
quadruplets
Further references