Clarify ALPAKA_ACCELERATOR_NAMESPACE and CopyToDevice in AlpakaCore README#49709
Clarify ALPAKA_ACCELERATOR_NAMESPACE and CopyToDevice in AlpakaCore README#49709cmsbuild merged 2 commits intocms-sw:masterfrom
Conversation
|
cms-bot internal usage |
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-49709/47281
|
|
A new Pull Request was created by @makortel for master. It involves the following packages:
@cmsbuild, @fwyzard, @makortel can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
|
@cmsbuild, please test Even if there is nothing to test |
| In EDProducers for each device-side data product a transfer from the device memory space to the host memory space is registered automatically. The data product is copied only if the job has another EDModule that consumes the host-side data product. For each device-side data product a specialization of `cms::alpakatools::CopyToHost` is required to exist. | ||
|
|
||
| In addition, for each host-side data product a transfer from the host memory space to the device meory space is registered autmatically **if** a `cms::alpakatools::CopyToDevice` specialization exists. The data product is copied only if the job has another EDModule that consumes the device-side data product. | ||
| In addition, for each host-side data product a transfer from the host memory space to the device meory space is registered automatically **if** a `cms::alpakatools::CopyToDevice` specialization exists. The data product is copied only if the job has another EDModule that consumes the device-side data product. **Note:** The header where the `cms::alpakatools::CopyToDevice` specialization is defined must be `#include`d in the EDProducer source file for the transfer to be registered, even if it is _seemingly_ unused. For example, for `PortableCollection` that means `DataFormats/Portable/interface/PortableCollection.h`. |
There was a problem hiding this comment.
Figuring out if the necessary #include is missing can be time consuming. I'm wondering if we should think about making the intent of whether a data product should be implicitly copied to the device or not more explicit (e.g. via additional template argument to produces()).
There was a problem hiding this comment.
Maybe it would be simpler to make sure the data format itself includes the cms::alpakatools::CopyToDevice specialization ?
Can we clean up the various PortableCollection-related headers to simplify things ?
There was a problem hiding this comment.
Maybe it would be simpler to make sure the data format itself includes the
cms::alpakatools::CopyToDevicespecialization ?
That would make the host data type header dependent on the device data type header (e.g. PortableHostCollection.h would depend on PortableDeviceCollection.h). But dependency-wise PortableDeviceCollection does not really add anything that PortableHostCollection wouldn't already depend on. And PortableDeviceCollection is templated on the device type (as effectively are the CopyTo{Host,Device}, so having those templates available during host-only compilation should have any ill effects (beyond tiny increase in compilation time) as long as they are not instantiated.
So maybe combining PortableHostCollection.h, PortableDeviceCollection.h, and the two PortableCollection.h's could work.
|
-1 Failed Tests: RelVals-INPUT Failed RelVals-INPUT
Comparison SummarySummary:
|
|
Comparison differences are related to #47071 |
|
ignore tests-rejected with external-failure
The workflow fails with an unreachable file also in the IBs. |
|
@fwyzard Do you have any comments? |
|
I'm ok with the clarifications, and if something else comes up we can update the README.md further. |
|
+heterogeneous |
|
This pull request is fully signed and it will be integrated in one of the next master IBs (test failures were overridden). This pull request will now be reviewed by the release team before it's merged. @ftenchini, @mandrenguyen, @sextonkennedy (and backports should be raised in the release meeting by the corresponding L2) |
|
+1 |
4 participants
PR description:
This PR extends the
HeterogeneousCore/AlpakaCore/README.mdby two clarificationsALPAKA_ACCELERATOR_NAMESPACEshould be usedCopyToDevicespecialization must be#includedThese cases came up in private discussions and I noticed we hadn't documented them.
Resolves cms-sw/framework-team#1755
PR validation:
None