Add possibility for a CopyToHost::postCopy() operation

[makortel]

PR description:

Following #45708 (comment) this PR adds the possibility for a CopyToHost<T>::postCopy() function, that is called by the implicit data product device-to-host copy operation after the copy has finished (but only if the postCopy() function is defined). This facility allows data products that need to be updated after a memcpy() (e.g. because they have pointers to itself) to be used without blocking synchronization calls in CopyToHost::copyAsync(). I expect (hope) we will ever have only at most few such data products.

The first commit has a C++17 SFINAE -based solution for checking if the CopyToHost<T>::postCopy() exists (that can be backported to 14_0_X), and the second commit replaces that with C++20 concepts-based solution.

Resolves cms-sw/framework-team#989

PR validation:

Added unit test runs on CPU-only and NVIDIA GPU nodes.

If this PR is a backport please specify the original PR and why you need to backport that PR. If this PR will be backported please specify to which release cycle the backport is meant for:

To be backported to 14_1_X, and to 14_0_X (first commit only). Following #45708 (comment) not to be backported.

[cmsbuild]

cms-bot internal usage

[cmsbuild]

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-45801/41540

• There are other open Pull requests which might conflict with changes you have proposed:
  • File HeterogeneousCore/AlpakaCore/README.md modified in PR(s): [RFC] Add {Copy,Move}ToDeviceCache<T> class templates and moveToDeviceAsync function template #43969

[cmsbuild]

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

It involves the following packages:

• DataFormats/PortableTestObjects (heterogeneous)
• HeterogeneousCore/AlpakaCore (heterogeneous)
• HeterogeneousCore/AlpakaInterface (heterogeneous)
• HeterogeneousCore/AlpakaTest (heterogeneous)

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

[makortel]

enable gpu

[makortel]

@cmsbuild, please test

[makortel]

type -changes-dataformats

The added data format classes are transient (and part of testing suite).

[cmsbuild]

+1

Size: This PR adds an extra 40KB to repository
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-824152/41145/summary.html
COMMIT: 6c357de
CMSSW: CMSSW_14_1_X_2024-08-26-1100/el8_amd64_gcc12
Additional Tests: GPU
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/45801/41145/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

• You potentially removed 5 lines from the logs
• Reco comparison results: 8 differences found in the comparisons
• DQMHistoTests: Total files compared: 44
• DQMHistoTests: Total histograms compared: 3328202
• DQMHistoTests: Total failures: 3
• DQMHistoTests: Total nulls: 0
• DQMHistoTests: Total successes: 3328179
• DQMHistoTests: Total skipped: 20
• DQMHistoTests: Total Missing objects: 0
• DQMHistoSizes: Histogram memory added: 0.0 KiB( 43 files compared)
• Checked 191 log files, 161 edm output root files, 44 DQM output files
• TriggerResults: no differences found

GPU Comparison Summary

Summary:

• You potentially added 10 lines to the logs
• Reco comparison results: 1 differences found in the comparisons
• DQMHistoTests: Total files compared: 6
• DQMHistoTests: Total histograms compared: 37022
• DQMHistoTests: Total failures: 662
• DQMHistoTests: Total nulls: 0
• DQMHistoTests: Total successes: 36360
• DQMHistoTests: Total skipped: 0
• DQMHistoTests: Total Missing objects: 0
• DQMHistoSizes: Histogram memory added: 0.0 KiB( 5 files compared)
• Checked 20 log files, 25 edm output root files, 6 DQM output files
• TriggerResults: no differences found

[cmsbuild]

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

[cmsbuild]

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-45801/41622

• There are other open Pull requests which might conflict with changes you have proposed:
  • File HeterogeneousCore/AlpakaCore/README.md modified in PR(s): [RFC] Add {Copy,Move}ToDeviceCache<T> class templates and moveToDeviceAsync function template #43969

[cmsbuild]

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

[makortel]

@cmsbuild, please test

[cmsbuild]

+1

Size: This PR adds an extra 20KB to repository
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-824152/41265/summary.html
COMMIT: 0db641f
CMSSW: CMSSW_14_2_X_2024-09-03-1100/el8_amd64_gcc12
Additional Tests: GPU
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/45801/41265/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

• You potentially added 3 lines to the logs
• Reco comparison results: 4 differences found in the comparisons
• DQMHistoTests: Total files compared: 44
• DQMHistoTests: Total histograms compared: 3328315
• DQMHistoTests: Total failures: 0
• DQMHistoTests: Total nulls: 0
• DQMHistoTests: Total successes: 3328295
• DQMHistoTests: Total skipped: 20
• DQMHistoTests: Total Missing objects: 0
• DQMHistoSizes: Histogram memory added: 0.0 KiB( 43 files compared)
• Checked 193 log files, 163 edm output root files, 44 DQM output files
• TriggerResults: no differences found

GPU Comparison Summary

Summary:

• You potentially removed 31 lines from the logs
• Reco comparison results: 1 differences found in the comparisons
• DQMHistoTests: Total files compared: 6
• DQMHistoTests: Total histograms compared: 37044
• DQMHistoTests: Total failures: 665
• DQMHistoTests: Total nulls: 0
• DQMHistoTests: Total successes: 36379
• DQMHistoTests: Total skipped: 0
• DQMHistoTests: Total Missing objects: 0
• DQMHistoSizes: Histogram memory added: 0.0 KiB( 5 files compared)
• Checked 20 log files, 25 edm output root files, 6 DQM output files
• TriggerResults: no differences found

[fwyzard]

+heterogeneous

@makortel I'm fine with the changes and with this approach.

I'm wondering if there could be a more self-contained approach, where the data structure could define a postCopy() or finalize() method that is automatically called, instead of adding the static postCopy() function in CopyToHost.
But that kind of clashes with the fact that SoAs don't generally have methods :-/

[cmsbuild]

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

[mandrenguyen]

+1

[makortel]

I'm wondering if there could be a more self-contained approach, where the data structure could define a postCopy() or finalize() method that is automatically called, instead of adding the static postCopy() function in CopyToHost.
But that kind of clashes with the fact that SoAs don't generally have methods :-/

The thing next closest to a method would be a standalone function. Just to remind, with CopyToHost I started with just function overloads, but rejected that approach when it became evident that inheritance would be a tolerated pattern in crafting the data format classes (because a function copyToHost(T const&) would bind fine for all types D that inherit from T, and I wanted those cases to result a compilation error instead because the function must be different for D). Therefore I did not consider standalone functions for this case.

Within the class template specialization approach I considered making a separate (say PostCopy<T>) class template, because I felt the operation would be a property of T rather than of "copying T to host". But then I realized the operation needs to be different for copy-to-host and copy-to-device (at minimum copy-to-device needs to be a kernel call), and furthermore for copy-to-device the modification kernel (or memset()) can be launched from the CopyToDevice<T>::copyAsync() directly (or in case of a kernel probably via a host-side function called from there).

Given that the postCopy() is, in the end, tied to the CopyToHost operation, and that I hope the postCopy() won't be needed widely, I opted in adding it to the CopyToHost class template specialization. At least the two parts of the copy operation end up close together in the code.

These decisions can, of course, be revisited if my assumptions above turn out to be wrong.