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

Pixel Alpaka Migration: AlpakaInterface Updates [II] #41284

Closed

Conversation

AdrianoDee
Copy link
Contributor

@AdrianoDee AdrianoDee commented Apr 5, 2023

PR description:

This PR stems from #41117 and it's the 2nd of a series of smaller PRs.

This is It includes all the needed additions to HeterogenousCore. It creates a new sub-package HeterogeneousCore/AlpakaUtilities that includes all the needed utilities (such as histograms and vectors) for Alpaka mimicking what's done in HeterogenousCore/CUDAUtilities. It also adds some helpers in HeterogeneousCore/AlpakaInterface/interface/workdivision.h for range computation and looping.

This includes #40932 and supersedes it with the latest comments received addressed.

Using PixelTracksAlpaka#15 as a log report to keep track of all the comments.

This PR and #41282, once reviewed, may be merged as they are. They do not depend on the other PRs.

PR validation:

It compiles. Running base tests. No regression expected.

@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 5, 2023

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-41284/35059

@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 5, 2023

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

It involves the following packages:

  • HeterogeneousCore/AlpakaInterface (heterogeneous)
  • HeterogeneousCore/AlpakaUtilities (****)

The following packages do not have a category, yet:

HeterogeneousCore/AlpakaUtilities
Please create a PR for https://github.com/cms-sw/cms-bot/blob/master/categories_map.py to assign category

@cmsbuild, @makortel, @fwyzard can you please review it and eventually sign? Thanks.
@makortel, @missirol, @rovere 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
Copy link
Contributor Author

please test

Copy link
Contributor

@makortel makortel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about the unit tests (either ported from CUDAUtilities or from the pixeltrack-standalone)?

HeterogeneousCore/AlpakaInterface/interface/workdivision.h Outdated Show resolved Hide resolved
HeterogeneousCore/AlpakaInterface/interface/workdivision.h Outdated Show resolved Hide resolved
HeterogeneousCore/AlpakaInterface/test/alpaka/testVec.cc Outdated Show resolved Hide resolved
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the hackathon with @fwyzard and @rovere we concluded that the kind of classes in AlpakaUtilities here would be ok to be plated in AlpakaInterface (to avoid adding new package), for now at least.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved everything under AlpakaInterface.

radixSortImpl<TAcc, I, NS>(acc, (I const*)(a), ind, ind2, size, reorderFloat<TAcc, I>);
}

/* Not needed
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can it be removed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

HeterogeneousCore/AlpakaUtilities/interface/demangle.h Outdated Show resolved Hide resolved
HeterogeneousCore/AlpakaUtilities/interface/initialise.h Outdated Show resolved Hide resolved
@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 5, 2023

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-96c764/31848/summary.html
COMMIT: 0a63573
CMSSW: CMSSW_13_1_X_2023-04-05-1100/el8_amd64_gcc11
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/41284/31848/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

  • You potentially added 7 lines to the logs
  • Reco comparison results: 7 differences found in the comparisons
  • DQMHistoTests: Total files compared: 47
  • DQMHistoTests: Total histograms compared: 3365163
  • DQMHistoTests: Total failures: 3
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3365138
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 46 files compared)
  • Checked 202 log files, 155 edm output root files, 47 DQM output files
  • TriggerResults: no differences found

@cmsbuild
Copy link
Contributor

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-41284/35136

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

@slava77
Copy link
Contributor

slava77 commented Apr 28, 2023

@AdrianoDee @cms-sw/heterogeneous-l2
can this be integrated by itself or is this waiting for review of other "pixel alpaka migration" parts >=3 or yet something else?

@smuzaffar smuzaffar modified the milestones: CMSSW_13_1_X, CMSSW_13_2_X May 4, 2023
@slava77
Copy link
Contributor

slava77 commented May 4, 2023

@AdrianoDee @cms-sw/heterogeneous-l2 can this be integrated by itself or is this waiting for review of other "pixel alpaka migration" parts >=3 or yet something else?

now that we moved to 13_2_X, can the integration proceed or is it blocked by something?

@fwyzard
Copy link
Contributor

fwyzard commented May 5, 2023

PixelTracksAlpaka#16 should be integrated in the branch of the PR first.

I would like to provide more updates, but I will need to have a full working recipe to make sure I won't break any of the new workflows 🤷🏻‍♂️

@AdrianoDee
Copy link
Contributor Author

PixelTracksAlpaka#16 should be integrated in the branch of the PR first.

I would like to provide more updates, but I will need to have a full working recipe to make sure I won't break any of the new workflows 🤷🏻‍♂️

To be honest I had not noticed that (apparently I don't get notifications for PR being opened!). We are working on getting a working config, will come back as soon as possible.

@smuzaffar
Copy link
Contributor

Milestone for this pull request has been moved to CMSSW_14_0_X.Please open a backport if it should also go in to CMSSW_13_3_X.

@AdrianoDee
Copy link
Contributor Author

Closing since superseded by #43064

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