Refactor of PortableCollections #49860
Merged
+525
−360
Conversation
This file contains hidden or 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
Contributor
|
cms-bot internal usage |
Contributor
Author
|
type ngt |
Contributor
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-49860/47547
|
Contributor
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-49860/47577
|
Contributor
|
Pull request #49860 was updated. @Dr15Jones, @bsunanda, @civanch, @cmsbuild, @fwyzard, @hjkwon260, @kpedro88, @makortel, @mdhildreth, @valsdav, @y19y19 can you please check and sign again. |
Contributor
|
please test |
Contributor
|
This one would have to be signed and merged by 11PM CET for the backport to be included in 16_0_0 |
Contributor
|
+heterogeneous |
Contributor
|
@mandrenguyen thanks for clarifying the timeline. |
Contributor
|
-1 Failed Tests: UnitTests RelVals-AMD_W7900 Failed Unit TestsI found 1 errors in the following unit tests: ---> test RecoTrackerLSTCore-standalone-compilation had ERRORS Failed RelVals-AMD_W7900
Comparison SummarySummary:
|
Contributor
Seems like this comment removed the dependency on #49964, but it works for me if I run the unit test locally. |
Contributor
Yes, I wanted to make sure the failures were not due to some strange interplay. |
Contributor
|
ignore tests-rejected with ib-failure |
Contributor
|
Are the @cms-sw/geometry-l2 and @cms-sw/ml-l2 signatures very relevant or just incidental. If it's the latter, we can consider bypassing them. |
Contributor
|
The changes should be purely technical, mostly swapping the order of two arguments to make them consistent with the other SoA constructors. |
Contributor
Contributor
|
The |
Contributor
|
merge |
This was referenced Jan 30, 2026
cmsbuild
added a commit
that referenced
this pull request
Jan 30, 2026
[Backport to 16_0_X (#49860)] Refactor of PortableCollections
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
11 participants
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.
The
PortableCollectionshave an inconsistent interface if used with a normalLayoutor with anSoABlocks Layout. In the later case, theAlpaka Queueis the first argument, while in the first case it is the other way around. This PR addresses the problem and sets theAlpaka Queuealways as the first argument.Furthermore, the constructors of the
PortableCollectionsare streamlined to always usestd::integalas the input type for the elements. Then achecked_casthas been implemented that checks if casting tointis safe. If so, the inputs are casted toint. Casting tointis necessary since theSoA Layoutsuseintfor the number of elements due to the dependency onROOT.This PR has to be addressed after #49734.
@felicepantaleo fyi