Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Split ZVertexSoA in a run-time sized PortableMultiCollection #45887

Merged
merged 5 commits into from
Sep 13, 2024

Conversation

fwyzard
Copy link
Contributor

@fwyzard fwyzard commented Sep 5, 2024

PR description:

Split ZVertexSoA in two layouts, and wrap them in a multi-layout collection.
Make the ZVertex SoA collections run-time sized, and replace the ZVertexHost/ZVertexDevice template classes inheriting from PortableHostMultiCollection/PortableDeviceMultiCollection with type aliases.

Remove versioning info from class description for dictionaries.
Port comments from the CUDA version.

The maximum number of vertices is now a configuration parameter of the PixelVertexProducerAlpaka EDProducer, while all device-side checks now make use of the various SoA capacity.
The maximum number of vertices is set to

  • 256 for Run 3
  • 512 for Phase 2
  • 32 for heavy ions
    These values are probably conservative, and could be further reduced.
    The tests use the legacy hard-coded values, taken from the old Run 3 defaults.

The documentation in DataFormats/VertexSoA/README.md is updated accordingly.

PR validation:

Unit tests and workflow 12834.403 pass.

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 5, 2024

cms-bot internal usage

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 5, 2024

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-45887/41639

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 5, 2024

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

It involves the following packages:

  • DQM/SiPixelHeterogeneous (dqm)
  • DataFormats/VertexSoA (heterogeneous, reconstruction)
  • RecoTauTag/HLTProducers (hlt)
  • RecoTracker/PixelTrackFitting (reconstruction)
  • RecoTracker/PixelVertexFinding (reconstruction)

@Martin-Grunewald, @antoniovagnerini, @cmsbuild, @fwyzard, @jfernan2, @makortel, @mandrenguyen, @mmusich, @nothingface0, @rvenditti, @syuvivida, @tjavaid can you please review it and eventually sign? Thanks.
@GiacomoSguazzoni, @JanFSchulte, @VinInn, @VourMa, @azotz, @dgulhan, @felicepantaleo, @fioriNTU, @gpetruc, @idebruyn, @jandrea, @mbluj, @missirol, @mmusich, @mtosi, @rovere, @silviodonato, @threus 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

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 5, 2024

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-45887/41642

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 5, 2024

Pull request #45887 was updated. @Martin-Grunewald, @antoniovagnerini, @cmsbuild, @fwyzard, @jfernan2, @makortel, @mandrenguyen, @mmusich, @nothingface0, @rvenditti, @syuvivida, @tjavaid can you please check and sign again.

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 5, 2024

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-45887/41685

@fwyzard
Copy link
Contributor Author

fwyzard commented Sep 11, 2024

+heterogeneous

@jfernan2
Copy link
Contributor

+1

@mmusich
Copy link
Contributor

mmusich commented Sep 12, 2024

+hlt

  • (direct) changes in HLT domain are minimal (RecoTauTag/HLTProducers/src/L2TauTagNNProducerAlpaka.cc)

@fwyzard
Copy link
Contributor Author

fwyzard commented Sep 12, 2024

kind ping to @cms-sw/dqm-l2 🙏🏻

@antoniovagnerini
Copy link

+1

@cmsbuild
Copy link
Contributor

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

@mandrenguyen
Copy link
Contributor

+1

@cmsbuild cmsbuild merged commit 5b0530e into cms-sw:master Sep 13, 2024
15 checks passed
@fwyzard fwyzard deleted the ZVertexMultiLayout branch September 13, 2024 05:29
@mandrenguyen
Copy link
Contributor

cross posting the following comment:
#46686 (comment)

Is there a failure related to this PR?

@makortel
Copy link
Contributor

Is there a failure related to this PR?

Nothing has been reported since the PR was merged, and the IB GPU tests are clean (for the history that is visible). On the other hand, this doesn't prove much because the workflow that failed in #46686 (comment), 14949.402, is not tested in IBs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants