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

Add SoA macros to CUDADataFormats/Common. #35951

Closed
wants to merge 12 commits into from

Conversation

ericcano
Copy link
Contributor

@ericcano ericcano commented Nov 2, 2021

PR description:

The SoA macros generate classes that subdivide externally allocated buffer in
columns with alignement adequate for GPUs (by default).
An test validates the SoA and provides usage example.

PR validation:

This PR provide new tools and does not interfere yet with current production code. The tools itself is validated with a test program.

@cmsbuild
Copy link
Contributor

cmsbuild commented Nov 2, 2021

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-35951/26358

  • This PR adds an extra 28KB to repository

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

@cmsbuild
Copy link
Contributor

cmsbuild commented Nov 2, 2021

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-35951/26359

  • This PR adds an extra 36KB to repository

@cmsbuild
Copy link
Contributor

cmsbuild commented Nov 2, 2021

A new Pull Request was created by @ericcano (Eric Cano) for master.

It involves the following packages:

  • DataFormats/Common (core)

@cmsbuild, @smuzaffar, @Dr15Jones, @makortel can you please review it and eventually sign? Thanks.
@makortel, @wddgit, @rovere 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

@makortel
Copy link
Contributor

makortel commented Nov 3, 2021

Thanks @ericcano. DataFormats/Common is not a good place for this code though, because it would add a dependence on Eigen, and, if I understood correctly, currently these SoAs can not be stored in ROOT.

Given the dependence on Eigen, even a new package could be the justified. I presume the intention would be to use the SoA's also in pure host code (without any dependence on CUDA), the best placement would likely still be under DataFormats (even if for now the Event products using the SoA's would be transient). There is a strong connection to GPUs, so perhaps the package would be best placed under heterogeneous category for review and maintenance.

@cms-sw/heterogeneous-l2

@cmsbuild
Copy link
Contributor

cmsbuild commented Nov 3, 2021

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-35951/26390

  • This PR adds an extra 24KB to repository

@cmsbuild
Copy link
Contributor

cmsbuild commented Nov 3, 2021

Pull request #35951 was updated. @cmsbuild, @smuzaffar, @Dr15Jones, @makortel can you please check and sign again.

@makortel
Copy link
Contributor

makortel commented Nov 5, 2021

@ericcano Would you be able to present this model of building SoAs in the core software meeting on next Tuesday (Nov 9) or on the following week (Nov 16)?

@ericcano
Copy link
Contributor Author

ericcano commented Nov 5, 2021

@makortel Yes, next Tuesday is fine.
Regarding the location, I will move all to CUDADataFormats/Common

@cmsbuild
Copy link
Contributor

cmsbuild commented Nov 5, 2021

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-35951/26446

@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 6, 2022

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-35951/29179

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

@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 6, 2022

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-35951/29185

@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 6, 2022

Pull request #35951 was updated. @cmsbuild, @makortel, @fwyzard can you please check and sign again.

@smuzaffar smuzaffar modified the milestones: CMSSW_12_4_X, CMSSW_12_5_X May 16, 2022
@smuzaffar smuzaffar modified the milestones: CMSSW_12_5_X, CMSSW_12_6_X Aug 28, 2022
@fwyzard
Copy link
Contributor

fwyzard commented Sep 15, 2022

I think this PR has been superseded by all the development work merged in 12.5.x and 12.6.x, up to #39319 .

@ericcano, do you agree this one could be closed ?

@ericcano
Copy link
Contributor Author

@ericcano, do you agree this one could be closed ?

Yes

@ericcano ericcano closed this Sep 15, 2022
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.

6 participants